pyright: 15 errors to 0, by narrowing where the code already guarantees it - #139
Merged
Conversation
…es it Every one of these was code that is correct today and could not be shown to be. **pred_census_ln.py (4).** The validation raises when `stats is None` and either `prior` or `coverage` was given, and every use of `stats` sits inside one of those two conditions -- a chain no checker follows through a compound `and`. Asserted at the three use points, which also states the invariant for the next reader. The `coverage` validation is now a nested `if` rather than a compound one, which reads better anyway. **pred_wiki_origin.py (2).** The base declares VOCABFN and RACEFN as `str | None` because non-LSTM models have neither. This model sets both. Narrowing them in the subclass does not work -- a mutable ClassVar is invariant, so `str` is not assignable to `str | None`, and trying it traded two errors for two different ones. Narrowed at the call site instead. **dict_models.py (9).** pandas-stubs types `df[key]` as `Series | DataFrame`, because a column label could be duplicated; a scalar key yields a Series and a list key a DataFrame, which the stubs cannot express. Five Series call sites and one DataFrame assignment are now cast, with the reason. `census_marginal` returns a cached value the branch above always sets, so that is asserted rather than cast. No behaviour changes: 211 tests pass, ruff and format clean.
soodoku
added a commit
that referenced
this pull request
Aug 20, 2026
…es it (#139) Every one of these was code that is correct today and could not be shown to be. **pred_census_ln.py (4).** The validation raises when `stats is None` and either `prior` or `coverage` was given, and every use of `stats` sits inside one of those two conditions -- a chain no checker follows through a compound `and`. Asserted at the three use points, which also states the invariant for the next reader. The `coverage` validation is now a nested `if` rather than a compound one, which reads better anyway. **pred_wiki_origin.py (2).** The base declares VOCABFN and RACEFN as `str | None` because non-LSTM models have neither. This model sets both. Narrowing them in the subclass does not work -- a mutable ClassVar is invariant, so `str` is not assignable to `str | None`, and trying it traded two errors for two different ones. Narrowed at the call site instead. **dict_models.py (9).** pandas-stubs types `df[key]` as `Series | DataFrame`, because a column label could be duplicated; a scalar key yields a Series and a list key a DataFrame, which the stubs cannot express. Five Series call sites and one DataFrame assignment are now cast, with the reason. `census_marginal` returns a cached value the branch above always sets, so that is asserted rather than cast. No behaviour changes: 211 tests pass, ruff and format clean.
soodoku
added a commit
that referenced
this pull request
Aug 20, 2026
…es it (#139) Every one of these was code that is correct today and could not be shown to be. **pred_census_ln.py (4).** The validation raises when `stats is None` and either `prior` or `coverage` was given, and every use of `stats` sits inside one of those two conditions -- a chain no checker follows through a compound `and`. Asserted at the three use points, which also states the invariant for the next reader. The `coverage` validation is now a nested `if` rather than a compound one, which reads better anyway. **pred_wiki_origin.py (2).** The base declares VOCABFN and RACEFN as `str | None` because non-LSTM models have neither. This model sets both. Narrowing them in the subclass does not work -- a mutable ClassVar is invariant, so `str` is not assignable to `str | None`, and trying it traded two errors for two different ones. Narrowed at the call site instead. **dict_models.py (9).** pandas-stubs types `df[key]` as `Series | DataFrame`, because a column label could be duplicated; a scalar key yields a Series and a list key a DataFrame, which the stubs cannot express. Five Series call sites and one DataFrame assignment are now cast, with the reason. `census_marginal` returns a cached value the branch above always sets, so that is asserted rather than cast. No behaviour changes: 211 tests pass, ruff and format clean.
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.
Every one of these was code that is correct today and could not be shown to be. No behaviour changes.
pred_census_ln.py— 4The validation raises when
stats is Noneand eitherpriororcoveragewas given, and every use ofstatssits inside one of those two conditions — a chain no checker follows through a compoundand:Asserted at the three use points, which also states the invariant for the next reader. The
coveragevalidation is now a nestedifrather than a compound one, which reads better anyway.pred_wiki_origin.py— 2The base declares
VOCABFN/RACEFNasstr | Nonebecause non-LSTM models have neither; this model sets both.Narrowing them in the subclass does not work — a mutable
ClassVaris invariant, sostris not assignable tostr | None. I tried it and it traded two errors for two different ones, so it is narrowed at the call site instead.dict_models.py— 9pandas-stubs types
df[key]asSeries | DataFrame, because a column label could be duplicated. A scalar key yields a Series and a list key a DataFrame — which the stubs cannot express. Five Series call sites and one DataFrame assignment are cast, each with the reason.census_marginalreturns a cached value the branch above always sets, so that is asserted rather than cast.Verification
pyright 15 → 0,
ruff checkandruff format --checkclean, 211 tests pass.Combined with the earlier scoping PR, this repo goes from 130 pyright errors and 12,249 codespell findings to zero of each.