diff --git a/CHANGELOG.md b/CHANGELOG.md index 3729529..55da00d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - The `supports_snp_labels` and `supports_secondary_axis` boolean properties are removed. Optional capabilities are negotiated with `@runtime_checkable` protocols (`SupportsRegionHighlight`, `SupportsSNPLabels`, `SupportsSecondaryAxis`) checked by `isinstance`. `supports_hover` stays a boolean. - **Legend styling is uniform across plot families.** The five per-family legend styles are replaced by one. Visible differences from 1.x: the LD lead-SNP marker is 7pt rather than 6pt; eQTL, fine-mapping, and effect legend text is 9pt rather than 8pt, making those boxes slightly taller; and the effect legend now carries an "Effect" title. Legend content, colours, and ordering are unchanged. - **`schemas.py` validators are expressed on `DataFrameValidator`** instead of a parallel hand-rolled implementation. Which frames are accepted or rejected is unchanged; error wording differs where the engine phrases a check differently (`Missing columns: [...]` for `Missing required column`, `values <= 0` for `non-positive`, `null values` for `missing values`). A frame with a missing column now also reports its dtype and range faults instead of stopping at the first phase. +- **BREAKING: load-time validation rejects nulls consistently.** `validate_gwas_dataframe` already required non-null positions and p-values; `validate_eqtl_dataframe`, `validate_finemapping_dataframe`, and `validate_genes_dataframe` did not, so a file with a blank `p_value`, `pip`, `start`, or `end` loaded silently and lost those rows later at plot time. All four now reject nulls in the numeric columns they range-check, naming the offending column. Plot-time intake is unchanged and still filters nulls with a warning, which is the intended two-tier split. - **BREAKING: two never-read parameters are removed**: `validate_gwas_dataframe(rs_col=...)` and `validate_finemapping_dataframe(cs_col=...)`. Both were accepted and documented but never used. - **Internal**: coordinate liftover moved behind a `CoordinateLifter` port with a pure `liftover_positions` (`_liftover.py`), and recombination header detection was extracted as the pure `ensure_recomb_header`. `liftover_recombination_map` keeps its signature. diff --git a/CONTEXT.md b/CONTEXT.md index d4b01b8..d68ed87 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -57,6 +57,12 @@ visualization concepts the library renders. Settled decisions live in - **eQTL `gene` requirement** — required at load (multi-gene GTEx files keyed by gene) but optional at plot (frames may already be gene-filtered). Same intentional two-tier shape as GWAS. +- **Nulls are a load-time failure, not a plot-time one** — every `schemas.py` + validator rejects nulls in the numeric columns it range-checks, so a malformed + file fails at load with the offending column named. Plot-time intake stays + lenient and filters nulls with a warning (`prepare_pvalue_data`, + `prepare_eqtl_for_plotting`), because a frame reaching the plotter may have + been assembled by the caller rather than loaded from a file. ## Deepenings from the design grill diff --git a/src/pylocuszoom/schemas.py b/src/pylocuszoom/schemas.py index 0eeaa86..3a13828 100644 --- a/src/pylocuszoom/schemas.py +++ b/src/pylocuszoom/schemas.py @@ -80,6 +80,7 @@ def validate_eqtl_dataframe( _loader_validator(df, "eQTL") .require_columns(["pos", "p_value", "gene"]) .require_numeric(["pos", "p_value"]) + .require_not_null(["pos", "p_value"]) .require_range("pos", min_val=0, exclusive_min=True) .require_pvalue("p_value") .validate() @@ -110,6 +111,7 @@ def validate_finemapping_dataframe( _loader_validator(df, "Fine-mapping") .require_columns(["pos", "pip"]) .require_numeric(["pos", "pip"]) + .require_not_null(["pos", "pip"]) .require_range("pos", min_val=0, exclusive_min=True) .require_range("pip", min_val=0, max_val=1) .validate() @@ -140,6 +142,7 @@ def validate_genes_dataframe( _loader_validator(df, "Gene annotation") .require_columns(["chr", "start", "end", "gene_name"]) .require_numeric(["start", "end"]) + .require_not_null(["start", "end"]) .require_range("start", min_val=0) .require_ordering("start", "end") .validate() diff --git a/tests/test_validation_contract.py b/tests/test_validation_contract.py index 5d19c8b..989b764 100644 --- a/tests/test_validation_contract.py +++ b/tests/test_validation_contract.py @@ -1,10 +1,13 @@ """Prose-independent contract for the loader schema validators. -Pins what callers can rely on across a change of validation implementation: -which inputs are accepted, which raise, the exception class, and which column -names the error names. Deliberately asserts nothing about error phrasing, so -the same file passes before and after ``schemas.py`` moves onto +Pins what callers can rely on: which inputs are accepted, which raise, the +exception class, and which column names the error names. Deliberately asserts +nothing about error phrasing, so a change of validation implementation is a +checkable claim rather than a promise. It held unchanged across the move onto ``DataFrameValidator``. + +This is the strict load-time tier. Plot-time intake is deliberately more +permissive; see the two-tier note in ``CONTEXT.md``. """ import pandas as pd @@ -55,21 +58,11 @@ def _genes(**overrides): validate_eqtl_dataframe, pd.DataFrame({"pos": [100], "p_value": [1.0], "gene": ["BRCA1"]}), ), - ( - "eqtl-null-p-tolerated", - validate_eqtl_dataframe, - pd.DataFrame({"pos": [100, 200], "p_value": [0.05, None], "gene": ["A", "B"]}), - ), ( "finemapping-pip-bounds-inclusive", validate_finemapping_dataframe, pd.DataFrame({"pos": [100, 200], "pip": [0.0, 1.0]}), ), - ( - "finemapping-null-pip-tolerated", - validate_finemapping_dataframe, - pd.DataFrame({"pos": [100, 200], "pip": [0.5, None]}), - ), ("genes-minimal", validate_genes_dataframe, _genes()), ("genes-start-at-zero", validate_genes_dataframe, _genes(start=[0])), ("genes-point-feature", validate_genes_dataframe, _genes(start=[1000], end=[1000])), @@ -212,6 +205,38 @@ def test_accepted_input_returns_the_same_object(validator, df): pd.DataFrame({"pos": [0], "pip": [0.5]}), ["pos"], ), + ( + "eqtl-null-pvalue", + validate_eqtl_dataframe, + pd.DataFrame({"pos": [100, 200], "p_value": [0.05, None], "gene": ["A", "B"]}), + ["p_value"], + ), + ( + "eqtl-null-pos", + validate_eqtl_dataframe, + pd.DataFrame({"pos": [100, None], "p_value": [0.05, 0.1], "gene": ["A", "B"]}), + ["pos"], + ), + ( + "finemapping-null-pip", + validate_finemapping_dataframe, + pd.DataFrame({"pos": [100, 200], "pip": [0.5, None]}), + ["pip"], + ), + ( + "finemapping-null-pos", + validate_finemapping_dataframe, + pd.DataFrame({"pos": [100, None], "pip": [0.5, 0.6]}), + ["pos"], + ), + ( + "genes-null-start", + validate_genes_dataframe, + _genes( + chr=["1", "1"], start=[1000, None], end=[2000, 3000], gene_name=["A", "B"] + ), + ["start"], + ), ( "genes-missing-all", validate_genes_dataframe,