-
Notifications
You must be signed in to change notification settings - Fork 5.7k
Remove Python 2 support #1715
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
Remove Python 2 support #1715
Conversation
Thanks for the PR! |
Currently I do not keep close track of new additions to the code. Therefore I need to go over this again and perform a few changes at some places (e.g. I saw a few cases of old style classes and python 2 version checks). If there is an expected release date for v13 I will take some time before it to review the code and check for places needing changes. |
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.
Hi! I finally got around to have a look at your PR. Thanks again for you work!
Overall looks good to me. I left a few minor comments/questions. Of course, master would need to be merged. Would you care to do that?
Also, just a thought: Have you run pythons 2to3
on the lib to double check? But imo, it's okay, if we miss some py2 workarounds. We can always remove them, when we find them later on …
Currently I have a few other things to do but I will surely find some spare time to merge master ;) Yes I ran 2to3 and IIRC another converter though I have not rerun them after the latest commits merging master in here (also why I missed on of the Probably should also squash some of the commits merging master / rebase this or will this get done when this gets merged anyhow? I am not sure how you handle this and I know that rewriting history is generally not the best idea... |
That'd be great :) Take your time!
We'll squash on merge, so no need for sqaushing yourself |
Hi. Just wanted to leave a kind ping :) If you I see that you won't get around to merge master in the forseeable future, feel free to comment. Then I'll try and do that ;) |
Thanks for the ping :D. I may have some spare time tomorrow and friday.
Otherwise I will reach out to you.
…On Wed, Jun 10, 2020, 21:14 Bibo-Joshi ***@***.***> wrote:
Hi. Just wanted to leave a kind ping :) If you I see that you won't get
around to merge master in the forseeable future, feel free to comment. Then
I'll try and do that ;)
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1715 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AFZCIFHKP7NTZWJAZASQHJDRV7LSBANCNFSM4KI2JCWA>
.
|
Fix #1538 Failing test is |
Thanks for the update!
No need to worry. Just one more question about the r-strings & related stuff: Did some tests fail for you or did pyupgrade that for you? Just double checking as those gave me headache on the API 4.8 update and I don't want things to break :D |
Pyupgrade did those for me - I just did the line breaks manually |
Okay, then. Merging. Thanks a lot for your contribution! |
This pull requests removes several Python 2 specific code segments and workarounds.
Additionally the docs were updated to state that only Python 3.5+ is supported.
The current changes were done according to the checklist in #1538 and a few search queries over the codebase for version checks etc.
This pull request also removes a few checks for version < 3.5 as these are not supported anymore and I thought this would be a good time to to this as I was searching for them anyways. If this is not desired it can be reversed.
This pull request does not refactor any code to use more Python 3 only features as this would require manual checking of the entire codebase for places where such improvements could be made.