black icon indicating copy to clipboard operation
black copied to clipboard

Is --target-version the minimum or should all be explicitly listed?

Open hugovk opened this issue 5 years ago • 16 comments

With old Black 18.9b0, I had this in pyproject.toml:

[tool.black]
py36 = true

The new Black 19.3b0 warns:

--py36 is deprecated and will be removed in a future version. Use --target-version py36 instead.

The docs say:

  -t, --target-version [py27|py33|py34|py35|py36|py37|py38]
                                  Python versions that should be supported by
                                  Black's output. [default: per-file auto-
                                  detection]
  --py36                          Allow using Python 3.6-only syntax on all
                                  input files.  This will put trailing commas
                                  in function signatures and calls also after
                                  *args and **kwargs. Deprecated; use
                                  --target-version instead. [default: per-file
                                  auto-detection]

I see Black's own pyproject.toml includes:

[tool.black]
target_version = ['py36', 'py37', 'py38']

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?

hugovk avatar Mar 14 '19 19:03 hugovk

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.

JelleZijlstra avatar Mar 14 '19 21:03 JelleZijlstra

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?

ambv avatar Mar 14 '19 22:03 ambv

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 --target-version=py36 as target-version=py36 fails with confusing errors.

I think it's fine to allow supplying a range (although I'd prefer something like py27...py38 or py37... that conveys the meaning better), but the docs don't reflect that currently.

hynek avatar Mar 16 '19 20:03 hynek

Another confusion.

Let's say I support Python 3.5-3.8 so have this in pyproject.toml:

[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 .pre-commit.yaml need to look like this?

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!

hugovk avatar Nov 01 '19 19:11 hugovk

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?

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 f"{x}" will be SyntaxError and only F"{x}" will be legal. In that case, if you set target_version to 3.5-3.8, we'd still emit f"{x}" (because Black's style prefers lowercase string prefixes), but with 3.5-3.9 we'd have to emit F"{x}".

In practice that sort of scenario doesn't seem terribly likely, so I'd be OK with making target_version just a minimum requirement.

JelleZijlstra avatar Nov 01 '19 21:11 JelleZijlstra

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 :smile:

Zac-HD avatar Dec 29 '19 13:12 Zac-HD

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.

ambv avatar Mar 04 '20 22:03 ambv

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 --maximum-version argument will be the least of the problems dumped on the library ecosystem.

Zac-HD avatar Mar 05 '20 01:03 Zac-HD

My suggestion would be to read project.requires-python from pyproject.toml; you can compare using packaging.specifiers to see what the allowed versions are. cibuildwheel 1.9 does this. Since PEP 621, that is the canonical place for this information, and powerful enough to provide a subset or limit if needed.

See https://github.com/joerick/cibuildwheel/blob/58c5e56e9f271e32020b687f62cc02003602edd2/cibuildwheel/main.py#L173 and https://github.com/joerick/cibuildwheel/blob/58c5e56e9f271e32020b687f62cc02003602edd2/cibuildwheel/util.py#L77

henryiii avatar Feb 12 '21 20:02 henryiii

if you set target_version to 3.5-3.8, we'd still emit f"{x}" (because Black's style prefers lowercase string prefixes), but with 3.5-3.9 we'd have to emit F"{x}".

This doesn't apply for python 3.3 does it? Black still outputs f-strings even though they are not supported.

deathaxe avatar May 23 '21 07:05 deathaxe

This doesn't apply for python 3.3 does it

Black doesn't support Python 3.3. Also, note that you're quoting a hypothetical out of context.

JelleZijlstra avatar May 23 '21 14:05 JelleZijlstra

@hugovk

And then would .pre-commit.yaml need to look like this?

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).

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.

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"

scottclowe avatar Jun 28 '21 12:06 scottclowe

@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.

ichard26 avatar Jun 28 '22 15:06 ichard26

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.

yilei avatar Jan 20 '23 18:01 yilei

@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.

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:

target-version = ['py38', 'py39', 'py310', 'py311']

In the future it would be equivalent of:

target-version = ['py38']

itdependsnetworks avatar Aug 28 '23 17:08 itdependsnetworks

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 'py312' as a valid target version.

adamjstewart avatar Mar 13 '24 10:03 adamjstewart