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
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. Running
isort 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 for
pylint to understand how it works, so it will often trigger
import-error warnings on imports from it. Rather than rewrite the imports that
python-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 of
isinstance(), 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".
If you have to make other changes to the code generated by
python-modernize, please note them here.