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

feat: drop Python 3.9 and test on Python 3.10/3.12 #9213

Merged
merged 6 commits into from
May 22, 2024

Conversation

deepyaman
Copy link
Contributor

@deepyaman deepyaman commented May 20, 2024

Description of changes

According to NEP-29, we could have dropped Python 3.9 last month.

  • Test on 3.10/3.12 in CI, except for a few exceptions (e.g. Flink).
  • Apply pyupgrade rules to remove code for Python<3.10.
  • Remove workaround for DuckDB geospatial tests.

Issues closed

N/A

@deepyaman deepyaman force-pushed the refactor/pyupgrade/drop-py39 branch 3 times, most recently from e6740c2 to 7187b22 Compare May 20, 2024 18:36
@deepyaman deepyaman changed the title refactor(pyupgrade): target Python 3.10 in upgrade chore: drop Python 3.9 support; test on 3.10, 3.12 May 20, 2024
@deepyaman deepyaman marked this pull request as ready for review May 20, 2024 20:16
Copy link
Member

@jcrist jcrist left a comment

Choose a reason for hiding this comment

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

Overall this looks (mostly) good to me. There are two (kinds) of changes made by pyupgrade that I'd like to revert, but the CI and metadata updates look good to me.

ibis/backends/bigquery/__init__.py Outdated Show resolved Hide resolved
ibis/backends/bigquery/compiler.py Outdated Show resolved Hide resolved
ci/make_geography_db.py Outdated Show resolved Hide resolved
@deepyaman deepyaman force-pushed the refactor/pyupgrade/drop-py39 branch from b4d6aa3 to 928eaa1 Compare May 20, 2024 23:52
@deepyaman deepyaman requested a review from jcrist May 21, 2024 13:25
Copy link
Member

@jcrist jcrist left a comment

Choose a reason for hiding this comment

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

Provided tests pass, this looks good to me! We should get a review by one other person before merging (@gforsyth?) just to ensure I didn't miss anything around the CI setup.

@deepyaman
Copy link
Contributor Author

deepyaman commented May 21, 2024

Provided tests pass, this looks good to me! We should get a review by one other person before merging (@gforsyth?) just to ensure I didn't miss anything around the CI setup.

FYI tests have passed; the required nix 3.9 test just won't complete, because it's removed.

@cpcloud
Copy link
Member

cpcloud commented May 21, 2024

I'll remove the required nix 3.9 test

@cpcloud
Copy link
Member

cpcloud commented May 21, 2024

@jcrist @deepyaman I think historically we made this a breaking change. Should we considering changing that convention?

@jcrist
Copy link
Member

jcrist commented May 21, 2024

I think historically we made this a breaking change. Should we considering changing that convention?

I think that dropping a Python version in a minor release seems fine (it's also pretty common in the ecosystem). Since we've updated the minimum version requirement, even pip's solver won't allow them to install the new release with an older version of python. The "breakage" here isn't something a user should be able to encounter.

Copy link
Member

@cpcloud cpcloud left a comment

Choose a reason for hiding this comment

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

LGTM.

Can we make this a feature or fix PR?

@deepyaman deepyaman force-pushed the refactor/pyupgrade/drop-py39 branch from a51f884 to 69d627f Compare May 21, 2024 18:43
@deepyaman deepyaman changed the title chore: drop Python 3.9 support; test on 3.10, 3.12 feat: drop Python 3.9 and test on Python 3.10/3.12 May 21, 2024
@deepyaman
Copy link
Contributor Author

LGTM.

Can we make this a feature or fix PR?

Done! Also rebased.

@deepyaman deepyaman force-pushed the refactor/pyupgrade/drop-py39 branch from 69d627f to 9483cf2 Compare May 21, 2024 23:31
@gforsyth gforsyth merged commit c06285e into ibis-project:main May 22, 2024
75 checks passed
@deepyaman deepyaman deleted the refactor/pyupgrade/drop-py39 branch May 22, 2024 14:23
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.

5 participants