From df7ddac5c0b4ae7857d263346b780e5726aa471a Mon Sep 17 00:00:00 2001 From: Michael Denyer Date: Sun, 26 Jul 2026 23:51:27 +0100 Subject: [PATCH] refactor: clear the 22 pyrefly baseline entries in validation/compare.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two root causes, both in `validation/compare.py`. `load_gemma_assoc` built a `dict[str, str | int | float]` through `_parse_assoc_field` and splatted it into `AssocResult(**row)`. The union reaches every parameter, so all 15 fields error at the one call site. Constructs `AssocResult` explicitly instead, with per-field `int()`/`float()` and an `_opt_float` helper for the six `float | None` columns. Header names already equal `AssocResult` field names, so the mapping is direct. `_parse_assoc_field`, `_ASSOC_STR_FIELDS` and `_ASSOC_INT_FIELDS` had no other callers and are gone. `compare_assoc_results` ran the same four-way test-type dispatch twice: once to compute the per-column results, once to combine them into `passed`. The second chain read `p_score_result.passed`, `p_lrt_result.passed` and `l_mle_result.passed` with no narrowing, since the assignment happened in the first chain. Merged into one dispatch, so each branch computes its columns and its verdict together. That also removes a latent `AttributeError`: the two chains ordered `is_score_test` and `is_lrt_test` differently, so a result set satisfying both predicates would compute the Score columns and then read the LRT ones. Also casts the LRT `np.all(...)` results to `bool` for `passed`. Verified equivalent rather than assumed: parsing all 19 `.assoc.txt` fixtures (90,644 rows, all seven header layouts) with the old and new parsers gives byte-identical `AssocResult` reprs, and `compare_assoc_results` returns identical results over 95 cases (identity, three perturbation scales, and a length mismatch, per fixture). Baseline regenerated on Python 3.11 / numpy 2.4.6 per #121: 174 -> 152 entries. `pyrefly check --baseline` reports 0 errors on both 3.11.13/numpy 2.4.6 and 3.13.5/numpy 2.5.1. `pytest tests/` — 2241 passed, 10 skipped, 3 xfailed. --- pyrefly-baseline.json | 264 -------------------------------- src/jamma/validation/compare.py | 110 +++++++------ 2 files changed, 59 insertions(+), 315 deletions(-) diff --git a/pyrefly-baseline.json b/pyrefly-baseline.json index d17748c..8e6214b 100644 --- a/pyrefly-baseline.json +++ b/pyrefly-baseline.json @@ -768,270 +768,6 @@ "concise_description": "`dict[Unknown, Unknown]` is not assignable to `PipelineTiming`", "severity": "error" }, - { - "line": 299, - "column": 40, - "stop_line": 299, - "stop_column": 45, - "path": "src/jamma/validation/compare.py", - "code": -2, - "name": "bad-argument-type", - "description": "Unpacked keyword argument `float | int | str` is not assignable to parameter `chr` with type `str` in function `jamma.lmm.stats.AssocResult.__init__`", - "concise_description": "Unpacked keyword argument `float | int | str` is not assignable to parameter `chr` with type `str` in function `jamma.lmm.stats.AssocResult.__init__`", - "severity": "error" - }, - { - "line": 299, - "column": 40, - "stop_line": 299, - "stop_column": 45, - "path": "src/jamma/validation/compare.py", - "code": -2, - "name": "bad-argument-type", - "description": "Unpacked keyword argument `float | int | str` is not assignable to parameter `rs` with type `str` in function `jamma.lmm.stats.AssocResult.__init__`", - "concise_description": "Unpacked keyword argument `float | int | str` is not assignable to parameter `rs` with type `str` in function `jamma.lmm.stats.AssocResult.__init__`", - "severity": "error" - }, - { - "line": 299, - "column": 40, - "stop_line": 299, - "stop_column": 45, - "path": "src/jamma/validation/compare.py", - "code": -2, - "name": "bad-argument-type", - "description": "Unpacked keyword argument `float | int | str` is not assignable to parameter `ps` with type `int` in function `jamma.lmm.stats.AssocResult.__init__`", - "concise_description": "Unpacked keyword argument `float | int | str` is not assignable to parameter `ps` with type `int` in function `jamma.lmm.stats.AssocResult.__init__`", - "severity": "error" - }, - { - "line": 299, - "column": 40, - "stop_line": 299, - "stop_column": 45, - "path": "src/jamma/validation/compare.py", - "code": -2, - "name": "bad-argument-type", - "description": "Unpacked keyword argument `float | int | str` is not assignable to parameter `n_miss` with type `int` in function `jamma.lmm.stats.AssocResult.__init__`", - "concise_description": "Unpacked keyword argument `float | int | str` is not assignable to parameter `n_miss` with type `int` in function `jamma.lmm.stats.AssocResult.__init__`", - "severity": "error" - }, - { - "line": 299, - "column": 40, - "stop_line": 299, - "stop_column": 45, - "path": "src/jamma/validation/compare.py", - "code": -2, - "name": "bad-argument-type", - "description": "Unpacked keyword argument `float | int | str` is not assignable to parameter `allele1` with type `str` in function `jamma.lmm.stats.AssocResult.__init__`", - "concise_description": "Unpacked keyword argument `float | int | str` is not assignable to parameter `allele1` with type `str` in function `jamma.lmm.stats.AssocResult.__init__`", - "severity": "error" - }, - { - "line": 299, - "column": 40, - "stop_line": 299, - "stop_column": 45, - "path": "src/jamma/validation/compare.py", - "code": -2, - "name": "bad-argument-type", - "description": "Unpacked keyword argument `float | int | str` is not assignable to parameter `allele0` with type `str` in function `jamma.lmm.stats.AssocResult.__init__`", - "concise_description": "Unpacked keyword argument `float | int | str` is not assignable to parameter `allele0` with type `str` in function `jamma.lmm.stats.AssocResult.__init__`", - "severity": "error" - }, - { - "line": 299, - "column": 40, - "stop_line": 299, - "stop_column": 45, - "path": "src/jamma/validation/compare.py", - "code": -2, - "name": "bad-argument-type", - "description": "Unpacked keyword argument `float | int | str` is not assignable to parameter `af` with type `float` in function `jamma.lmm.stats.AssocResult.__init__`", - "concise_description": "Unpacked keyword argument `float | int | str` is not assignable to parameter `af` with type `float` in function `jamma.lmm.stats.AssocResult.__init__`", - "severity": "error" - }, - { - "line": 299, - "column": 40, - "stop_line": 299, - "stop_column": 45, - "path": "src/jamma/validation/compare.py", - "code": -2, - "name": "bad-argument-type", - "description": "Unpacked keyword argument `float | int | str` is not assignable to parameter `beta` with type `float` in function `jamma.lmm.stats.AssocResult.__init__`", - "concise_description": "Unpacked keyword argument `float | int | str` is not assignable to parameter `beta` with type `float` in function `jamma.lmm.stats.AssocResult.__init__`", - "severity": "error" - }, - { - "line": 299, - "column": 40, - "stop_line": 299, - "stop_column": 45, - "path": "src/jamma/validation/compare.py", - "code": -2, - "name": "bad-argument-type", - "description": "Unpacked keyword argument `float | int | str` is not assignable to parameter `se` with type `float` in function `jamma.lmm.stats.AssocResult.__init__`", - "concise_description": "Unpacked keyword argument `float | int | str` is not assignable to parameter `se` with type `float` in function `jamma.lmm.stats.AssocResult.__init__`", - "severity": "error" - }, - { - "line": 299, - "column": 40, - "stop_line": 299, - "stop_column": 45, - "path": "src/jamma/validation/compare.py", - "code": -2, - "name": "bad-argument-type", - "description": "Unpacked keyword argument `float | int | str` is not assignable to parameter `logl_H1` with type `float | None` in function `jamma.lmm.stats.AssocResult.__init__`", - "concise_description": "Unpacked keyword argument `float | int | str` is not assignable to parameter `logl_H1` with type `float | None` in function `jamma.lmm.stats.AssocResult.__init__`", - "severity": "error" - }, - { - "line": 299, - "column": 40, - "stop_line": 299, - "stop_column": 45, - "path": "src/jamma/validation/compare.py", - "code": -2, - "name": "bad-argument-type", - "description": "Unpacked keyword argument `float | int | str` is not assignable to parameter `l_remle` with type `float | None` in function `jamma.lmm.stats.AssocResult.__init__`", - "concise_description": "Unpacked keyword argument `float | int | str` is not assignable to parameter `l_remle` with type `float | None` in function `jamma.lmm.stats.AssocResult.__init__`", - "severity": "error" - }, - { - "line": 299, - "column": 40, - "stop_line": 299, - "stop_column": 45, - "path": "src/jamma/validation/compare.py", - "code": -2, - "name": "bad-argument-type", - "description": "Unpacked keyword argument `float | int | str` is not assignable to parameter `p_wald` with type `float | None` in function `jamma.lmm.stats.AssocResult.__init__`", - "concise_description": "Unpacked keyword argument `float | int | str` is not assignable to parameter `p_wald` with type `float | None` in function `jamma.lmm.stats.AssocResult.__init__`", - "severity": "error" - }, - { - "line": 299, - "column": 40, - "stop_line": 299, - "stop_column": 45, - "path": "src/jamma/validation/compare.py", - "code": -2, - "name": "bad-argument-type", - "description": "Unpacked keyword argument `float | int | str` is not assignable to parameter `p_score` with type `float | None` in function `jamma.lmm.stats.AssocResult.__init__`", - "concise_description": "Unpacked keyword argument `float | int | str` is not assignable to parameter `p_score` with type `float | None` in function `jamma.lmm.stats.AssocResult.__init__`", - "severity": "error" - }, - { - "line": 299, - "column": 40, - "stop_line": 299, - "stop_column": 45, - "path": "src/jamma/validation/compare.py", - "code": -2, - "name": "bad-argument-type", - "description": "Unpacked keyword argument `float | int | str` is not assignable to parameter `l_mle` with type `float | None` in function `jamma.lmm.stats.AssocResult.__init__`", - "concise_description": "Unpacked keyword argument `float | int | str` is not assignable to parameter `l_mle` with type `float | None` in function `jamma.lmm.stats.AssocResult.__init__`", - "severity": "error" - }, - { - "line": 299, - "column": 40, - "stop_line": 299, - "stop_column": 45, - "path": "src/jamma/validation/compare.py", - "code": -2, - "name": "bad-argument-type", - "description": "Unpacked keyword argument `float | int | str` is not assignable to parameter `p_lrt` with type `float | None` in function `jamma.lmm.stats.AssocResult.__init__`", - "concise_description": "Unpacked keyword argument `float | int | str` is not assignable to parameter `p_lrt` with type `float | None` in function `jamma.lmm.stats.AssocResult.__init__`", - "severity": "error" - }, - { - "line": 583, - "column": 17, - "stop_line": 583, - "stop_column": 38, - "path": "src/jamma/validation/compare.py", - "code": -2, - "name": "missing-attribute", - "description": "Object of class `NoneType` has no attribute `passed`", - "concise_description": "Object of class `NoneType` has no attribute `passed`", - "severity": "error" - }, - { - "line": 584, - "column": 17, - "stop_line": 584, - "stop_column": 36, - "path": "src/jamma/validation/compare.py", - "code": -2, - "name": "missing-attribute", - "description": "Object of class `NoneType` has no attribute `passed`", - "concise_description": "Object of class `NoneType` has no attribute `passed`", - "severity": "error" - }, - { - "line": 585, - "column": 17, - "stop_line": 585, - "stop_column": 36, - "path": "src/jamma/validation/compare.py", - "code": -2, - "name": "missing-attribute", - "description": "Object of class `NoneType` has no attribute `passed`", - "concise_description": "Object of class `NoneType` has no attribute `passed`", - "severity": "error" - }, - { - "line": 595, - "column": 17, - "stop_line": 595, - "stop_column": 36, - "path": "src/jamma/validation/compare.py", - "code": -2, - "name": "missing-attribute", - "description": "Object of class `NoneType` has no attribute `passed`", - "concise_description": "Object of class `NoneType` has no attribute `passed`", - "severity": "error" - }, - { - "line": 596, - "column": 17, - "stop_line": 596, - "stop_column": 36, - "path": "src/jamma/validation/compare.py", - "code": -2, - "name": "missing-attribute", - "description": "Object of class `NoneType` has no attribute `passed`", - "concise_description": "Object of class `NoneType` has no attribute `passed`", - "severity": "error" - }, - { - "line": 604, - "column": 17, - "stop_line": 604, - "stop_column": 38, - "path": "src/jamma/validation/compare.py", - "code": -2, - "name": "missing-attribute", - "description": "Object of class `NoneType` has no attribute `passed`", - "concise_description": "Object of class `NoneType` has no attribute `passed`", - "severity": "error" - }, - { - "line": 619, - "column": 16, - "stop_line": 619, - "stop_column": 26, - "path": "src/jamma/validation/compare.py", - "code": -2, - "name": "bad-argument-type", - "description": "Argument `builtins.bool | numpy.bool | Unknown` is not assignable to parameter `passed` with type `builtins.bool` in function `AssocComparisonResult.__init__`", - "concise_description": "Argument `builtins.bool | numpy.bool | Unknown` is not assignable to parameter `passed` with type `builtins.bool` in function `AssocComparisonResult.__init__`", - "severity": "error" - }, { "line": 192, "column": 40, diff --git a/src/jamma/validation/compare.py b/src/jamma/validation/compare.py index 6b58450..d181c53 100644 --- a/src/jamma/validation/compare.py +++ b/src/jamma/validation/compare.py @@ -203,11 +203,9 @@ def load_gemma_kinship(path: Path) -> np.ndarray: return np.loadtxt(path) -# GEMMA .assoc.txt column headers are identical to AssocResult field names, -# so parsing is a generic header->field map keyed by column name rather than a -# per-format positional unpack. Only the field *type* varies by column. -_ASSOC_INT_FIELDS = frozenset({"ps", "n_miss"}) -_ASSOC_STR_FIELDS = frozenset({"chr", "rs", "allele1", "allele0"}) +# GEMMA .assoc.txt column headers are identical to AssocResult field names, so +# parsing is a header->field map keyed by column name rather than a per-format +# positional unpack. Which columns are present varies by layout, not their order. def _assoc_header_layouts() -> frozenset[tuple[str, ...]]: @@ -238,13 +236,10 @@ def _assoc_header_layouts() -> frozenset[tuple[str, ...]]: _ASSOC_HEADER_LAYOUTS = _assoc_header_layouts() -def _parse_assoc_field(name: str, raw: str) -> str | int | float: - """Cast one .assoc.txt cell to the type of its AssocResult field.""" - if name in _ASSOC_STR_FIELDS: - return raw - if name in _ASSOC_INT_FIELDS: - return int(raw) - return float(raw) +def _opt_float(row: dict[str, str], name: str) -> float | None: + """Cast an optional .assoc.txt cell, or None when its layout omits the column.""" + raw = row.get(name) + return None if raw is None else float(raw) def load_gemma_assoc(path: Path) -> list[AssocResult]: @@ -289,14 +284,27 @@ def load_gemma_assoc(path: Path) -> list[AssocResult]: for line in f: fields = line.strip().split("\t") - row = { - name: _parse_assoc_field(name, val) - for name, val in zip(cols, fields, strict=True) - } - # GEMMA LRT formats omit beta/se; AssocResult requires them. - row.setdefault("beta", float("nan")) - row.setdefault("se", float("nan")) - results.append(AssocResult(**row)) + row = dict(zip(cols, fields, strict=True)) + results.append( + AssocResult( + chr=row["chr"], + rs=row["rs"], + ps=int(row["ps"]), + n_miss=int(row["n_miss"]), + allele1=row["allele1"], + allele0=row["allele0"], + af=float(row["af"]), + # GEMMA LRT formats omit beta/se; AssocResult requires them. + beta=float(row.get("beta", "nan")), + se=float(row.get("se", "nan")), + logl_H1=_opt_float(row, "logl_H1"), + l_remle=_opt_float(row, "l_remle"), + p_wald=_opt_float(row, "p_wald"), + p_score=_opt_float(row, "p_score"), + l_mle=_opt_float(row, "l_mle"), + p_lrt=_opt_float(row, "p_lrt"), + ) + ) return results @@ -541,10 +549,15 @@ def _lambda(field: str, *, check_upper: bool) -> ComparisonResult: actual_arr, expected_arr, mask, config.lambda_rtol, config.atol, field ) + no_id_mismatch = len(mismatched) == 0 + p_score_result: ComparisonResult | None = None p_lrt_result: ComparisonResult | None = None l_mle_result: ComparisonResult | None = None + # Each branch computes the columns active for its test type and the overall + # pass from exactly those columns. The two must agree on which columns exist, + # so they share one dispatch rather than repeating the test-type conditions. if is_all_tests: pwald_result = _pvalue("p_wald", config.pvalue_rtol) p_score_result = _pvalue("p_score", config.pvalue_rtol) @@ -552,27 +565,6 @@ def _lambda(field: str, *, check_upper: bool) -> ComparisonResult: logl_result = _logl("logl_H1 skipped (not in GEMMA -lmm 4 format)") lambda_result = _lambda("l_remle", check_upper=False) l_mle_result = _lambda("l_mle", check_upper=True) - elif is_score_test: - p_score_result = _pvalue("p_score", config.pvalue_rtol) - pwald_result = _skipped_result("p_wald skipped (Score test)") - logl_result = _skipped_result("logl_H1 skipped (Score test)") - lambda_result = _skipped_result("l_remle skipped (Score test)") - elif is_lrt_test: - p_lrt_result = _pvalue("p_lrt", config.p_lrt_rtol) - l_mle_result = _lambda("l_mle", check_upper=True) - pwald_result = _skipped_result("p_wald skipped (LRT)") - logl_result = _skipped_result("logl_H1 skipped (LRT)") - lambda_result = _skipped_result("l_remle skipped (LRT)") - else: # Wald - pwald_result = _pvalue("p_wald", config.pvalue_rtol) - logl_result = _logl("logl_H1 skipped (reference missing logl_H1 column)") - lambda_result = _lambda("l_remle", check_upper=False) - - # Overall pass: every column active for the test type must pass and SNP IDs - # must line up. LRT has NaN beta/se by construction, so it verifies both - # sides are all-NaN rather than checking the (meaningless) compare result. - no_id_mismatch = len(mismatched) == 0 - if is_all_tests: all_passed = ( beta_result.passed and se_result.passed @@ -585,9 +577,30 @@ def _lambda(field: str, *, check_upper: bool) -> ComparisonResult: and l_mle_result.passed and no_id_mismatch ) + elif is_score_test: + p_score_result = _pvalue("p_score", config.pvalue_rtol) + pwald_result = _skipped_result("p_wald skipped (Score test)") + logl_result = _skipped_result("logl_H1 skipped (Score test)") + lambda_result = _skipped_result("l_remle skipped (Score test)") + all_passed = ( + beta_result.passed + and se_result.passed + and af_result.passed + and p_score_result.passed + and no_id_mismatch + ) elif is_lrt_test: - beta_all_nan = np.all(np.isnan(actual_beta)) and np.all(np.isnan(expected_beta)) - se_all_nan = np.all(np.isnan(actual_se)) and np.all(np.isnan(expected_se)) + p_lrt_result = _pvalue("p_lrt", config.p_lrt_rtol) + l_mle_result = _lambda("l_mle", check_upper=True) + pwald_result = _skipped_result("p_wald skipped (LRT)") + logl_result = _skipped_result("logl_H1 skipped (LRT)") + lambda_result = _skipped_result("l_remle skipped (LRT)") + # LRT has NaN beta/se by construction, so check both sides are all-NaN + # rather than the (meaningless) compare result. + beta_all_nan = bool( + np.all(np.isnan(actual_beta)) and np.all(np.isnan(expected_beta)) + ) + se_all_nan = bool(np.all(np.isnan(actual_se)) and np.all(np.isnan(expected_se))) all_passed = ( af_result.passed and beta_all_nan @@ -596,15 +609,10 @@ def _lambda(field: str, *, check_upper: bool) -> ComparisonResult: and l_mle_result.passed and no_id_mismatch ) - elif is_score_test: - all_passed = ( - beta_result.passed - and se_result.passed - and af_result.passed - and p_score_result.passed - and no_id_mismatch - ) else: # Wald + pwald_result = _pvalue("p_wald", config.pvalue_rtol) + logl_result = _logl("logl_H1 skipped (reference missing logl_H1 column)") + lambda_result = _lambda("l_remle", check_upper=False) all_passed = ( beta_result.passed and se_result.passed