Using python-modernize
Steps
Run python-modernize on the Python files matching the INCR ticket you're working on, to make the code more compatible with Python 3.x without breaking Python 2.7 support:
Comment on the ticket to indicate that you are about to start working on it.
Launch a development shell in a Docker container via either "make shell" or devstack's "make lms-shell". Alternatively, you can create a new virtualenv or conda environment and install modernize and isort into it (this is a better choice on Windows).
python-modernize -w <directory you want to modernize>
isort -rc <directory you want to modernize>
Make sure the changes look reasonable and submit them as a pull request; mention this ticket in the description and include INCR-<Ticket Number> in the name.
Ask for tests to be triggered if they don't start automatically.
Diagnose any test failures caused by the changes, and either fix them or ask for help.
If you run into unexpected errors, see this document for common problems: https://openedx.atlassian.net/wiki/spaces/AC/pages/977666218/Using+python-modernize+effectively
Tips
The python-modernize
utility from the modernize package is very useful for quickly updating Python 2 code to also support Python 3, but the changes it makes aren't always ideal (or even correct). Here are some tips on how to improve its output.
- Also run isort on the output -
python-modernize
tends to add new imports after all existing imports, and when adding "from __future__ import absolute_imports"
fails to combine it on the same line with any existing similar imports. Runningisort
on the same files should fix this, just make sure there aren't any comments near reordered imports giving a good reason why they need to stay in an unusual order (we've caused a bug or two in the past by failing to notice this). - Incorrect cPickle imports -
python-modernize
replaces "cPickle" in any imports with "six.moves.cPickle"; this works sometimes, but not if the original code was "import cPickle as pickle". In this case, the rewritten import will fail and needs to be updated to "from six.moves import cPickle as pickle
". - Disable pylint warnings for some six.moves imports - The
six.moves
package is structured in a way that makes it difficult forpylint
to understand how it works, so it will often triggerimport-error
warnings on imports from it. Rather than rewrite the imports thatpython-modernize
suggests, it's usually better to append "# pylint: disable=import-error
" (two spaces before the comment) to the end of such lines when pylint complains about them. - Use of six.string_types outside of isinstance -
python-modernize
blindly replaces occurrences of "basestring" with "six.string_types". This is fine for its most common use as the second argument ofisinstance()
, but can fail in other contexts where the switch from a type to a tuple containing a single type causes problems. In such cases, you may need to replace "six.string_types" with "six.string_types[0]".
If you have to make other changes to the code generated by python-modernize
, please note them here.