types: annotate metrics, utils, inspection and visualisation, and start the mypy ratchet - #167
Merged
Conversation
…rt the mypy ratchet
py.typed ships in the wheel, so a user's type checker treats every unannotated
function here as Any. That is worse than shipping no type information at all:
it silently disables checking at the boundary instead of admitting there is
nothing to check. mypy has been in CI since the beginning without
disallow_untyped_defs, so nothing stopped the backlog growing.
Annotates the five subpackages that were nearly there already (datasets was
already clean; inspection 1, visualisation 2, utils 5, metrics 6) and adds a
[[tool.mypy.overrides]] block turning disallow_untyped_defs on for exactly
those. A new unannotated function in a listed subpackage now fails CI, which is
the point: the list only grows.
Verified the ratchet actually bites rather than merely being present:
printf 'def _probe(x):\n return x\n' >> philanthropy/metrics/_fairness.py
python -m mypy philanthropy
philanthropy/metrics/_fairness.py:113: error: Function is missing a type
annotation [no-untyped-def]
One annotation is a fix rather than a transcription. ensure_local_path was
declared -> str while returning its argument unchanged, and both call sites may
hand it a Path. It is now generic in that argument, which is what the docstring
already said.
Idiom follows the surrounding code: plain typing (Collection, Sequence,
Optional, Any). numpy.typing appears nowhere in this package and one PR in the
middle of a ratchet is the wrong place to introduce it.
model_selection, experimental, cli, models and preprocessing remain. Counts and
the order to do them in are in #166.
shivamlalakiya
enabled auto-merge (squash)
September 5, 2026 21:04
This was referenced Sep 5, 2026
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.
Part 1 of #166.
Why
py.typedships in the wheel, so a user's type checker treats every unannotated function in thispackage as
Any. That is worse than shipping no type information at all: it silently disableschecking at the boundary instead of admitting there is nothing to check. mypy has been in CI since
the beginning without
disallow_untyped_defs, so nothing stopped the backlog growing. An AST countputs it at 159 functions.
What changed
Annotated the five subpackages that were nearly there already:
datasets(already clean),inspection(1),visualisation(2),utils(5),metrics(6).Added the ratchet. A
[[tool.mypy.overrides]]block turnsdisallow_untyped_defs = trueon forexactly those modules. The list only ever grows, one subpackage per PR, and collapses into a
top-level flag when it covers everything. The comment in
pyproject.tomlsays that, and carries theremaining counts so the next person does not have to re-derive them.
The ratchet actually bites
A config block that looks strict but is not is worse than no block, so this was checked rather than
assumed:
One annotation is a fix, not a transcription
ensure_local_pathwas declared-> strwhile returning its argument unchanged, and both call sites(
cli.py,_grateful_patient.py) may hand it aPath. It is now generic in that argument via aTypeVar, which is what its docstring already claimed ("the unchanged path"). The docstring'sReturnstype is corrected to match.Idiom
Plain
typing(Collection,Sequence,Optional,Any), matching the rest of the package.numpy.typing.ArrayLikeappears nowhere here, and one PR in the middle of a ratchet is the wrongplace to introduce a second vocabulary. #166 says so too, so the remaining PRs stay consistent.
Verification
Full suite runs in the pre-push hook and in CI on this PR.