-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Is --target-version the minimum or should all be explicitly listed? #751
Comments
You should include all Python versions that you want your code to run under. In practice, though, 3.6 through 3.8 are currently the same as far as Black is concerned, so either variant will do the same. Let us know if you think the docs can be improved. |
Jelle, I get that certain edge cases in the grammar are only available in the newer versions, so we need the lower bound. Why do we need the upper bound, too? |
FWIW I just had to look into black's source cod too, to figure out how to set target-version. From the docs it's unclear that it takes multiple versions and reusing I think it's fine to allow supplying a range (although I'd prefer something like |
Another confusion. Let's say I support Python 3.5-3.8 so have this in [tool.black]
target_version = ['py35', 'py36', 'py37', 'py38'] If I don't have this config file and want to do the same thing directly on the command line, do I need to repeat each argument? $ black --target-version py35 --target-version py36 --target-version py37 --target-version py38 . And then would repos:
- repo: https://github.com/psf/black
rev: 19.10b0
hooks:
- id: black
args: ["--target-version", "py35", "--target-version", "py36", "--target-version", "py37", "--target-version", "py38"] These are pretty long, and it'd be nice to have a shorter range argument (or just a single lower bound). I'm also curious why we need the upper bound. In practice, 3.5-3.8 is currently the same as just 3.5. What sort of theoretical cases are there where it'd be different? Thanks! |
In theory, a new version of Python could change the syntax such that a kind of formatting that Black thinks is ideal is no longer supported. As a hypothetical example, maybe Python 3.9 will decide that all string prefixes have to be uppercase, so In practice that sort of scenario doesn't seem terribly likely, so I'd be OK with making target_version just a minimum requirement. |
I was very surprised to discover it wasn't already a minimum requirement, and that only happened when I stumbled across this issue! FWIW I think making forward-compatibilty the easy option is also better for the ecosystem 😄 |
At the same time I agree that it should almost certainly be 3.6+, 3.7+, 3.8+, and so on. But Jelle's got a point about possible syntax breaking changes in the future. I'm not sure if we should make this change. |
I'd hope the answer is "Python 2 taught us not to make breaking changes to the syntax" - and if we do, adding an optional |
My suggestion would be to read See https://github.com/joerick/cibuildwheel/blob/58c5e56e9f271e32020b687f62cc02003602edd2/cibuildwheel/__main__.py#L173 and https://github.com/joerick/cibuildwheel/blob/58c5e56e9f271e32020b687f62cc02003602edd2/cibuildwheel/util.py#L77 |
This doesn't apply for python 3.3 does it? Black still outputs f-strings even though they are not supported. |
Black doesn't support Python 3.3. Also, note that you're quoting a hypothetical out of context. |
You can alternatively write the yaml with the arguments stacked in a vertical list instead, which I think is a lot easier to read when you're specifying more than three versions for black to target.
|
* port to anyio v3 * configure strict pytest * support py3.6 and py3.9 * add all black target-versions psf/black#751 (comment) * upgrade florimondmanca/azure-pipelines-templates * upgrade black * add nocov * widen typing-extensions dep * docment 3.6 support * drop aiometer._concurrency
Summary: according psf/black#751 , set the minimum target-version of black to py36 & fix the style issues Pull Request resolved: #896 Reviewed By: vivekmig Differential Revision: D35038239 Pulled By: aobo-y fbshipit-source-id: 7dd76d35cace8aaa286eebb092f038bb0f2a120d
@JelleZijlstra and I have decided to make --target-version the minimum. It better fits user expectations and it's the only way to reasonably support #3124. We'll still need to decide how specifying a single vs multiple versions will work though. |
While implementing #3489, I came up with another need: a project may want to set a minimum version but still allow auto detecting target version when it uses newer syntaxes from later versions. A concrete example: a projects supports Python 3.9+, but it may still contain some files with pattern matching syntax (a common case is its test files, it wants to make sure it works with pattern matching, but the test only runs on Python 3.10+). Without specifying a minimum version of 3.9, it can't use parenthesized context managers (when the file doesn't contain syntaxes from 3.9+). But setting --target-version=3.9 today would make it fail to parse 3.10+ syntaxes. |
Is it safe to say that thought it has been decided and has not been implemented? My interpretation of this thread is that current commendation for py38-py311 support would be:
In the future it would be equivalent of:
|
Any updates on this thread? I would also prefer specifying only the minimum version as it doesn't require using a bleeding edge version of black just to have it recognize |
With old Black 18.9b0, I had this in
pyproject.toml
:The new Black 19.3b0 warns:
The docs say:
I see Black's own
pyproject.toml
includes:If I target just
py36
, does it treat that as a minimum?Is that the same as
['py36', 'py37', 'py38']
?It recommended to explicitly list all the targetted versions?
The text was updated successfully, but these errors were encountered: