-
-
Notifications
You must be signed in to change notification settings - Fork 348
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.6 support #2210
Drop Python 3.6 support #2210
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2210 +/- ##
==========================================
+ Coverage 99.51% 99.61% +0.10%
==========================================
Files 114 114
Lines 14803 14731 -72
Branches 2349 2334 -15
==========================================
- Hits 14731 14675 -56
+ Misses 47 38 -9
+ Partials 25 18 -7
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are many other things to remove:
- the ipython hack in ci.sh
- the mention of "Python 3.6" in docs/source/reference-io.rst
- setup.py that still says "3.6-or-better"
- trio/_core/_entry_queue: we should say dict is always ordered
- trio/_core/_run.py: remove _count_context_run_tb_frames and CONTEXT_RUN_TB_FRAMES
- trio/_highlevel_ssl_helpers.py: remove lines that says py36
- trio/_util.py: remove "and you're using Python 3.6"
There are others, but I stopped at this point.
My strategy for finding those is to grep for 3.6, py36 and "3, 6" and do the same for 3.5 and 3.7, excluding notes-to-self and docs/source/history.rst. I then ask myself: "if written today, would we still mention the version number".
Turns out it this is still needed on PyPy.
I've updated all the code and comments I could find. |
PyPy added support in 7.3.4, the first release with Python 3.7 support that was not beta.
They always exist in Python 3.7+
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I added three more commits, one of them found when searching for "3.7" (in requirements.txt) and the other found by looking at the coverage changes. There are two coverage issues left that I don't know how to fix properly:
if hasattr(ssl, "OP_NO_TLSv1_3")
is now always True, so we don't test the other configuration as hoped. I don't think there's a good way to use an old OpenSSL version, so we should probably bite the bullet and remove that branch- In trio/_util.py, BaseMeta is always ABCMeta in tests, does that mean we can remove the GenericMeta branch? The comment suggests otherwise
See those issues for yourself in https://app.codecov.io/gh/python-trio/trio/compare/2210/changes/. Those two issues seem minor enough that I've approved this PR, but please review my commits before merging.
This flag was introduced in Python 3.7 so it's always there now.
I looked through your changes and they seem fine to me. |
Is it customary to squash commits on a merge in this projects? That would be my preference here. |
Thanks for the review and the assistance! |
Thank you for working on this! BTW I forgot, but we need a deprecation newsfragment. It can be as simple as https://github.com/python-trio/trio/pull/1481/files#diff-01d318a61ac955b73555728cb95e87911f78ed8cca35f236214f2403c5df81a3 |
This is the (required) first step towards adopting PEP 654 (exception groups).