refactor: tighten five annotations that were narrower than the values - #129
Merged
Conversation
Clears 7 pyrefly baseline entries across five src modules (77 -> 70). Each is
a declaration that did not describe what the code actually passes or holds.
`core/progress.progress_iterator(iterable: Iterator)` only ever does
`for i, item in enumerate(iterable)`, so `Iterable` is what it requires;
`Iterator` rejected the `range` that `io/plink.py` passes it. Widening a
parameter cannot break a caller.
`io/plink.py` also fed `total=` an `int | ndarray`, because `bed.sid_count`
carries that type through `n_chunks`. It is an int at runtime, so it says so.
`lmm/likelihood._frozen(data: list[int])` is called with tuples on two of its
five call sites. It passes `data` straight to `np.array`, which takes any
sequence, so the parameter is `Sequence[int]`.
`lmm/loco.py` annotated `snps_global_mask: np.ndarray | None` on the assignment
*inside* the branch that builds it, which made the declared type a union at the
point of the very next item assignment. Declared before the branch instead, with
the array built through a local.
`lmm/special.chi2_sf_batch` called `.astype` on the result of a
`np.frompyfunc` ufunc, declared as returning a scalar. `np.asarray(..., dtype=)`
is the same cast and types correctly.
`gwas.GWASResult.timing = field(default_factory=dict)` infers
`dict[Unknown, Unknown]` rather than `GWASTiming`; the TypedDict is callable and
returns the same `{}`. Same fix as `PipelineResult.timing` in #123.
No behaviour change, and checked rather than argued, because `special.py` feeds
p-value computation and `loco.py` gates SNP selection:
- `chi2_sf_batch` is bit-identical across 13 edge cases (NaN, negative, zero,
1e-300, 1e-12, 700, 1e6, inf) against the previous expression evaluated
side by side.
- All four LMM modes on both backends produce byte-identical .assoc.txt.
- The LOCO path is identical: same .assoc.txt, pve/pve_se, per-chromosome
kinship sums and stats cache.
`pytest tests/` -- 2243 passed, 10 skipped, 3 xfailed.
Baseline regenerated on Python 3.11 / numpy 2.4.6 per #121; 0 errors on both
3.11.13/2.4.6 and 3.13.5/2.5.1.
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.
Continues the pyrefly burn-down. 77 → 70 entries. Seven entries across five
srcmodules, each a declaration that did not describe what the code actually passes or holds.
core/progress.progress_iterator(iterable: Iterator)Iterableio/plink.pytotal=getsint | ndarrayint(bed.sid_count)lmm/likelihood._frozen(data: list[int])called with tuplesSequence[int]lmm/loco.pyunion annotation inside the branchlmm/special.chi2_sf_batch.astypeon afrompyfuncresultnp.asarray(..., dtype=)gwas.GWASResult.timing = field(default_factory=dict)default_factory=GWASTimingA few worth expanding:
progress_iteratoronly ever doesfor i, item in enumerate(iterable), soIterableis what it actually requires.
Iteratorrejected therangethatio/plink.pypasses it.Widening a parameter cannot break a caller.
loco.pyannotatedsnps_global_mask: np.ndarray | Noneon the assignment inside thebranch that builds it, which made the declared type a union at the point of the very next
item assignment. Declared before the branch instead.
_frozenpassesdatastraight tonp.array, which takes any sequence; two of its fivecall sites pass tuples.
GWASResult.timingis the same fix asPipelineResult.timingin refactor: declare real types in pipeline.py and pipeline_config.py #123.Verification
Two of these sit on numerically sensitive paths —
special.pyfeeds p-value computation andloco.pygates SNP selection — so this was checked, not argued.chi2_sf_batchis bit-identical across 13 edge cases (NaN, negative, zero, 1e-300, 1e-12,0.5, 1, 2, 10, 100, 700, 1e6, inf), comparing against the previous expression evaluated side
by side in the same process:
.assoc.txt.assoc.txt, pve/pve_se, per-chromosome kinship sums, stats cachepytest tests/ruff check/ruff format --checkpyrefly check --baselineon 3.11.13/numpy 2.4.6 and 3.13.5/numpy 2.5.1What's left in
src/Seven entries, deliberately not in this PR because they are not one-line annotations:
cli.py:549,gwas.py:189—backend: strreachingPipelineConfig'sLiteral. The CLIalready constrains it with
click.Choice, so the type can simply say so.cli.py:339—-gk/-lmmare validated as mutually exclusive and one-required ~70 linesabove the dispatch, so
lmmis not narrowed at the call.lmm/runner_numpy_streaming.py×4 —RunnerTimingmisuse (.clear()anddict(...)on a TypedDict) plus a_LazySnpMetaassigned to alist | Noneparameter,currently held together by an
assertin production code.Next PR.