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.7 support, drop pypy3.7-3.8, add pypy3.10 (except on windows) #2668

Merged
merged 23 commits into from
Jul 17, 2023

Conversation

jakkdl
Copy link
Member

@jakkdl jakkdl commented Jun 23, 2023

With 3.7 EOL coming up in 4 days, I thought it time to start writing a PR for dropping it so #2655 can move ahead afterwards.

Only thing I stumbled on so far (until CI tells me otherwise) was trio/_util:coroutine_or_error - which seems to be more than an easy search/replace fix.

@jakkdl jakkdl requested a review from A5rocks June 23, 2023 12:10
@codecov
Copy link

codecov bot commented Jun 23, 2023

Codecov Report

Merging #2668 (08c972b) into master (6f187fb) will increase coverage by 0.04%.
The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2668      +/-   ##
==========================================
+ Coverage   98.84%   98.88%   +0.04%     
==========================================
  Files         113      113              
  Lines       16508    16482      -26     
  Branches     3010     3000      -10     
==========================================
- Hits        16317    16299      -18     
+ Misses        134      126       -8     
  Partials       57       57              
Impacted Files Coverage Δ
trio/_core/_tests/test_multierror.py 100.00% <ø> (ø)
trio/_path.py 100.00% <ø> (ø)
trio/_socket.py 100.00% <ø> (ø)
trio/_tests/test_ssl.py 99.85% <ø> (-0.01%) ⬇️
trio/_unix_pipes.py 100.00% <ø> (ø)
trio/_util.py 100.00% <ø> (ø)
trio/socket.py 100.00% <ø> (ø)
trio/_channel.py 100.00% <100.00%> (ø)
trio/_core/_run.py 99.24% <100.00%> (ø)
trio/_core/_tests/tutil.py 98.03% <100.00%> (+11.88%) ⬆️
... and 1 more

trio/_core/_tests/tutil.py Outdated Show resolved Hide resolved
trio/_util.py Show resolved Hide resolved
@A5rocks
Copy link
Contributor

A5rocks commented Jun 26, 2023

Hi, if I actually am able to release stuff I want to do a patch release with current changes (and without this PR), then soon after a minor release with this PR. This way there's a published version of trio that supports 3.7-3.12 (beta, at least). Sorry it took me a while to figure that out, but that means this PR will have to wait a bit!

@jakkdl
Copy link
Member Author

jakkdl commented Jun 26, 2023

Hi, if I actually am able to release stuff I want to do a patch release with current changes (and without this PR), then soon after a minor release with this PR. This way there's a published version of trio that supports 3.7-3.12 (beta, at least). Sorry it took me a while to figure that out, but that means this PR will have to wait a bit!

That sounds like a good approach, no worries at all!

@A5rocks
Copy link
Contributor

A5rocks commented Jun 26, 2023

We can probably replace supplement (I just realied @final does no runtime checking) _util.Final with a @final from typing?

https://github.com/python-trio/trio/blob/master/trio/_util.py#L268-L290

@jakkdl
Copy link
Member Author

jakkdl commented Jun 26, 2023

I have no clue what the notes-to-self directory is, but ran pyupgrade (and black) on that as well.

@jakkdl
Copy link
Member Author

jakkdl commented Jun 26, 2023

We can probably replace supplement (I just realied @final does no runtime checking) _util.Final with a @final from typing?

https://github.com/python-trio/trio/blob/master/trio/_util.py#L268-L290

Hmm, I'm failing to find a way to get both runtime and static checking to work with a single decorator/metaclass (it seems mypy checks for @final by identity against typing.final? Even reassigning typing.final to a different variable and using that variable as a decorator doesn't work). So this would entail doubling up and having both @final and metaclass=Final on the classes.

@jakkdl
Copy link
Member Author

jakkdl commented Jul 3, 2023

Dropping windows pypy3.10, leaving that for another PR.

