Meta: Run check-peps.py in pre-commit

[hugovk]

For #4063.

This will allow us to remove the monster regexes in pre-commit (and also check-peps from lint.yml).

We had commented it out when check-peps.py was first added in #3275 in August 2023. I seem to remember we tried to enable it during the October 2023 sprint, but couldn't find an invocation that worked on Windows and macOS/Linux.

Let's try again.

This passes locally for me on macOS:

❯ pre-commit run --all-files
Normalize mixed line endings......................................................................Passed
Sort codespell ignore list........................................................................Passed
Check for case conflicts..........................................................................Passed
Check for merge conflict markers..................................................................Passed
Check that executables have shebangs..............................................................Passed
Check that shebangs are executable................................................................Passed
Check that VCS links are permalinks...............................................................Passed
Check JSON........................................................................................Passed
Check TOML........................................................................................Passed
Check YAML........................................................................................Passed
Format with Black.................................................................................Passed
Lint with Ruff....................................................................................Passed
Format tox.ini....................................................................................Passed
Sphinx lint.......................................................................................Passed
Check RST: No single backticks....................................................................Passed
Check RST: No backticks touching text.............................................................Passed
Check RST: 2 colons after directives..............................................................Passed
Check PEPs for metadata and content enforcement...................................................Passed
PEPs must have all required headers...............................................................Passed
PEP header order must follow PEP 12...............................................................Passed
'PEP' header must be a number 1-9999..............................................................Passed
'Title' must be 1-79 characters...................................................................Passed
'Author' must be list of 'Name <email@example.com>, ...'..........................................Passed
'Sponsor' must have format 'Name <email@example.com>'.............................................Passed
'Delegate' must have format 'Name <email@example.com>'............................................Passed
'Discussions-To' must be a thread URL.............................................................Passed
'Status' must be a valid PEP status...............................................................Passed
'Type' must be a valid PEP type...................................................................Passed
'Topic' must be for a valid sub-index.............................................................Passed
'Content-Type' must be 'text/x-rst'...............................................................Passed
`Requires`/`Replaces`/`Superseded-By` must be 'NNN' PEP IDs.......................................Passed
'Created' must be a 'DD-mmm-YYYY' date............................................................Passed
'Python-Version' must be a 'X.Y[.Z]` version......................................................Passed
'Post-History' must be '`DD-mmm-YYYY <Thread URL>`__, ...'........................................Passed
'Resolution' must be a direct thread/message URL or '`DD-mmm-YYYY <URL>`__'.......................Passed
Check that PEPs aren't linked directly............................................................Passed
Check that RFCs aren't linked directly............................................................Passed

Or check-peps directly:

❯ pre-commit run --all-files check-peps
Check PEPs for metadata and content enforcement..........................Passed

This PR temporarily runs pre-commit all operating systems on the CI, and it passes, but perhaps GitHub Actions has extra aliases.

Please could someone on Windows try this?

---

📚 Documentation preview 📚: https://pep-previews--4071.org.readthedocs.build/

[willingc]

Nice!

[hugovk]

Updated branch to remove regexes from pre-commit.

Installing the venv (PR) is a bit slower. Here's clearing the pre-commit cache then installing the hooks (but not running the checks):

❯ hyperfine \
--prepare "pre-commit clean; git checkout main"        "pre-commit install-hooks  # main" \
--prepare "pre-commit clean; git checkout lint-all-os" "pre-commit install-hooks  # PR"
Benchmark 1: pre-commit install-hooks  # main
  Time (mean ± σ):     19.100 s ±  0.357 s    [User: 10.244 s, System: 3.620 s]
  Range (min … max):   18.661 s … 19.648 s    10 runs

Benchmark 2: pre-commit install-hooks  # PR
  Time (mean ± σ):     21.741 s ±  1.071 s    [User: 11.817 s, System: 4.185 s]
  Range (min … max):   20.493 s … 24.397 s    10 runs

Summary
  pre-commit install-hooks  # main ran
    1.14 ± 0.06 times faster than pre-commit install-hooks  # PR

But the regexes are slow to run. Here's clearing the cache, installing hooks and running the checks. It's slightly slower with the PR, but not by much:

