Fix failing stubgen tests

[hamdanal]

This PR fixes the failing test on master https://github.com/python/mypy/actions/runs/7513432949/job/20455178894.
/cc @JelleZijlstra

[JelleZijlstra]

Thanks for the quick fix!

[hamdanal]

I think this is the cause the test didn't run on the PEP-604 PR

[hamdanal]

black doesn't agree with stubgen on number of empty lines between functions with docstring in stubs. I skipped linting these files because they are automatically generated.

[AlexWaygood]

I'm not sure we want to exclude all pre-commit hooks on the test-data/ directory, though -- things like trailing-whitespace and end-of-file-fixer are pretty useful there.

It looks like this directory is already excluded from black in pyproject.toml, which should be used as a config file even when black is invoked via pre-commit; I'm slightly confused about why this is necessary:

mypy/pyproject.toml

Lines 22 to 26 in e28925d

	force-exclude = '''
	^/mypy/typeshed|
	^/mypyc/test-data|
	^/test-data
	'''

[hamdanal]

I have no idea why it ran on these files but:

$ git commit -m "Fix failing stubtest tests"
[INFO] Initializing environment for https://github.com/astral-sh/ruff-pre-commit.
[INFO] Installing environment for https://github.com/astral-sh/ruff-pre-commit.
[INFO] Once installed this environment will be reused.
[INFO] This may take a few minutes...
trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
black....................................................................Failed
- hook id: black
- files were modified by this hook

reformatted test-data/pybind11_fixtures/expected_stubs_with_docs/pybind11_fixtures/__init__.pyi

(I know the message says stubtest :D)

But aside, should any formatter run on these files? The problem is that they are generated by a script that will diff them and fail if any change is found, including any whitespace change.

[AlexWaygood]

they are generated by a script that will diff them and fail if any change is found, including any whitespace change.

I'm not sure that's necessarily true — #15237 made a bunch of whitespace changes to test-case files, and didn't cause any tests to fail (admittedly, it was mostly just adding newlines at the ends of files)

[hamdanal]

I removed the added exclude, and ran pre-commit run black -a, it did not reformat the file.
I then ran pre-commit run black --files test-data/pybind11_fixtures/e xpected_stubs_with_docs/pybind11_fixtures/__init__.pyi and it did reformat it.

Without pre-commit:

$ black test-data/
No Python files are present to be formatted. Nothing to do 😴

$ black test-data/pybind11_fixtures/expected_stubs_with_docs/pybind11_fixtures/__init__.pyi
reformatted test-data/pybind11_fixtures/expected_stubs_with_docs/pybind11_fixtures/__init__.pyi

All done! ✨ 🍰 ✨
1 file reformatted.

$ # clean up then run
$ black test-data/pybind11_fixtures/expected_stubs_with_docs/pybind11_fixtures/
reformatted test-data/pybind11_fixtures/expected_stubs_with_docs/pybind11_fixtures/__init__.pyi
reformatted test-data/pybind11_fixtures/expected_stubs_with_docs/pybind11_fixtures/demo.pyi

All done! ✨ 🍰 ✨
2 files reformatted.

I am not sure what is going on TBH but looks like black is not forcibly excluding these files in all cases.

[AlexWaygood]

oh, I expect that's because the --files pre-commit flag means that pre-commit passes those files directly to black as a CLI argument, and CLI arguments override the pyproject.toml config for black

[AlexWaygood]

maybe add it as a hook-specific exclude entry in .pre-commit-config.yaml?

[hamdanal]

Yes but even in this case it should not reformat these files. --force-exclude help says

  --force-exclude TEXT            Like --exclude, but files and directories
                                  matching this regex will be excluded even
                                  when they are passed explicitly as
                                  arguments. This is useful when invoking
                                  Black programmatically on changed files,
                                  such as in a pre-commit hook or editor
                                  plugin.

Also you can see above that even when invoking black directly without pre-commit, it is still formatting these excluded files.

[JelleZijlstra]

% black -vv  test-data/pybind11_fixtures/expected_stubs_with_docs/pybind11_fixtures 
Identified `/Users/jelle/py/mypy/test-data/pybind11_fixtures` as project root containing a pyproject.toml.
Found input source directory: "/Users/jelle/py/mypy/test-data/pybind11_fixtures/expected_stubs_with_docs/pybind11_fixtures"
reformatted /Users/jelle/py/mypy/test-data/pybind11_fixtures/expected_stubs_with_docs/pybind11_fixtures/__init__.pyi
reformatted /Users/jelle/py/mypy/test-data/pybind11_fixtures/expected_stubs_with_docs/pybind11_fixtures/demo.pyi

I believe it's because of the pyproject.toml in the subdirectory. We have an open issue on Black complaining about that; it asks that Black ignore pyproject.toml files that don't contain a Black config and keep looking further up the tree.

[AlexWaygood]

Thanks, sorry for the nitpicks! :)

No idea why black was still formatting those files regardless, but luckily we have a black maintainer subscribed to notifications on this PR who might be able to chime in ;)

[github-actions]

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