From 9c381978cccbfbe7a8f050dabbf4040c7802f1f0 Mon Sep 17 00:00:00 2001 From: SkyeAv Date: Fri, 14 Aug 2026 13:54:42 -0700 Subject: [PATCH] fix(lib): emit p-value columns in scientific notation again The KGX-compliance rework (#71) made format_numeric() emit p_value / adjusted_p_value as real JSON numbers because numeric_slot_kind reports the float-typed Biolink slots -- silently retiring the {:.4e} branch for exactly the columns it existed for, so edges shipped shortest-repr numbers ("p_value":0.0001) instead of the controlled notation the tutorial documents ("p_value":"1.0000e-03"). P-value-like columns are now always formatted as scientific-notation strings ahead of the numeric-slot short-circuit. Biolink validation runs in Pydantic's lax mode, which coerces the numeric string back, so validate-kgx stays green (pinned by a new record in the validate_kgx test). Non-p-value numeric columns keep the model-typed behavior: a real JSON number once a future biolink-model types the slot, {:.4g} strings until then. --- CHANGELOG.md | 5 +++++ docs/tutorial.md | 6 +++--- src/tablassert/biolink.py | 12 ++++++----- src/tablassert/lib.py | 43 +++++++++++++++++++++++---------------- tests/test_biolink.py | 10 ++++++--- tests/test_e2e_smoke.py | 13 ++++++------ tests/test_lib.py | 36 ++++++++++++++++++++------------ 7 files changed, 78 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 28adc15..3bc9cd8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ All notable changes to this project are documented in this file. +## Unreleased + +### Fixed +- **P-value columns are emitted in scientific notation again.** The KGX-compliance rework (#71) made `format_numeric()` emit `p_value` / `adjusted_p_value` as real JSON numbers because the `numeric_slot_kind` lookup reports the `float`-typed Biolink slots — which silently retired the `{:.4e}` scientific-notation branch for exactly the columns it existed for, so edges shipped shortest-repr numbers (`"p_value":0.0001`, `"p_value":0.05`) instead of the controlled notation the tutorial documents (`"p_value":"1.0000e-03"`). P-value-like columns are now always formatted as scientific-notation strings regardless of the slot's model type: Biolink validation here and downstream runs in Pydantic's lax mode, which coerces the numeric string back, so `validate-kgx` stays green (verified against the pinned `biolink-model` classes). Non-p-value numeric columns keep the model-typed behavior: a real JSON number once a future biolink model types the slot (`biolink/biolink-model#1770` / `#1774`), controlled `{:.4g}` strings until then. + ## 12.0.0 - 2026-08-14 ### Breaking Changes diff --git a/docs/tutorial.md b/docs/tutorial.md index ff7106f..4b05153 100644 --- a/docs/tutorial.md +++ b/docs/tutorial.md @@ -172,10 +172,10 @@ Example output: head -n 2 tutorial-output/TUTORIAL_KG_1.0.0.edges.ndjson ``` -Example output (numeric annotation columns are emitted as controlled-notation strings — p-values in scientific notation): +Example output (p-values are emitted as controlled scientific-notation strings; the study size rides the inlined supporting study because no association class in biolink-model 4.4.3 declares that slot): ```json -{"id":"2cfea591-0f8f-33af-a7df-03da531d3359","subject":"HGNC:11998","predicate":"biolink:associated_with","object":"MONDO:0008903","p_value":"1.0000e-03","supporting_study_size":"450"} -{"id":"7b1c9d02-5e8a-4f3b-9c1d-2a6e8f0b4d7c","subject":"HGNC:1100","predicate":"biolink:associated_with","object":"MONDO:0005041","p_value":"1.0000e-04","supporting_study_size":"1200"} +{"id":"2cfea591-0f8f-33af-a7df-03da531d3359","subject":"HGNC:11998","predicate":"biolink:associated_with","object":"MONDO:0008903","p_value":"1.0000e-03","publications":["PMID:12345678"]} +{"id":"7b1c9d02-5e8a-4f3b-9c1d-2a6e8f0b4d7c","subject":"HGNC:1100","predicate":"biolink:associated_with","object":"MONDO:0005041","p_value":"1.0000e-04","publications":["PMID:12345678"]} ``` **RIG file:** diff --git a/src/tablassert/biolink.py b/src/tablassert/biolink.py index c016069..7bc3936 100644 --- a/src/tablassert/biolink.py +++ b/src/tablassert/biolink.py @@ -695,11 +695,13 @@ def _scalar_types(annotation: Any) -> set[type]: def numeric_slot_kind(field: str) -> str | None: """Return ``"int"`` / ``"float"`` when a Biolink association slot has a numeric range. - Tablassert stringifies its numeric annotation columns for notation control, but - Biolink types ``p_value`` and ``adjusted_p_value`` as ``float`` and (with - ``biolink/biolink-model#1770``) ``supporting_study_size`` as ``int``. Those must be - emitted as real JSON numbers. Derived from the installed model so the answer - tracks whatever version is pinned. + Tablassert stringifies its numeric annotation columns for notation control (p-value + columns are ALWAYS scientific-notation strings, which Pydantic's lax validation + coerces back for the ``float``-typed ``p_value`` / ``adjusted_p_value`` slots), but + a non-p-value column that the installed model types ``int`` (``supporting_study_size`` + once ``biolink/biolink-model#1770`` lands) or ``float`` must be emitted as a real + JSON number. Derived from the installed model so the answer tracks whatever version + is pinned. Args: field: Edge column name. diff --git a/src/tablassert/lib.py b/src/tablassert/lib.py index 63f6fe3..188d1bb 100644 --- a/src/tablassert/lib.py +++ b/src/tablassert/lib.py @@ -592,15 +592,19 @@ def clean_numeric(lf: pl.LazyFrame) -> pl.LazyFrame: def format_numeric(lf: pl.LazyFrame) -> pl.LazyFrame: """Normalize numeric annotation columns for output. - Columns that map to a numeric Biolink slot are emitted as real JSON numbers: - ``p_value`` and ``adjusted_p_value`` are typed ``float`` in the model (and - ``supporting_study_size`` ``integer`` once ``biolink/biolink-model#1770`` lands), - so writing ``"6.5200e-06"`` produces a file that strict consumers reject even - though Pydantic's lax mode happens to coerce it. - - Columns with no numeric Biolink slot keep the controlled string notation -- - p-value-like names use scientific (``{:.4e}``), others decimal general - (``{:.4g}``) -- because they end up in human-readable text (the inlined + P-value columns (any name containing ``p_value``) are always emitted as + controlled scientific-notation strings (``{:.4e}``, e.g. ``"1.0000e-03"``): + notation is part of the output contract (see the tutorial's edge example), and + shortest-repr JSON numbers would render the same values as ``0.0001`` / ``0.05``. + Biolink types ``p_value`` / ``adjusted_p_value`` as ``float``, but the validation + here and downstream runs in Pydantic's lax mode, which coerces the numeric string + back -- so the notation control costs no KGX validity. + + Remaining numeric columns (``effect_size`` / ``supporting_study_size``) that a + future biolink model types ``int`` / ``float`` (e.g. once + ``biolink/biolink-model#1770`` / ``#1774`` land) are emitted as real JSON numbers; + today's untyped ones keep the controlled decimal general string notation + (``{:.4g}``) because they end up in human-readable text (the inlined ``StudyResult`` description or ``supporting_text``). Null values stay null. Args: @@ -617,15 +621,20 @@ def format_numeric(lf: pl.LazyFrame) -> pl.LazyFrame: # Collection point: batch formatting for notation control. df: pl.DataFrame = lf.collect() for c in numeric_columns(df.columns): - kind: str | None = numeric_slot_kind(c) - if kind == "float": - df = df.with_columns(pl.col(c).cast(pl.Float64, strict=False).alias(c)) - continue - if kind == "int": - df = df.with_columns(pl.col(c).cast(pl.Float64, strict=False).round().cast(pl.Int64, strict=False).alias(c)) - continue df = df.with_columns(pl.col(c).cast(pl.Float64, strict=False).alias(c)) - fmt: str = "{:.4e}" if "p_value" in c.lower() else "{:.4g}" + if "p_value" in c.lower(): + # Scientific notation is the p-value output contract; the branch ordering is + # the guard, so the numeric-slot short-circuits below can never fire for + # these columns even though the model types them ``float``. + fmt: str = "{:.4e}" + else: + kind: str | None = numeric_slot_kind(c) + if kind == "float": + continue + if kind == "int": + df = df.with_columns(pl.col(c).round().cast(pl.Int64, strict=False).alias(c)) + continue + fmt = "{:.4g}" formatted: list[str | None] = [None if v is None else fmt.format(v) for v in df[c].to_list()] df = df.with_columns(pl.Series(c, formatted)) return df.lazy() diff --git a/tests/test_biolink.py b/tests/test_biolink.py index 7865dce..1b4d895 100644 --- a/tests/test_biolink.py +++ b/tests/test_biolink.py @@ -467,11 +467,15 @@ def test_validate_kgx_separates_pending_extras_from_real_failures(tmp_path: Path # A real defect: p_value is typed float, so a non-numeric string can never validate. + json.dumps({**base, "id": "e2", "p_value": "not-a-number"}) + "\n" + # The output contract: format_numeric emits p-values as scientific-notation strings; + # lax validation coerces them back to the float slot, so this record is fully valid. + + json.dumps({**base, "id": "e3", "p_value": "1.0000e-03"}) + + "\n" ) report: dict[str, Any] = validate_kgx(nodes, edges) - assert report["edges"]["total"] == 2 - assert report["edges"]["valid"] == 0 # strict: both fail - assert report["edges"]["valid_excluding_pending"] == 1 # the effect_size edge is forgiven + assert report["edges"]["total"] == 3 + assert report["edges"]["valid"] == 1 # strict: only the scientific-notation p_value edge passes + assert report["edges"]["valid_excluding_pending"] == 2 # the effect_size edge is forgiven assert report["ok"] is False assert report["ok_excluding_pending"] is False # the real defect still fails assert "effect_size: extra_forbidden" in report["edges"]["problems"] diff --git a/tests/test_e2e_smoke.py b/tests/test_e2e_smoke.py index 7fc0818..dfbbc7f 100644 --- a/tests/test_e2e_smoke.py +++ b/tests/test_e2e_smoke.py @@ -196,8 +196,9 @@ def test_build_pipeline_coerces_statistical_annotations(tmp_path: Path, monkeypa Declares annotations with non-canonical source spellings (``p value``, ``sample size``, ``odds ratio``, ``effect type``) and asserts the emitted KGX edge carries the coerced - canonical fields flat on the edge (``p_value`` as a JSON number, ``effect_size`` in - controlled notation, ``effect_type`` with the alias mapped to the ``EffectTypes`` enum) + canonical fields flat on the edge (``p_value`` in controlled scientific-notation + string form, ``effect_size`` in controlled decimal notation, ``effect_type`` with the + alias mapped to the ``EffectTypes`` enum) and routes the auto-derived ``statistical_significance_qualifier`` plus ``supporting_study_size`` into the inlined Study. Proves the whole coercion pipeline (``coerce_pvalue_columns`` / ``coerce_study_size_columns`` / ``coerce_effect_size_columns`` @@ -252,11 +253,11 @@ def test_build_pipeline_coerces_statistical_annotations(tmp_path: Path, monkeypa edge: dict[str, Any] = edges[0] # Raw annotation names normalized to canonical Biolink fields flat on the edge. - # p_value is a numeric Biolink float slot (emitted as a real JSON number), while - # effect_size has no numeric slot and keeps controlled {:.4g} string notation. - assert isinstance(edge["p_value"], float) + # p_value keeps the controlled {:.4e} scientific-notation string contract (Biolink's + # float typing is satisfied by lax coercion), while effect_size keeps {:.4g} notation. + assert isinstance(edge["p_value"], str) assert isinstance(edge["effect_size"], str) - assert float(edge["p_value"]) == 0.01 + assert edge["p_value"] == "1.0000e-02" assert edge["effect_size"] == "0.85" assert edge["effect_type"] == "spearmans_rho" # "Spearman" alias mapped to the EffectTypes enum assert "sample size" not in edge diff --git a/tests/test_lib.py b/tests/test_lib.py index 9aa40a5..6735a34 100644 --- a/tests/test_lib.py +++ b/tests/test_lib.py @@ -1079,18 +1079,20 @@ def test_clean_numeric_idempotent_on_float64() -> None: assert twice.schema["p_value"] == pl.Float64 -def test_format_numeric_emits_p_values_as_numbers() -> None: - """P-value columns are emitted as real JSON numbers, not formatted strings. +def test_format_numeric_emits_p_values_as_scientific_strings() -> None: + """P-value columns are emitted as controlled scientific-notation strings. - Biolink types ``p_value`` / ``adjusted_p_value`` as ``float``; writing - ``"1.0000e-08"`` yields a file strict consumers reject even though Pydantic's lax - mode happens to coerce it back. + Notation is part of the output contract (the tutorial's edge example shows + ``"p_value":"1.0000e-03"``); Biolink's ``float`` typing is satisfied by Pydantic's + lax coercion, so the notation control costs no KGX validity. The + ``numeric_slot_kind`` float short-circuit must never fire for p-value columns. """ lf: pl.LazyFrame = pl.DataFrame({"p_value": ["1e-8", "0.05", "0.001"], "adjusted_p_value": ["0.0001", "0.1", "0.2"]}).lazy() result: pl.DataFrame = format_numeric(clean_numeric(lf)).collect() - assert result["p_value"].to_list() == [1e-08, 0.05, 0.001] - assert result["adjusted_p_value"].to_list() == [0.0001, 0.1, 0.2] - assert result.schema["p_value"] == pl.Float64 + assert result["p_value"].to_list() == ["1.0000e-08", "5.0000e-02", "1.0000e-03"] + assert result["adjusted_p_value"].to_list() == ["1.0000e-04", "1.0000e-01", "2.0000e-01"] + assert result.schema["p_value"] == pl.String + assert result.schema["adjusted_p_value"] == pl.String def test_format_numeric_decimal_general() -> None: @@ -1105,7 +1107,7 @@ def test_format_numeric_preserves_nulls() -> None: """format_numeric preserves nulls as null.""" lf: pl.LazyFrame = pl.DataFrame({"p_value": ["1e-8", "N/A", "0.05"]}).lazy() result: pl.DataFrame = format_numeric(clean_numeric(lf)).collect() - assert result["p_value"].to_list() == [1e-08, None, 0.05] + assert result["p_value"].to_list() == ["1.0000e-08", None, "5.0000e-02"] def test_format_numeric_cleans_float_noise() -> None: @@ -1124,14 +1126,22 @@ def test_format_numeric_noop_without_numeric_columns() -> None: def test_format_numeric_nulls_stripped_from_ndjson_rows() -> None: - """cleaned and formatted null numeric values are stripped from NDJSON rows.""" - lf: pl.LazyFrame = pl.DataFrame({"subject": ["BRCA1", "TP53"], "p_value": ["1e-8", "N/A"], "effect_size": ["0.85", "0.42"]}).lazy() + """cleaned and formatted null numeric values are stripped from NDJSON rows. + + A zero p-value formats to ``"0.0000e+00"`` — a non-empty string, so it survives + ``strip_nulls`` instead of vanishing like the pre-#71 bug. + """ + lf: pl.LazyFrame = pl.DataFrame( + {"subject": ["BRCA1", "TP53", "EGFR"], "p_value": ["1e-8", "N/A", "0"], "effect_size": ["0.85", "0.42", "1.0"]} + ).lazy() formatted: pl.DataFrame = format_numeric(clean_numeric(lf)).collect() rows: list[dict[str, Any]] = [strip_nulls(r) for r in formatted.iter_rows(named=True)] - assert rows[0] == {"subject": "BRCA1", "p_value": 1e-08, "effect_size": "0.85"} + assert rows[0] == {"subject": "BRCA1", "p_value": "1.0000e-08", "effect_size": "0.85"} assert "p_value" not in rows[1] assert rows[1]["subject"] == "TP53" assert rows[1]["effect_size"] == "0.42" + assert rows[2]["p_value"] == "0.0000e+00" # zero survives strip_nulls + assert rows[2]["effect_size"] == "1" def test_compile_graph_emits_ndjson(monkeypatch: Any, tmp_path: Path, rig_factory: Any) -> None: @@ -2542,7 +2552,7 @@ def test_compile_subgraph_e2e_column_cleanup_and_numeric_annotations(monkeypatch assert result["original_subject"] == "BRCA-1 [alias]" assert result["object"] == "HGNC:11998" assert result["original_object"] == "TP 53" - assert result["p_value"] == 1e-08 + assert result["p_value"] == "1.0000e-08" # Both slots are unattached in biolink-model 4.4.3, so they are preserved on the # inlined StudyResult instead of being emitted unvalidatably on the edge. described: str = result["has_supporting_studies"][next(iter(result["has_supporting_studies"]))]["has_study_results"][0]["description"]