@jakkdl jakkdl changed the title drop python 3.7 support drop python 3.7 support, drop pypy3.7 and pypy3.8, add pypy3.10 (except on windows) Jul 3, 2023
@jakkdl jakkdl changed the title drop python 3.7 support, drop pypy3.7 and pypy3.8, add pypy3.10 (except on windows) drop python 3.7 support, drop pypy3.7-3.8, add pypy3.10 (except on windows) Jul 3, 2023
@jakkdl jakkdl mentioned this pull request Jul 3, 2023
@jakkdl
Copy link
Member Author

jakkdl commented Jul 5, 2023

@A5rocks wanna review/merge?

Copy link
Contributor

@A5rocks A5rocks left a comment

Choose a reason for hiding this comment

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

Looks mostly good. Please edit pyproject.toml to make black use py38 as a minimum version. I don't think it changes anything though.

Sorry about the delay in reviewing! ... I swear I had another thing in mind to ask you to change but whatever, we'll catch it eventually and it won't break anything.

@@ -66,7 +66,7 @@ jobs:
# support this for PyPy presently so we get no help there.
Copy link
Contributor

Choose a reason for hiding this comment

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

note to self: test if this is supported yet. It probably works!

.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
@@ -143,7 +143,7 @@ jobs:
strategy:
fail-fast: false
matrix:
python: ['3.7', '3.8', '3.9', '3.10', 'pypy-3.8-nightly', 'pypy-3.9-nightly']
python: ['3.8', '3.9', '3.10', 'pypy-3.9-nightly', 'pypy-3.10-nightly']
Copy link
Contributor

Choose a reason for hiding this comment

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

Mind adding at least pypy 3.10 here?

Copy link
Member Author

Choose a reason for hiding this comment

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

In addition to pypy-3.10-nightly? Ubuntu tests both pypy-3.10 and pypy-3.10-nightly, windows & mac has always only tested pypy-nightlies (no clue why they do that instead of the stable releases though)

README.rst Outdated Show resolved Hide resolved
docs/source/reference-core.rst Outdated Show resolved Hide resolved
test-requirements.in Outdated Show resolved Hide resolved
test-requirements.txt Outdated Show resolved Hide resolved
trio/_core/_tests/tutil.py Show resolved Hide resolved
Copy link
Contributor

@A5rocks A5rocks left a comment

Choose a reason for hiding this comment

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

Just remembered the one more thing: can you make a news fragment?

@jakkdl jakkdl requested a review from A5rocks July 7, 2023 10:31
@@ -0,0 +1 @@
Drop support for python3.7, pypy3.7 and pypy3.8.
Copy link
Member

Choose a reason for hiding this comment

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

should be Python 3.7 and PyPy 3.7/3.8 or similar.

Copy link
Contributor

@A5rocks A5rocks left a comment

Choose a reason for hiding this comment

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

This seems fine now.

Except for capitalization in the newsfragment, I suppose.

Copy link
Contributor

@A5rocks A5rocks left a comment

Choose a reason for hiding this comment

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

Sorry, I've been thinking about it and IMO this isn't really a "headline" feature; in the past this was a .removal.

That plus the capitalization thing are my only concerns, after which feel free to merge!

@jakkdl
Copy link
Member Author

jakkdl commented Jul 17, 2023

This seems fine now.

Except for capitalization in the newsfragment, I suppose.

Oh, I missed that capitalization was what Fuyukai actually was requesting be modified. Thanks for clarifying :)

@jakkdl
Copy link
Member Author

jakkdl commented Jul 17, 2023

Sorry, I've been thinking about it and IMO this isn't really a "headline" feature; in the past this was a .removal.

That plus the capitalization thing are my only concerns, after which feel free to merge!

removal wasn't listed in newsfragments/README.rst - added it + description it should be used when dropping versions. Feel free to modify that if you want to

@jakkdl jakkdl merged commit bf9fe64 into python-trio:master Jul 17, 2023
@jakkdl jakkdl deleted the drop_3.7 branch July 17, 2023 07:59
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