Skip to content
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

Format and check all Python code using python black #1776

Merged
merged 2 commits into from
Dec 6, 2018
Merged

Conversation

wohali
Copy link
Member

@wohali wohali commented Nov 29, 2018

Inspired by our formatting work on Elixir, this PR proposes doing the same thing for our Python code.

Python Black is an opinionated code formatter now adopted by prominent Python projects such as pip. It is easy to use and provides both formatting and check modes.

The python-black target builds a python3 venv at .venv and installs black if possible. Since black is Python 3.6 and up only, we skip the check on systems with an older Python 3.x and print a warning. Our CI systems will still catch problems from people who submit PRs who don't have 3.6+ since CI is configured to run this check automatically on at least one platform (e.g., Jenkins Ubuntu Bionic, or Python 3.6 explicitly requested under Travis CI.)

Assuming the check runs, it fails if any file would be changed by the formatter.

A Makefile target is provided that actually does the updating (make python-black-update, it simply omits the --check flag) for happy dev fingers to fix up python files.

The change to the docs dependency in rebar.config.script is necessary because of embedded Python files in that repo that have only just been re-formatted. Once docs is tagged with 2.3.0, this dep will be reverted to a tag-type reference, not a commit.

@nickva I had to change the location of the Mango venv to src/mango/.venv so it would be automatically ignored by the formatter, otherwise it tries to format the entire venv. Oops :)

Testing recommendations

If you have Python 3.6+, try make python-black and see if it works.

if you don't, try the same command and make sure it is a noop (with an exit code of 0).

Checklist

  • Code is written and works correctly;
  • Changes are covered by tests;
  • Documentation reflects the changes;

@wohali wohali mentioned this pull request Nov 29, 2018
3 tasks
@wohali wohali force-pushed the python-black branch 4 times, most recently from 160c2ca to be6c902 Compare November 29, 2018 06:33
@wohali
Copy link
Member Author

wohali commented Dec 1, 2018

Any comments from @janl @davisp @iilyak @eiri or anyone else who does Python-y things? Waiting on a +1 or comments to the contrary.

Copy link
Member

@davisp davisp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 Yay consistency!

The Makefile target builds a python3 venv at .venv and installs
black if possible. Since black is Python 3.6 and up only, we
skip the check on systems with an older Python 3.x.
@wohali wohali merged commit 33d4f6d into master Dec 6, 2018
@wohali wohali deleted the python-black branch December 6, 2018 23:04
janl pushed a commit that referenced this pull request Feb 7, 2019
The Makefile target builds a python3 venv at .venv and installs
black if possible. Since black is Python 3.6 and up only, we
skip the check on systems with an older Python 3.x.
janl pushed a commit that referenced this pull request Feb 17, 2019
The Makefile target builds a python3 venv at .venv and installs
black if possible. Since black is Python 3.6 and up only, we
skip the check on systems with an older Python 3.x.
AGrzes pushed a commit to AGrzes/couchdb that referenced this pull request Nov 2, 2019
The Makefile target builds a python3 venv at .venv and installs
black if possible. Since black is Python 3.6 and up only, we
skip the check on systems with an older Python 3.x.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants