-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
fix: replace unsafe six.PY3 with PY2 for better future compatibility with Python 4 #10081
Conversation
8f15925
to
4b7b089
Compare
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.
Testing for "is (not) PY2" instead of "is (not) PY3" is indeed more robust, thanks for catching that!
The change is done to an auto-generated file
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.
(speaking for the PubSub library)
The change is good, but needs to be made in the synth file that is used to generate the file changed here.
The changes to the files in other libraries are fine, as those files are not auto-generated (AFAIK).
153d003
to
1401457
Compare
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.
LGTM with one final remark.
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.
LGTM. Thanks @hugovk!
Will delay merging for a day or so if anybody else wants to review it.
The test failures are not related, also happen on master. Investigating. Update: At least for Update 2: The Storage failures also seem to be a regression, will submit a fix for that, too (edit: submitted). |
Fixes #10158.
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
(edit by @plamut: opened the issue for better traceability)
We don't yet know if 3.10 or 4.0 will follow Python 3.9, but whichever it is, it will probably happen in 2020 when Python 3.9 reaches beta and work begins on Python 3.9+1.
There's some code which uses
six.PY3
:Where:
When run on Python 4, this will run the Python 2 code!
Instead, use
six.PY2
.Found using https://github.com/asottile/flake8-2020.