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 #499

Merged
merged 1 commit into from
Feb 13, 2023
Merged

Drop Python 3.8 support #499

merged 1 commit into from
Feb 13, 2023

Conversation

djhoese
Copy link
Member

@djhoese djhoese commented Feb 10, 2023

It's what all the cool kids are doing.

@djhoese djhoese added enhancement backwards-incompatibility Causes backwards incompatibility or introduces a deprecation labels Feb 10, 2023
@djhoese djhoese self-assigned this Feb 10, 2023
@djhoese
Copy link
Member Author

djhoese commented Feb 10, 2023

Note I also removed some old multiprocessing import code in setup.py that said it could be removed when Python 2 support was dropped.

pnuu
pnuu approved these changes Feb 10, 2023
@codecov
Copy link

codecov bot commented Feb 10, 2023

Codecov Report

Merging #499 (0cc31d4) into main (b8ecc71) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #499   +/-   ##
=======================================
  Coverage   94.33%   94.33%           
=======================================
  Files          74       74           
  Lines       12947    12947           
=======================================
  Hits        12214    12214           
  Misses        733      733           
Flag Coverage Δ
unittests 94.33% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@pnuu
Copy link
Member

pnuu commented Feb 10, 2023

Gnaah. Don't know what I did there. But LGTM.

One thought I had was that maybe the python_requires shouldn't be so strict, most of the code will work fine with earlier versions. Though even in that case we should state that only the tested Python versions are actually supported.

@djhoese
Copy link
Member Author

djhoese commented Feb 10, 2023

I'm noticing codecov is complaining of lost coverage in the multiprocessing parts (_spatial_mp.py) for modules that haven't been changed, even in #498 which doesn't have these python version changes. Makes me wonder if something has changed in some package (numpy? pyproj?) that is no longer accessing the __iter__ or other parts of these modules.

@djhoese
Copy link
Member Author

djhoese commented Feb 10, 2023

Though even in that case we should state that only the tested Python versions are actually supported.

The problem is that python_requires can control what versions of the library PyPI returns. So if we make a release that supports Python 3.7 and then make a bug fix release that introduces syntax that is 3.8+ only then we'd have to remember to drop support in a bug fix version (or release a new minor release just for a bug fix). My point is I think CI needs to stay in sync with what we advertise as supported and it might be better to do it sooner rather than later. 🤷‍♂️

@pnuu
Copy link
Member

pnuu commented Feb 10, 2023

Ah, right. That'd get complicated and confusing quickly. So yeah, lets stay in sync with the CI.

@coveralls
Copy link

Coverage Status

Coverage: 93.862%. Remained the same when pulling 0cc31d4 on djhoese:drop-py38 into b8ecc71 on pytroll:main.

@djhoese djhoese merged commit f76d5a1 into pytroll:main Feb 13, 2023
@djhoese djhoese deleted the drop-py38 branch February 13, 2023 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backwards-incompatibility Causes backwards incompatibility or introduces a deprecation enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants