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

Introduce black formatter #134

Merged
merged 5 commits into from
Nov 28, 2020
Merged

Introduce black formatter #134

merged 5 commits into from
Nov 28, 2020

Conversation

no2chem
Copy link
Contributor

@no2chem no2chem commented Nov 21, 2020

This PR introduces the black formatter as a pre-commit hook.

When you run git commit, black automatically re-formats the code before passing it to flake8.

example:

vscode ➜ /workspaces/wideq (format ✗) $ git commit -a -m "introduce black formatter"
black....................................................................Passed
Flake8...................................................................Passed

There are some differences in how black and flake8 interpret pep. I'm not really opinionated here as I am not a python expert, but it seems that the black authors think they are right in interpreting the PEP, and black fixes the formatting in a consistent way that an explicit rule shouldn't be required. The exceptions are defined in the .flake8 file.

This change should make it easier to compare changes, as small style differences should be make consistent by the tool. For example, #132 has a lot of changes because I ran black just to correct the flake8 errors. This eliminates formatting differences from pull requests.

You can always skip the precommit hooks by running

git commit --no-verify

@no2chem
Copy link
Contributor Author

no2chem commented Nov 21, 2020

well, it seems to really not want to run automatically on the test directory, not sure why...

@sampsyo
Copy link
Owner

sampsyo commented Nov 28, 2020

Hi there—although I'm not a frequent Black user, I'm OK with adding it if users are excited about automatic formatting. (TBH, I find gentle feedback from flake8 enough to ensure consistency, although I know some folks really get a lot of value out of automatic reformatters.)

If we're going to do this, however, I think it would be best to enforce its use "server-side" rather than relying on folks to use the pre-commit hook. That would mean reconfiguring our GitHub actions to run something like black --check or black-primer in addition to flake8 and mypy. Would anybody be interested in taking care of that?

pyproject.toml Outdated
@@ -1,5 +1,5 @@
[build-system]
requires = ["flit"]
requires = ["flit", "pre-commit"]
Copy link
Owner

Choose a reason for hiding this comment

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

I don't think this should be considered a build-time dependency.

README.md Outdated
-------
To ensure consistent formatting across pull requests, install the precommit hooks to auto format your code using `pre-commit install`.

The code will be auto-formatted by `black` to ensure consistent style.
Copy link
Owner

Choose a reason for hiding this comment

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

I'm gong to rephrase this a bit to discuss the use of Black more broadly.

@sampsyo sampsyo merged commit 267a880 into sampsyo:master Nov 28, 2020
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.

2 participants