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

fix: Drop python 3.8-3.9, upgrade deps, use ruff format #429

Merged
merged 5 commits into from
Oct 14, 2024
Merged

Conversation

max-sixty
Copy link
Member

No description provided.

@max-sixty max-sixty changed the title fix: Drop python 3.8, upgrade deps, use ruff format fix: Drop python 3.8-3.9, upgrade deps, use ruff format Oct 8, 2024
@max-sixty max-sixty requested a review from eitsupi October 8, 2024 17:38
@@ -23,30 +23,29 @@ repository = "https://github.com/prql/pyprql"
version = "0.12.1"

[tool.poetry.dependencies]
python = "^3.8"
python = "^3.10"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bumped this all the way to 3.10 since otherwise requires careful management of ipython dependency

Colab is on 3.10, which is some standard

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this.

Bumped this all the way to 3.10 since otherwise requires careful management of ipython dependency

Colab is on 3.10, which is some standard

Hmmm, I don't know if it's worth dropping Python 3.9 just for that.

Isn't Colab based on 3.10 because python3 on Ubuntu 22.04 is 3.10?
https://launchpad.net/ubuntu/jammy/+package/python3

That has nothing to do with the release cycle of Python itself (since Ubuntu LTS is released every two years)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's related to Ubuntu: https://colab.google/articles/py3.10

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the context!

In any case, I don't know if there's any justification for dropping Python 3.9, given that duckdb, pyarrow and polars support 3.9.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree it's not ideal. But pandas etc have dropped 3.9, and it seems like we're going to need to do some dependency surgery to make ipython work with it — I'm mostly thinking whether that's worthwhile...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why the pandas version is relevant here.
pandas does not seem a dependency of IPython.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I just mean that pandas is a big library that others follow — it's not like every other library support 3.9 except for us. Does that make more sense?

@max-sixty
Copy link
Member Author

I think unless someone wants to do the work to make it py-3.8 compatible, it's a net benefit to merge this...

@max-sixty max-sixty merged commit d85ec04 into main Oct 14, 2024
7 checks passed
@max-sixty max-sixty deleted the python3.8 branch October 14, 2024 16:48
Copy link
Member

@eitsupi eitsupi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think unless someone wants to do the work to make it py-3.8 compatible, it's a net benefit to merge this...

I don't understand why you treat Python 3.8 and 3.9 the same, Python 3.9 is still supported right?

@@ -23,30 +23,29 @@ repository = "https://github.com/prql/pyprql"
version = "0.12.1"

[tool.poetry.dependencies]
python = "^3.8"
python = "^3.10"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why the pandas version is relevant here.
pandas does not seem a dependency of IPython.

Copy link
Member

@eitsupi eitsupi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The badge shown in the README needs to be updated.

@eitsupi eitsupi mentioned this pull request Oct 14, 2024
2 tasks
@max-sixty
Copy link
Member Author

I don't understand why you treat Python 3.8 and 3.9 the same, Python 3.9 is still supported right?

3.8 & 3.9 are both difficult to get work; they require making custom IPython dependency specifications. The easiest way to see is to try and build this while supporting 3.9...

@eitsupi
Copy link
Member

eitsupi commented Oct 15, 2024

Given that upstream jupysql supports Python 3.9, I am not sure if your argument makes sense. In my opinion this is simply a limitation due to current CI and poetry, not a limitation of the Python package itself.

@max-sixty
Copy link
Member Author

I agree it's surprising. And I'm sure it's possible to support 3.8. It's just a tradeoff of whether it's worth the investment.

Big fan of uv, I use it myself. FWIW it required more rather than less work to get xarray working with it (but once it does, it's reliable and really fast)

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.

2 participants