Repository navigation
mypy over core/ reaches zero; the ratchet becomes a tripwire - #123
Merged
Merged
Conversation
`core/coords`, `core/eval`, `core/fingerprinting` and `core/slam` held the
last 125 of the 413 mypy errors measured over `core/` under the pinned
2.3.1, and this takes all four to zero. 408 under mypy 1.19 at the start,
413 when the pin moved, four waves, nothing left.
So `tests/test_mypy_debt_only_shrinks.py` stops being a ratchet. Its own
instruction was to delete each code's entry at zero, because an absent code
already failed the `appeared` check; carried to its conclusion, deleting the
last entry leaves "mypy reports nothing", which is what it asserts now, with
the mypy output printed on failure so the red names the lines. The
stale-baseline companion test is deleted rather than kept trivially green:
with BASELINE gone it iterated an empty dict and could not fail, and a test
that cannot fail reads as coverage from every angle except the one that
decides whether it means anything. Verified by mutation, not by reading -- a
temporary unannotated def in `se2.py` turns it red naming
`core\slam\se2.py:427: Function is missing a type annotation`.
Type-only throughout, and measured. The 12 ch2 + ch5 + ch7 examples that
import these four packages produce 1272 lines of stdout between them;
captured before and after with `IPIN_FIGS_DIR` at a scratch directory, every
printed character is identical, all twelve exit 0. Two identical runs of the
unmodified tree were diffed first to establish that 1272 is a clean floor,
and the first attempt was not: the fingerprinting tables carry a `Time (ms)`
column that is wall-clock per query and moves between runs while every
accuracy figure beside it holds still. Normalising that column is what made
the floor clean; without it the noise would have been indistinguishable from
a finding.
Four things are worth knowing.
**The third `wrap_angle` was here, and this time `float()` was right.** Waves
B and D each found a wrapping helper declared `float -> float` that the suite
disproved, so `core.slam.se2.wrap_angle` was the obvious third. It is not:
every call site in the repository passes a scalar -- two in `factors.py` on
`diff[2]` and `residual[2]`, two inside `se2.py` itself, and
`test_wrap_array_of_angles` loops elementwise on purpose -- and the array
form is `core.utils.wrap_angle`, already annotated `float | np.ndarray`. So
`return float(...)` stands, and the proof is that `float()` raises on any
array: all four ch7 examples would have crashed and instead print identically.
**A test settled the one judgement call, and it settled it against my first
answer.** `fit_classifier`'s `zone_type` is `Literal["rp"]` while the body
dispatches on "grid" and "cluster" to raise NotImplementedError, so mypy
calls those branches dead. Widening the Literal to admit all three clears the
error -- and fails
`test_fit_classifier_signature_advertises_only_supported_zone_type`, which
asserts `get_args(...) == ("rp",)`, while
`test_legacy_experimental_zone_types_raise_clear_errors` pins the branches.
Both halves are deliberate and tested, so this is the one place a
`# type: ignore[unreachable]` is honest, and it carries a comment naming both
tests. `warn_unused_ignores` is on, so the ignore is checked too.
**`np.isscalar` IS a TypeGuard, which is worse than Wave D's finding that it
is not.** numpy declares it `TypeGuard[generic | complex | str | bytes |
memoryview]`, so it does not intersect with the declared type, it replaces
it: widening two `preprocess.py` parameters made `range_val` join to a union
containing `str`, and `ndarray / str` has no overload. The widening was
reverted to the two parameters that actually needed it -- `ref_mean` and
`ref_std`, which the body assigns a float into. `ref_min`/`ref_max` carry the
same latent narrowness and produce no error, so they are left for whoever
needs them.
**Nothing here can move a rendered figure, which had to be checked rather
than argued.** `core/eval/plots.py` is annotations, `cast` and three
`assert ax is not None` in the `owns_figure` idiom, where mypy cannot narrow
through a bool. Regenerating every ch2 and ch5 figure in-repo leaves all of
them byte-identical but one, and that one is not this change: with `core/`
stashed the same 3180-line whitespace diff appears in
`comparison_all_methods.svg` from a pristine tree. Pre-existing drift,
reported separately.
The lint baseline moves in one place and downwards: UP045 137 -> 133,
because `float | np.ndarray | None` is the PEP 604 spelling of the two
widened parameters in each of two functions. Every other ruff rule is
unchanged -- new annotations are PEP 585/604 and `Iterable` comes from
`collections.abc`.
6 of 7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Wave E, the last one.
core/coords,core/eval,core/fingerprintingandcore/slamheld the remaining 125 errors;python -m mypy corenow reportsSuccess: no issues found in 76 source files.core/fingerprintingcore/slamcore/evalcore/coordsThe journey: 408 under mypy 1.19, re-measured at 413 when the pin moved to
2.3.1, then B (413→331), C (331→232), D (232→125), E (125→0).
What the guard asserts now
tests/test_mypy_debt_only_shrinks.pykept its path and changed meaning.BASELINEis gone, and with it the per-code budget; the assertion is that mypyreports nothing at all, and it prints the mypy output on failure so the red
names the lines rather than a number. The stale-baseline companion test is
deleted rather than kept trivially green — at zero it iterated an empty dict and
could not fail. The pinned-version self-check is unchanged.
Verified by mutation, not by reading: a temporary unannotated
definse2.pyturns it red with
core\slam\se2.py:427: error: Function is missing a type annotation [no-untyped-def],then was restored byte-identically.
The numpy-stub sensitivity is written into the docstring, because at zero it
bites harder: a numpy bump can create new errors, which now fail loudly instead
of moving a baseline.
Behaviour
Type-only. The 12 ch2 + ch5 + ch7 examples importing these packages produce
1272 lines of stdout, byte-identical before and after, all twelve rc=0. Two
identical runs of the unmodified tree established the floor first — and the
first attempt at that floor was dirty, because the fingerprinting tables carry a
wall-clock
Time (ms)column that moves while every accuracy figure beside itholds still.
Figure contract: regenerating every ch2 and ch5 figure in-repo leaves all
byte-identical but
comparison_all_methods.svg, and that one is not thischange — with
core/stashed, a pristine tree produces the same 3180-linewhitespace-only diff. Pre-existing drift, reported separately.
Three things worth reading
wrap_anglewas not the trap. Waves B and D each found awrapping helper whose
float -> floatthe suite disproved.core.slam.se2.wrap_anglelooks identical and is genuinely scalar: every callsite passes a scalar,
test_wrap_array_of_anglesloops elementwise on purpose,and the array form is
core.utils.wrap_angle.float()raises on arrays, sothe four ch7 examples printing identically is the proof.
fit_classifier'szone_type: Literal["rp"]makes its "grid"/"cluster"branches unreachable. Widening the Literal clears mypy and fails
test_fit_classifier_signature_advertises_only_supported_zone_type; thebranches are pinned by
test_legacy_experimental_zone_types_raise_clear_errors. Both halves aredeliberate, so this is the one honest
# type: ignore[unreachable], carryinga comment naming both tests.
np.isscalarIS aTypeGuard, which is worse than Wave D's finding thatit is not: it replaces the declared type rather than intersecting with it.
Widening two
preprocess.pyparameters maderange_valjoin to a unioncontaining
str. Scoped back to the two parameters that needed it.Verification
python -m mypy core→Success: no issues found in 76 source filestests/test_mypy_debt_only_shrinks.py tests/test_black_is_actually_run.py tests/test_doctests_that_pass_keep_passing.py tests/test_lint_debt_only_shrinks.py→ 30 passedtests/core/fingerprinting tests/core/eval tests/core/slam tests/ch5_fingerprinting→ 647 passed, 6 skippedtests/ch7_slam→ 53 passedtests/test_repo_conventions.py→ 481 passedtests/test_datasets_reproduce_from_their_recipe.py→ 43 passed (the behaviour proof forshadowing.py)tests/test_every_figure_has_a_demo_behind_it.py→ 200 passed, 8 skippedLint baseline moves once and downwards: UP045 137 → 133, because
float | np.ndarray | Noneis the PEP 604 spelling of the widened parameters.Every other ruff rule unchanged.
Test plan
# type: ignore[unreachable]inclassification.pyreads as a decision rather than a silencing🤖 Generated with Claude Code