DOC: cleanup pre-commit instructions following #2430

[neutrinoceros]

Description

I believe this was forgotten in #2430

Checklist - did you ...

• [N/A] Add a CHANGELOG entry if necessary?
• [N/A] Add / update tests if necessary?
• Add new / update outdated documentation?

[neutrinoceros]

I don't think this change warrants a mention in the changelog. I can add one if required but I'd be content with a "skip news" label.

[zsol]

Correct me if I'm wrong but if you don't specify that language_version anywhere then precommit will use whatever is on your system. If that's the case, I'd rather have it there as a suggestion as it's a reasonable default.

[neutrinoceros]

@zsol I believe this was previously discussed in #2430. Here's the key comment therein
Quote:

black's hooks already require a minimum_pre_commit_version of 2.9.2, which is after pre-commit itself dropped Python 2 support. So, pre-commit will already be running under Python 3, and in the absense of a langauge_version defined here, will reuse the same Python it is running under for the black hook.

[MarcoGorelli]

If I can give my 2 cents:

It's true that specifying Python3 is no longer necessary, as there's no longer any risk of black being run via Python2. However, black differs when run under Python3.7 or Python3.8.

Example:

$ cat t.py 
if a := 3 > 0: print(a)
$ vim .pre-commit-config.yaml 
$ cat .pre-commit-config.yaml 
repos:
- repo: https://github.com/psf/black
  rev: 21.9b0
  hooks:
  - id: black
$ pre-commit run black --all-files
black....................................................................Failed
- hook id: black
- exit code: 123

error: cannot format t.py: cannot use --safe with this file; failed to parse source file.  AST error message: invalid syntax (<unknown>, line 1)
Oh no! 💥 💔 💥
1 file failed to reformat.

$ vim .pre-commit-config.yaml 
$ cat .pre-commit-config.yaml 
repos:
- repo: https://github.com/psf/black
  rev: 21.9b0
  hooks:
  - id: black
    language_version: python3.8
$ pre-commit run black --all-files
black....................................................................Failed
- hook id: black
- files were modified by this hook

reformatted t.py
All done! ✨ 🍰 ✨
1 file reformatted.

So arguably it's still good advice to specify a language version.

Perhaps the docs could just be modified to show:

        language_version: python3.8 # Optional, to enforce a specific Python version

?

[neutrinoceros]

Wouldn't it be redundant with the target-version parameter ?

[MarcoGorelli]

Just setting -t py38 didn't work for me:

$ cat .pre-commit-config.yaml 
repos:
- repo: https://github.com/psf/black
  rev: 21.9b0
  hooks:
  - id: black
    args: ['-t', 'py38']

$ pre-commit run black -a
black....................................................................Failed
- hook id: black
- exit code: 123

error: cannot format t.py: cannot use --safe with this file; failed to parse source file.  AST error message: invalid syntax (<unknown>, line 1)
Oh no! 💥 💔 💥
1 file failed to reformat.

[neutrinoceros]

Interesting. In retrospect, I think I never realised this because the only large project I've used black via pre-commit with has 3.6 as a target, and black never supported an older version ?
edit: I was wrong, 2.7, 3.3, 3.4 and 3.5 were supported (don't know if they still are)

So the correct recommendation would actually be to merge align language-version with black's target-version ? Given the later can be an array, should we align with the oldest or with the most recent target ?

edit: following my first edit, I think the reason that target-version is an array is probably because it had to work with. target-version = ['2.7', '3.6'] for instance, but I don't think target-version = ['3.6', '3.9'] means anything more than target-version = ['3.6'], which would answer my question

[MarcoGorelli]

So the correct recommendation would actually be to merge align language-version with black's target-version ? Given the later can be an array, should we align with the oldest or with the most recent target ?

I've not looked much into target_version, but from #2383 (comment) it looks like it should be the newest target

[neutrinoceros]

Alright I updated the PR to reflect this discussion.

[zsol]

Thanks!