[go: up one dir, main page]

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

Drop Python 3.8 support, require Python 3.9+ #2084

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

EwoutH
Copy link
Contributor
@EwoutH EwoutH commented Jul 10, 2024

With the migration to Shapely 2.0 successfully completed and Python 3.13 soon coming in, this seems like a good moment to drop Python 3.8 support. It's almost end-of-life and SPEC 0 already recommended dropping Python 3.8 over a year ago.

So this PR bumps the required Python version to 3.9, removes 3.8 from the CI and test files.

Of course this will only affect future releases from the main branch, existing releases and releases from maintenance branches (like maint-2.0) stay compatible with Python 3.8 and can continued to be used.

With the migration to Shapely 2.0 successfully completed and Python 3.13 soon coming in, this seems like a good moment to drop Python 3.8 support. It's almost [end-of-life](https://endoflife.date/python) and [SPEC 0](https://scientific-python.org/specs/spec-0000/) already recommended dropping Python 3.8 over a year ago.

So this PR bumps the required Python version to 3.9, removes 3.8 from the CI and test files.

Of course this will only affect future releases from the main branch, existing releases and releases from maintenance branches (like maint-2.0) stay compatible with Python 3.8 and can continued to be used.
@coveralls
Copy link
coveralls commented Jul 10, 2024

Pull Request Test Coverage Report for Build 9889471302

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 5 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.1%) to 87.966%

Files with Coverage Reduction New Missed Lines %
shapely/set_operations.py 5 93.83%
Totals Coverage Status
Change from base Build 9872181557: 0.1%
Covered Lines: 2617
Relevant Lines: 2975

💛 - Coveralls

@EwoutH
Copy link
Contributor Author
EwoutH commented Jul 11, 2024

The linting error is a bit weird. It are multiple cases of flake8 throwing a E231 missing whitespace after ',' (or after ':') error. But all those cases are in the middle of strings, not actual code.

What I find strange, is why is it showing up now.

@EwoutH
Copy link
Contributor Author
EwoutH commented Jul 11, 2024

Couldn't pinpoint what's happening exactly, but on Python 3.11, the flake8 linting job passes without problem, so let's use that for this PR. Probably somehow related to PyCQA/flake8#1845.

The Windows error seems unrelated, please re-run that job.

@EwoutH
Copy link
Contributor Author
EwoutH commented Jul 11, 2024

@mwtoews, since you did the previous two version bumps (in #1453 and #1802), could you review this PR?

Maybe we also want to update NumPy and GEOS minimum version, but that can also be done in another PR.

@adamjstewart
Copy link
Contributor

What I find strange, is why is it showing up now.

Each new Python release contains subtle changes to the parser. Python 3.12 specifically contains changes to f-string parsing, and may include other parser changes. This would mean that what was once a string is now parsed as a string containing an expression. The linter would then have access to this new information and provide more detailed feedback.

My suggestion would be to always use the latest Python release for linting and correct any new linter errors that pop up. Of course, you can do this in a different PR if you want to keep this one focused.

@mwtoews
Copy link
Member
mwtoews commented Jul 20, 2024

Hi @EwoutH, apologies for the delay. We'll drop at least Python 3.8, but perhaps more. I'm not sure where a discussion should be held (there could already be an issue for it, I can't recall). Essentially, I'd like to see adoption for SPEC 0 but retain compatibility with older/supported GEOS versions. I'll add a review here soon.

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.

None yet

4 participants