❯ hyperfine \
--prepare "pre-commit clean; git checkout main"        "pre-commit run --all-files  # main" \
--prepare "pre-commit clean; git checkout lint-all-os" "pre-commit run --all-files  # PR"
Benchmark 1: pre-commit run --all-files  # main
  Time (mean ± σ):     23.344 s ±  1.101 s    [User: 19.321 s, System: 4.555 s]
  Range (min … max):   22.267 s … 25.755 s    10 runs

Benchmark 2: pre-commit run --all-files  # PR
  Time (mean ± σ):     23.743 s ±  2.272 s    [User: 18.638 s, System: 4.660 s]
  Range (min … max):   21.625 s … 28.937 s    10 runs

Summary
  pre-commit run --all-files  # main ran
    1.02 ± 0.11 times faster than pre-commit run --all-files  # PR

However, most of the time, we'll have a cache. Here's running the checks with a cache:

❯ hyperfine --warmup 1 \
--prepare "git checkout main"        "pre-commit run --all-files  # main" \
--prepare "git checkout lint-all-os" "pre-commit run --all-files  # PR"
Benchmark 1: pre-commit run --all-files  # main
  Time (mean ± σ):      5.865 s ±  0.556 s    [User: 11.285 s, System: 1.496 s]
  Range (min … max):    5.309 s …  7.075 s    10 runs

Benchmark 2: pre-commit run --all-files  # PR
  Time (mean ± σ):      2.911 s ±  0.150 s    [User: 8.989 s, System: 1.003 s]
  Range (min … max):    2.774 s …  3.274 s    10 runs

Summary
  pre-commit run --all-files  # PR ran
    2.01 ± 0.22 times faster than pre-commit run --all-files  # main

Twice as fast with the PR!

[AA-Turner]

With the trouble that it causes I'm half minded to suggest that we don't add check-peps.py to pre-commit. I fully agree with removing the regular expressions.

The reservation I have that isn't just with pre-commit's configuration is that if we recommend that PEP authors install the pre-commit hooks locally (which I think we do), it could be quite an annoying experience to write a PEP with small, frequent commits. The script is for quality control on PEP submission rather than during the drafting process (e.g. the headings checks are important for us as editors, but authors rightly focus more on the content).

That being said, the regexes are currently in pre-commit and we regularly have new PEPs that fail linting, so I'm not sure how frequently used pre-commit is in practice by said authors.

Anyway, I have tested the PR at present on Windows and can confirm it does work (slowly!).

A

[hauntsaninja]

In contributing docs, you could recommend pre-commit install -t pre-push --install-hooks. This makes pre-commit only run when you push, instead of when you commit, which makes it far more tolerable

[hugovk]

With the trouble that it causes I'm half minded to suggest that we don't add check-peps.py to pre-commit. I fully agree with removing the regular expressions.

I don't think it's much trouble?

The reservation I have that isn't just with pre-commit's configuration is that if we recommend that PEP authors install the pre-commit hooks locally (which I think we do), it could be quite an annoying experience to write a PEP with small, frequent commits. The script is for quality control on PEP submission rather than during the drafting process (e.g. the headings checks are important for us as editors, but authors rightly focus more on the content).

Small frequent commits should now be quicker -- it's 2x faster to run the script than the regexes (on macOS).

Some people really don't like pre-commit as a Git hook, and that's perfectly fine if they just run it on the CI. But at least I find it valuable to run it locally. Anyway, whether we recommend pre-commit locally is orthogonal to this PR.

That being said, the regexes are currently in pre-commit and we regularly have new PEPs that fail linting, so I'm not sure how frequently used pre-commit is in practice by said authors.

Indeed, and I think this is fine. CI is the safety net.

Anyway, I have tested the PR at present on Windows and can confirm it does work

Good to hear! 🎉

(slowly!).

Can you quantify? On macOS, when there's no cache, the cost of installing the venv and running the script is about the same as the savings of ditching the regexes. And when there is a cache, the PR is twice as fast running the script instead of regexes.

[AlexWaygood]

(slowly!).

Can you quantify? On macOS, when there's no cache, the cost of installing the venv and running the script is about the same as the savings of ditching the regexes. And when there is a cache, the PR is twice as fast running the script instead of regexes.

Creating a virtual environment is unfortunately far, far slower on Windows than it is on Unix, so this may be related to that. (uv helps a lot with this problem!)

[AA-Turner]

Yep, it's not any slower than creating a normal venv, but that is itself slow. Single digit seconds, though.