fix: make check_copyright_year run on Python 3.9 - #1002
Conversation
Line 21 annotates `-> int | None`. PEP 604 unions in an annotation are
evaluated at import time before 3.10, so on 3.9 the module raises TypeError
before it checks anything:
def git_last_modified_year(path: Path) -> int | None:
TypeError: unsupported operand type(s) for |: 'type' and 'NoneType'
`from __future__ import annotations` defers annotation evaluation, which is
what the other local hook scripts already do (check_lfs_pointers.py:6).
This has been silent since the script landed, for two compounding reasons:
the workflow sets SKIP: check-copyright-year, so CI never runs it, and anyone
whose python3 is 3.10 or newer sees it pass. Only a 3.9 contributor hits it,
and the hook fails open from their point of view -- it never reports a stale
year because it never starts.
Tested: python3.9.6 and python3.12.13, plain and --fix, plus `pre-commit run`
over the changed file. Before the change 3.9 raises and 3.12 passes; after it,
both pass.
Note for whoever merges: with the hook running again, `--all-files` now wants
to update the year on ~101 files that drifted while it was dead. That backlog
is deliberately not in this commit -- it is a separate, reviewable change.
Signed-off-by: Ivan Yang <yifanyang@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review. 📝 WalkthroughWalkthroughThe change adds Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: ⚪ Minimal · up to This localized change restores the copyright check on Python 3.9 without altering its intended behavior; no actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
Isn't Python 3.9 EOL already? Just curious what triggered the complaint on Python 3.9? |
|
hook runs PATH's |
2 participants
scripts/check_copyright_year.py:21 annotates
-> int | None. PEP 604 unions in annotations are evaluated at import time before Python 3.10, so on 3.9 the hook raises before it checks anything:Fix: one line —
from __future__ import annotations, matching whatcheck_lfs_pointers.py:6already does.Why this went unnoticed since February. Two things compound:
.github/workflows/pre-commit.yaml:23setsSKIP: check-copyright-year, so CI never runs it; and anyone on Python 3.10+ sees it pass. Only a 3.9 contributor hits it — and it fails open for them: it never reports a stale year because it never starts.Tested on 3.9.6 and 3.12.13, plain and
--fix, pluspre-commit runover the changed file. Before: 3.9 raises, 3.12 passes. After: both pass.One thing to know before merging. With the hook alive again,
pre-commit run --all-fileswants to update the year on ~101 files that drifted while it was dead — headers that stopped at 2025 while git says the file was touched in 2026. That backlog is deliberately not in this PR: it is a bulk, legal-adjacent edit that deserves its own review. It also self-heals, since the next commit touching any of those files fixes it.Summary by CodeRabbit