Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ All notable changes to this project are documented in this file.

### Fixed
- **`number_of_cases` is now emitted as a JSON integer instead of source text.** The slot is typed `int` on biolink-model's `EntityToDiseaseAssociation` / `EntityToPhenotypicFeatureAssociation`, but it was absent from `lib.numeric_columns`' exact set — since 15.1's `STUDY_SIZE_EXEMPT_PATTERN` (#119) stopped it being *renamed* onto `study_size`, nothing cast its *values*, so the raw TSV cell (`"1"`) shipped on the edge NDJSON. Pydantic's lax validation coerced the string back to int inside `validate_kgx`, so the record still validated, and the Rust `uuid_on_collision: merge` recompute already wrote a real int — leaving the shipped graph type-inconsistent edge to edge. `number_of_cases` now rides the same `clean_numeric` / `format_numeric` machinery as `study_size`: `numeric_slot_kind` reads the `int` typing off the installed model, fractional / negative / non-numeric counts become null, and the release-mode `drop_low_number_of_cases` filter is untouched (it already cast inline).
- **`--release` builds no longer reuse section parquets cached by other build modes.** The section-parquet cache key was `STORE / f"{mkhash(s)}.parquet"` with `release` injected into `Tcode` outside the hashed dict, and `Tcode.collect` quick-exits on any existing store file — so a parquet cached by a non-release build (or a pre-16.6.0 build, where the case-count filter silently no-op'd) was reused verbatim in `--release` builds, skipping the release filters and shipping `applied_to_treat` edges with `number_of_cases < 25`. A new `_section_store_path` helper generalizes the existing `.head.parquet` idiom: `--head`, `--release`, and `--qc` each get their own suffix, composing when combined (`<h>.head.release.qc.parquet`). `--qc` belongs because `fullmap_audit` drops rejects from the cached parquet too — the same cache-poisoning class. ([#140](https://github.com/SkyeAv/Tablassert/pull/140))
- **The release-mode case-count and effect-size filters tolerate non-numeric cells instead of crashing.** `drop_low_number_of_cases` and `drop_zero_effect_size` cast with strict `Float64`, but the `csv` op reads sources with `has_header=False`, so a TSV's header row flows through as data and the strict cast raised `InvalidOperationError` on the non-numeric header cell. Both ops now cast with `strict=False` (the `clean_numeric` idiom); non-numeric cells become null and are kept, dying later at entity resolution as before. ([#140](https://github.com/SkyeAv/Tablassert/pull/140))

## 16.6.1 - 2026-09-04

Expand Down
33 changes: 30 additions & 3 deletions src/tablassert/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,32 @@
BABEL_SYNONYM_RE: re.Pattern[str] = re.compile(r'<a href="([^"]+\.gz)"')


def _section_store_path(h: str, head: bool = False, release: bool = False, qc: bool = False) -> Path:
"""Name a section's cached subgraph parquet so build-mode flags never share a cache file.

The cached parquet's content depends on the build mode: ``--head`` samples
rows, ``--release`` applies the release-mode significance filters, and
``--qc`` drops fullmap-audit rejects. ``Tcode.collect`` quick-exits on any
existing store file, skipping the whole op chain, so each mode combination
must cache under its own suffix or one mode's build is silently reused as
another's.

Args:
h: Section config hash (``mkhash`` of the section dict).
head: ``--head`` preview build flag.
release: ``--release`` build flag.
qc: ``--qc`` build flag.

Returns:
Store path like ``<h>.parquet`` with ``.head`` / ``.release`` / ``.qc``
inserted before the extension for each set flag.
"""
from tablassert.utils import STORE

suffix: str = (".head" if head else "") + (".release" if release else "") + (".qc" if qc else "")
return STORE / f"{h}{suffix}.parquet"


def _load_table_indexed(args: tuple[int, Path]) -> tuple[int, object]:
"""Load one table, tagged with its input index (multiprocessing worker).

Expand Down Expand Up @@ -162,7 +188,7 @@ def build_graph_pipeline(
from tablassert.fullmap import fullmap_db_path
from tablassert.lib import Tcode, compile_graph, compile_subgraph
from tablassert.progress import flatten_pydantic_error, format_section_compact
from tablassert.utils import STORE, mkhash
from tablassert.utils import mkhash

# Stage 1/6: load tables.
progress.stage("Loading Tables")
Expand Down Expand Up @@ -204,8 +230,9 @@ def build_graph_pipeline(
for s in sections:
h: str = mkhash(s)
start(f"{Path(str(s['config'])).stem} · {h[:8]}")
# --head preview builds cache to a distinct .head.parquet so they never clobber full builds.
store: Path = STORE / (f"{h}.head.parquet" if head else f"{h}.parquet")
# Mode flags change the cached parquet's content, so each combination caches
# to a distinct file and can never quick-exit another mode's build.
store: Path = _section_store_path(h, head=head, release=release, qc=qc)
try:
tcode.append(
Tcode.model_validate(
Expand Down
19 changes: 14 additions & 5 deletions src/tablassert/lib.py
Original file line number Diff line number Diff line change
Expand Up @@ -1046,12 +1046,16 @@ def drop_zero_effect_size(lf: pl.LazyFrame, col: str = "effect_size") -> pl.Lazy
Notes:
Only filters when the effect-size column exists; no-op for sections
without an ``effect_size`` column. Null effect sizes are kept (no
score was detected for that row).
score was detected for that row). The cast is non-strict (like
``clean_numeric``) and paired with ``ne_missing``: a non-numeric cell --
e.g. a header row flowing through as data because the ``csv`` op reads
with ``has_header=False`` -- becomes null and is kept, dying later at
entity resolution instead of crashing the build with a strict-cast error.
"""
names: list[str] = lf.collect_schema().names()
if col not in names:
return lf
return lf.filter(pl.col(col).is_null() | (pl.col(col).cast(pl.Float64) != 0.0))
return lf.filter(pl.col(col).cast(pl.Float64, strict=False).ne_missing(0.0))


def drop_low_number_of_cases(lf: pl.LazyFrame, col: str = "number_of_cases", threshold: float = 25.0) -> pl.LazyFrame:
Expand All @@ -1068,13 +1072,18 @@ def drop_low_number_of_cases(lf: pl.LazyFrame, col: str = "number_of_cases", thr
Notes:
Only filters when the number-of-cases column exists; no-op for sections
without a ``number_of_cases`` column. Null case counts are kept (no count
was detected for that row). Only wired in for ``applied_to_treat``
sections at op-construction time, so it never touches other predicates.
was detected for that row). The cast is non-strict (like ``clean_numeric``)
with ``fill_null(True)``: a non-numeric cell -- e.g. a header row flowing
through as data because the ``csv`` op reads with ``has_header=False`` --
becomes null and is kept, dying later at entity resolution instead of
crashing the build with a strict-cast error. Only wired in for
``applied_to_treat`` sections at op-construction time, so it never
touches other predicates.
"""
names: list[str] = lf.collect_schema().names()
if col not in names:
return lf
return lf.filter(pl.col(col).is_null() | (pl.col(col).cast(pl.Float64) >= threshold))
return lf.filter(pl.col(col).cast(pl.Float64, strict=False).ge(threshold).fill_null(True))


def idxname(col: Any) -> str:
Expand Down
48 changes: 48 additions & 0 deletions tests/test_lib.py
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,38 @@ def test_tcode_collect_omits_drop_low_number_of_cases_for_other_predicates_in_re
assert "drop_not_significant" in names


def test_section_store_path_suffixes_every_build_mode() -> None:
"""_section_store_path gives each build-mode flag combination its own cache file name."""
assert cli._section_store_path("abc123").name == "abc123.parquet"
assert cli._section_store_path("abc123", head=True).name == "abc123.head.parquet"
assert cli._section_store_path("abc123", release=True).name == "abc123.release.parquet"
assert cli._section_store_path("abc123", qc=True).name == "abc123.qc.parquet"
# Modes compose instead of overwriting one another's suffix.
assert cli._section_store_path("abc123", head=True, release=True, qc=True).name == "abc123.head.release.qc.parquet"


def test_tcode_collect_quick_exit_never_serves_another_mode_cached_parquet(monkeypatch: Any, fixtures_path: Path, tmp_path: Path) -> None:
"""A parquet cached by a non-release build never quick-exits a release build (and vice versa)."""
# STORE is relative to the cwd, so chdir keeps the simulated cache inside tmp_path.
monkeypatch.chdir(tmp_path)
(tmp_path / ".tablassert" / "store").mkdir(parents=True)
data: Any = from_yaml(fixtures_path / "minimal_section.yaml")
# A previous non-release build cached this section's subgraph (filters skipped).
pl.DataFrame({"subject": ["cached"]}).write_parquet(cli._section_store_path("sectionhash"))
store: Path = cli._section_store_path("sectionhash", release=True)
tcode_model: Tcode = Tcode.model_validate( # pyright: ignore
{**data, "config": fixtures_path / "minimal_section.yaml", "store": store, "release": True}
)

collected: Any = tcode_model.collect(tmp_path / "fullmap.redb")

# The release store does not exist yet: the non-release parquet must not be reused.
assert not isinstance(collected, Path)
# Once the release-mode build has written its own parquet, the quick exit serves it.
pl.DataFrame({"subject": ["release"]}).write_parquet(store)
assert tcode_model.collect(tmp_path / "fullmap.redb") == store


def test_tcode_model_validate_rejects_duplicate_qualifier_keys(fixtures_path: Path) -> None:
"""Tcode construction rejects a section whose statement repeats a qualifier key.

Expand Down Expand Up @@ -1292,6 +1324,22 @@ def test_drop_low_number_of_cases_noop_without_column() -> None:
assert list(result["subject"]) == ["a", "b"]


def test_drop_low_number_of_cases_keeps_non_numeric_cells() -> None:
"""drop_low_number_of_cases keeps non-numeric cells (a header row read as data) instead of crashing on a strict cast."""
# The csv op reads sources with has_header=False, so a TSV's header row flows
# through as a data row; the strict cast raised InvalidOperationError on it.
lf: pl.LazyFrame = pl.DataFrame({"subject": ["hdr", "a", "b", "c"], "number_of_cases": ["number_of_cases", "30", "10", None]}).lazy()
result: pl.DataFrame = drop_low_number_of_cases(lf).collect()
assert list(result["subject"]) == ["hdr", "a", "c"]


def test_drop_zero_effect_size_keeps_non_numeric_cells() -> None:
"""drop_zero_effect_size keeps non-numeric cells (a header row read as data) instead of crashing on a strict cast."""
lf: pl.LazyFrame = pl.DataFrame({"subject": ["hdr", "a", "b", "c"], "effect_size": ["effect_size", "0.0", "1.5", None]}).lazy()
result: pl.DataFrame = drop_zero_effect_size(lf).collect()
assert list(result["subject"]) == ["hdr", "b", "c"]


def test_numeric_columns_matches_p_value_substring() -> None:
"""numeric_columns matches any column with P value in the name."""
names: list[str] = ["p_value", "adjusted_p_value", "log_p_value", "subject"]
Expand Down
Loading