-
Notifications
You must be signed in to change notification settings - Fork 30
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
PXP-459 Futurize/modernize (make it python 2/3 compatible) #16
Conversation
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.
Looks great!
I still don’t run doctest though since python -m doctest fails on python3 (No module named 'http.client'; 'http' is not a package ) and there are doctest errors in other files.
error: Argument 1 to "urlencode" has incompatible type "list[str]"; expected "Union[Mapping[Any, Any], Mapping[Any, Sequence[Any]], Sequence[tuple[Any, Any]], Sequence[tuple[Any, Sequence[Any]]]]" [arg-type]
I want to call unittest.assertIsInstance
Distutils had a `requires` keyword argument, but “`requires` is superseded by `install_requires` and should not be used anymore.” https://docs.python.org/3/distutils/setupscript.html#relationships-between-distributions-and-packages https://setuptools.pypa.io/en/latest/references/keywords.html#keyword-requires
Add the test requirements to extra_require in setup.py, and use pip freeze to list the exact versions of each dependency in requirements.txt so that tests are reproducible. Use python2 to pip freeze since the last version of pyparsing in python2 is 2.4.7 whereas it is 3.1.1 in python3
This duplicates requirements-test.txt
Add test that requirements.txt contains all the setup.py install_requires, and test that requirements.txt contains all the recursive dependencies.
“Nose has been in maintenance mode for the past several years and will likely cease without a new person/team to take over maintainership. New projects should consider using Nose2, py.test, or just plain unittest/unittest2.” https://nose.readthedocs.io/en/latest/ We can run `python -m unittest discover` instead of nose as of python 2.7 / 3.2 https://docs.python.org/3/library/unittest.html#unittest-test-discovery Use functools.wraps (python 2.5) instead of nose.tools.make_decorator
Fix errors given by flake8 --select=F401
Fix NameError that were caught by flake8 --select=F821, or from running mypy.
Use == and != to compare strings, not the is operator, since other implementations are not guaranteed to return True. In python3 this fixes SyntaxWarning: "is not" with a literal. Did you mean "!="?
When overriding __eq__, call isinstance instead of type(self) == type(other), and return NotImplemented instead of returning False so that python will try the reflected comparison. This allows a different class to consider itself equal to this class.
python -m autopep8 --in-place --recursive . E401 multiple imports on one line W291 trailing whitespace W391 blank line at end of file Additionally, fix this one manually W293 blank line contains whitespace
Use flake8 version 3.9.2 which can run from python 2.7 to 3.11 (but not python 3.12). Use a .flake8 file that ignores pep8 whitespace issues
Mox (https://code.google.com/archive/p/pymox/wikis/MoxDocumentation.wiki) and mox3 (https://opendev.org/openstack/mox3/) are deprecated; mock (https://mock.readthedocs.io/en/latest/) is the recommended replacement. We can use mock for now and then switch to unittest.mock when we drop python<3.3 support
These old methods were considered deprecated as of python 2.7, logged DeprecationWarnings in python 3.2, and were removed in python 3.12. assert_ -> assertTrue failIf -> assertFalse assertEquals -> assertEqual self\.assertTrue\(isinstance\((.*)\)(, .*)?\) -> self.assertIsInstance($1$2)
cgi.parse_qs has been deprecated since 2.6 (2008) (https://docs.python.org/2.6/whatsnew/2.6.html, https://bugs.python.org/issue600362) and was removed in python 3.0. 2to3 and derived fixers (modernize, futurize) do not fix cgi.parse_qs; you need to move it to urlparse first.
Add support for python 3 while retaining python 2.7 compatibility. Most of the changes were automatically done by modernize and futurize. I manually undid some of the unnecessary list(d.items()) from 2to3 --fix=dict. And I had to add b'' binary literals in some places. The more complex changes to make it compatible with python 3 are in the following commits. Add matrix to test on python 3. Add recursive dependencies to requirements.txt and requirements-test.txt.
python3 pickle checks that the __qualname__ does not contain <locals>, so we have to set a fake value in this test AttributeError: Can't pickle local object 'TestDataObjects.set_up_pickling_class.<locals>.BasicMost'
In python2, we subclassed JSONDecoder to forgive byte sequencess that are not valid UTF-8. But in python3 we can't do that because the bytes are decoded to str prior to JSONDecoder. This change makes us decode the whole bytes ourselves before calling json.loads.
ListObject used to set 'offset' and then 'limit', but the unit test assumes that the order is limit=...&offset=.... Since dict and kwargs are guaranteed to preserve insertion order in python3.6+ (https://www.python.org/dev/peps/pep-0468/), this change makes them insert in the same order as the test expects Fixes AssertionError: 'http://example.com/foo?offset=0&limit=10' != 'http://example.com/foo?limit=10&offset=0'
Upgrade simplejson to 3.3.0 since prior to that, decoding json with lone surrogates would raise “JSONDecodeError: Unpaired high surrogate”
@stanimoto since your review at f7b9c1d, I added a few more minor changes to be91cde. Then I performed force pushes with equal trees from be91cde to 3116611 to 532fba7 in order to clean up history (combine the futurize steps, rearrange commits which could be applied to python2, remove references to I don’t have permission to rebase and merge to this repo (“Only those with write access to this repository can merge pull requests.”). Would you mind doing so? |
I've updated the access to this repo. Could you try it now? |
PXP-459
Use the futurize script and future backport library to make this library compatible with both python2 and python3. Then I manually switched from
future.utils
andbuiltins
tosix.moves
to remove thefuture
dependency and to avoid changing python2 behavior (urlparse
).Generally, these are the steps to using futurize:
future~=0.18.2
library (which comes with the futurize script)futurize --stage1 *.py remoteobjects examples tests --write --nobackups
on all the files.futurize --stage1
consists of “safe” fixes: “The goal for this stage is to create most of the diff for the entire porting process, but without introducing any bugs.”, but “The output of stage 1 will probably not (yet) run on Python 3.”futurize --stage2 --unicode-literals *.py remoteobjects examples tests --write --nobackups
on all the files.futurize --stage2
. Stage2 makes the code import__future__
andfuture
backports so that hopefully it behaves the same on python2 and python3.b'…'
and functions likebytes
''
and functions likefuture.util.native_str
u'…'
or future’sbuiltins.str
Here are notes specific to remoteobjects
from __future__ import unicode_literals
since most strings are python attributes (keys of__dict__
,__get__
,__setattr__
,__name__
, etc) which must be native_str in python2 and gives weird errors if you use unicode (e.g. error onimport remoteobjects.fields
: “AttributeError: 'module' object has no attribute 'fields'” because one of the functions’__name__
was unicode instead of native_str)future.utils.with_metaclass
threw exceptions whereassix.with_metaclass
worked.str
andunicode
args (parse.py). On the other hand, six just renames the functions from python2 urllib that allow mixing unicode with str.from __future__ import absolute_import
to every file which is unnecessary for the most partlist()
wrappers infor x in d.keys():
, which is a bug in modernize so I filed a PR Add back list() when iterating over d.items() in a for statement PyCQA/modernize#262six.moves.urllib.urlparse
instead of future’s backfilledurllib.urlparse
. I went with modernize’s solution.Testing