chore(release): 8.2.1 — unify sig() coercion rigor, document coercion, expand tests - #77
chore(release): 8.2.1 — unify sig() coercion rigor, document coercion, expand tests#77SkyeAv wants to merge 4 commits into
Conversation
…he column coercions
sig() derived statistical_significance_qualifier from a p-value column chosen by a
naive substring selector ([c for c in names if col in c] + fuzz.ratio). The four
coerce_* steps already share a rigorous pattern: a *_target() regex classifier, a
canonical-name-wins guard, and fuzz.ratio against target.replace('_',' '). sig() now
reuses pvalue_target() for the same classification, prefers a raw p_value column over
adjusted_p_value, and applies canonical-wins. Five-band cascade and Biolink class rule
(qualifier omitted without a p-value column) are unchanged.
Coerce_pvalue_columns runs before sig in the pipeline, so realistic builds are
unaffected; only a contrived p_value-substring column that pvalue_target rejects now
correctly omits the qualifier. Adds three rigor tests.
Extends the real-redb build_pipeline smoke to prove the whole coercion pipeline wires through end-to-end: raw annotation names (p value, sample size, odds ratio, effect type) normalize to canonical Biolink edge fields (p_value as a JSON number, effect_size in controlled notation, effect_type with the alias mapped to the EffectTypes enum), while supporting_study_size and the auto-derived statistical_significance_qualifier route into the inlined Study. Builds a tiny redb, not a real build-fullmap.
The p_value / supporting_study_size / effect_size / effect_type auto-normalization, effect_type value-to-enum mapping, relationship_strength forward-rename, the auto-derived statistical_significance_qualifier bands, and the Biolink class rules were previously undocumented behavior. Adds an 'Automatic column coercion' subsection to the table configuration reference.
Bumps version 8.2.0 -> 8.2.1 (pyproject.toml, uv.lock self-package, CITATION.cff) and folds the unreleased section into 8.2.1: the multi-section source.url list breaking change, the build-fullmap --aria2c opt-in, the sig() rigor unification, the coercion docs, and the new tests.
|
Warning Review limit reached
Next review available in: 4 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Release 8.2.1. Unifies the
statistical_significance_qualifierderivation onto the same fuzzy-matching rigor the fourcoerce_*steps already use, documents the (previously undocumented) automatic column-coercion behavior, and expands the test suite — including an end-to-end smoke proving the whole coercion pipeline produces the expected canonical Biolink edge fields. Folds the unreleasedsource.urllist breaking change and thebuild-fullmap --aria2copt-in into the 8.2.1 notes.Coercion rigor
sig()was the outlier. It picked its p-value column with a naive substring selector ([c for c in names if col in c]+fuzz.ratio(c, "p_value")), whilecoerce_pvalue_columns/coerce_study_size_columns/coerce_effect_size_columns/coerce_effect_type_columnsall share a rigorous pattern: a*_target()regex classifier, a canonical-name-wins guard, andfuzz.ratioagainsttarget.replace("_", " ").sig()reusespvalue_target()for candidate classification, prefers a rawp_valuecolumn overadjusted_p_value, and applies the same canonical-wins + spaced-reference fuzzy rule. The five-band cascade and the Biolink class rule (qualifier omitted when no p-value column is present) are unchanged.Design
lib.py_source_ops),coerce_pvalue_columnsruns beforesig, so by the timesigruns every p-value column is already canonicalp_value/adjusted_p_value; old and new selectors pick the same column. The lone divergence is a contrived column that contains ap_valuesubstring but is rejected bypvalue_target(e.g.xp_value), which now correctly omits the qualifier instead of deriving a bogus one.build-fullmaprun needed to verify. The e2e smoke builds a tiny real redb (rs.build_fullmap_db) and drives the fullbuild_pipeline, matching the existing offline smoke pattern.Docs
docs/configuration/table.md: new "Automatic column coercion" subsection documenting that p-value / study-size / effect-size / effect-type columns are auto-normalized to canonical Biolink names (witheffect_typevalues mapped to theEffectTypesenum, unmatched →null, and the legacyrelationship_strengthrenamed forward toeffect_size), thatstatistical_significance_qualifieris auto-derived into five bands, and the Biolink class rules (nulleffect_typewhereeffect_sizeis absent; qualifier omitted without a p-value column).Tests
tests/test_lib.py: threesig()rigor tests — raw-p_value-over-adjusted_p_valuepreference, canonical-column-wins-over-alias, andpvalue_target-based exclusion of ap_value-substring look-alike.tests/test_e2e_smoke.py:test_build_pipeline_coerces_statistical_annotations— declares annotations with raw source spellings (p value,sample size,odds ratio,effect type) and asserts the real pipeline emits canonicalp_value(JSON number),effect_size(controlled notation),effect_type(alias → enum), and routessupporting_study_size+ the auto-derivedstatistical_significance_qualifierinto the inlined Study.Release
8.2.0→8.2.1inpyproject.toml,uv.lock(self-package), andCITATION.cff; folds## Unreleasedinto## 8.2.1. This release also includes the previously-mergedsource.urllist breaking change (PR feat(models)!: accept multiple source URLs per section #76) and thebuild-fullmap --aria2copt-in (PR feat: add aria2c build-fullmap downloads #74), now attributed to 8.2.1 in the changelog.Testing
uv run pytest→761 passed, 34 skipped(87% total;src/tablassert/coerce.pyat 100%).uv run ruff check src tests→All checks passed!;uv run ruff format --check src tests→ all formatted.uv run pyright src/tablassert/coerce.py tests/test_lib.py tests/test_e2e_smoke.py→0 errors.uv run pytest tests/test_e2e_smoke.py::test_build_pipeline_coerces_statistical_annotations→ passed (real redb, end-to-end).CODE_REVIEWERpass (fresh-eyes on the diff) returned approve with two release-polish should-fixes, both applied (softened the "unaffected" changelog claim; pinnedp_value/effect_sizeJSON types in the e2e assertion).