STYLE: Specify target-version for black #29607
Conversation
| @@ -10,3 +10,23 @@ requires = [ | |||
| "numpy==1.16.0; python_version=='3.6' and platform_system=='AIX'", | |||
| "numpy==1.16.0; python_version>='3.7' and platform_system=='AIX'", | |||
| ] | |||
|
|
|||
| [tool.black] | |||
| target-version = ['py36', 'py37', 'py38'] | |||
There was a problem hiding this comment.
Looking over the black issue tracker it looks like we should be specifying all versions instead of just the lower bound. The logic being that a new version of Python could change the syntax such that a kind of formatting that black thinks is ideal is no longer supported.
There was a problem hiding this comment.
is there a reason for pyproject.toml instead of setup.cfg?
There was a problem hiding this comment.
black does not support configuration through setup.cfg nor do they plan on supporting it in the future
| @@ -56,7 +56,7 @@ if [[ -z "$CHECK" || "$CHECK" == "lint" ]]; then | |||
| black --version | |||
|
|
|||
| MSG='Checking black formatting' ; echo $MSG | |||
| black . --check --exclude '(asv_bench/env|\.egg|\.git|\.hg|\.mypy_cache|\.nox|\.tox|\.venv|_build|buck-out|build|dist|setup.py)' | |||
| black . --check | |||
There was a problem hiding this comment.
Moved the --exclude option to pyproject.toml since I had to create an entry there for target-version. Also has the advantage that users running black (either explicitly or through pre-commit) will now pick this up as well, so local behavior will match CI.
|
can u update the template and pre-commit hooks as well? or do they read the pyproject.toml? |
|
I tested it locally and it looks like it reads the |
|
Yes, when we started using black, we didn't yet have the pyproject.toml, so needed to put the config flags into the CI call itself. But now we have that, we should indeed move the config there. |
|
lgtm. can you merge master (conflict); merge on green. |
|
thanks @jschendel ! |
The
target-versionoption explicitly tellsblackthat it can use Python features from the supplied versions. The default is per-file detection by looking for the presence specific syntax, e.g. f-strings would indicate py36.Currently
blackdoes not detect this for us since we just bumped to py36, so detection will not occur until f-strings are added, which would in turn necessitate these changes as part of the f-string PRs, making them a bit more convoluted than needed. This resolves that issue by explicitly tellingblackour supported versions and enforcing those changes for the entire codebase in this PR.