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-filesNormalize mixed line endings......................................................................PassedSort codespell ignore list........................................................................PassedCheck for case conflicts..........................................................................PassedCheck for merge conflict markers..................................................................PassedCheck that executables have shebangs..............................................................PassedCheck that shebangs are executable................................................................PassedCheck that VCS links are permalinks...............................................................PassedCheck JSON........................................................................................PassedCheck TOML........................................................................................PassedCheck YAML........................................................................................PassedFormat with Black.................................................................................PassedLint with Ruff....................................................................................PassedFormat tox.ini....................................................................................PassedSphinx lint.......................................................................................PassedCheck RST: No single backticks....................................................................PassedCheck RST: No backticks touching text.............................................................PassedCheck RST: 2 colons after directives..............................................................PassedCheck PEPs for metadata and content enforcement...................................................PassedPEPs must have all required headers...............................................................PassedPEP 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>`__'.......................PassedCheck that PEPs aren't linked directly............................................................PassedCheck that RFCs aren't linked directly............................................................Passed

Or check-peps directly:

❯ pre-commit run --all-files check-pepsCheck 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 runsBenchmark 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 runsSummary 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 runsBenchmark 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 runsSummary 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 runsBenchmark 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 runsSummary 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.