Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove nbdev #29

Merged
merged 2 commits into from
Oct 17, 2024
Merged

Remove nbdev #29

merged 2 commits into from
Oct 17, 2024

Conversation

Askir
Copy link
Contributor

@Askir Askir commented Oct 16, 2024

Changes to Remove nbdev

I did a few things to get rid of nbdev so far, thought I'd already open a PR to discuss how to move forward:

1. Moved examples into simple pytest code

These are not quite idiomatic test functions right now:

  • They depend on a running timescale instance as well as a working OpenAI API connection.
  • They are also simply quite long and hard to read.

I would aim to clean them up in the future. However there is value in continuing to run the exact example code in tests as nbdev used to do. This guarantees that users of the library have a smooth experience when reading and experimenting with the docs.

2. Removed nbdev specific comments and files

3. Moved the build and dependency system to poetry uv

This is simply the tool I am most familiar with, happy to use uv instead if we decide this is the better way to go. It is definitely more compliant with newer PEPs.

4. Correctly identified 3.10 as the minimal python version

There is simply some isinstance(Union) code that doesn't work in python <= 3.9 it was previously labelled wrongly as compatible.

5. Installed ruff for formatting and linting

I tried to also fix linting errors but there is still 70 left, I'd fix this in a separate PR.

What else is missing?

1. CI hasn't been changed at all yet

Tests are skipped right now anyways and I cannot simply run pytest due to the problems mentioned in 1.). I also haven't adapted the build action yet. I am not sure if I should do this considering the goal is to merge this with pgai (and subsequently we have to unify the build and dependency mangement there anyway).

2. Notebooks are sitll present in the nbs folder

I think there is probably some value in having example notebooks around and maybe even generate the docs from them? Happy to discuss this further, didn't put much thought into this yet.

@CLAassistant
Copy link

CLAassistant commented Oct 16, 2024

CLA assistant check
All committers have signed the CLA.

@Askir Askir requested a review from cevian October 16, 2024 23:24
@alejandrodnm
Copy link

alejandrodnm commented Oct 17, 2024

Hey @Askir I was planning on standardize our projects to use UV do you think we should do that, or use poetry?

@Askir
Copy link
Contributor Author

Askir commented Oct 17, 2024

@alejandrodnm happy to try out uv again.
I changed the PR. First impressions:

  • It's nice that uv ships with a python version manager as well, no need for pyenv
  • Installing dev dependencies seems a more cumbersome than with poetry (I had to manually run uv pip install -e ".[dev]" in addition to uv lock/uv sync)
  • uv doesn't have a build backend yet so I also added hatch another tool, I haven't used before.

@Askir Askir merged commit 108539c into main Oct 17, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants