feat: add effect_size and effect_type edge attributes (Biolink PR #1774) - #69
Merged
Conversation
Mirror the p-value/study-size coercion architecture for the merged-but-unreleased PR #1774 edge attributes (absent from pinned biolink-model 4.4.3, defined locally): - coerce.py: effect_size_target / coerce_effect_size_columns rename effect-size-like columns (effect size, ES, beta, log2FC, odds/hazard/risk ratio, correlation/rho/r-style statistic names, and the OLD relationship_strength name) to canonical effect_size. effect_type_target / coerce_effect_type_columns rename effect-type-like columns, then coerce VALUES into the 25 permissible enum values via a case/separator-insensitive alias table + rapidfuzz fuzzy fallback; values matching nothing drop to null (biolink range is the enum). Biolink class rule applied post-coercion: effect_type is nulled wherever effect_size is null/absent. - biolink.py: EFFECT_TYPE_VALUES (verbatim PR diff, close_mappings ignored) + EffectTypes str-enum via _build_str_enum with the KnowledgeLevels-style TYPE_CHECKING stub; add effect_size / effect_type to TABLASERT_EDGE_EXTRAS so they survive fold_unknown_to_supporting_text and reach final edges; drop the now-superseded relationship_strength from extras (consistent with sample_size fold behavior). - lib.py: both coercions wired into _source_ops after coerce_study_size_columns and before clean_numeric; PHASE_OF "clean"; numeric_columns exact set gains effect_size and drops the superseded sample_size / relationship_strength (coercion renames them before clean_numeric/format_numeric run). - docs + agent prompts: old annotation names moved to supporting_study_size / effect_size / effect_type in advanced-example.md, examples.md, tutorial.md, table.md, tutorial-table.yaml, agent.py INSTRUCTIONS, and optimized_instructions.yaml. - tests: +28 covering rename/best-fuzzy-pick/no-op/value-coercion/class-rule/folding and alias drift guards; existing old-name final-edge assertions moved to the new names (never weakened). Fixture config TEXT keeps old annotation names (coercion renames at build time). Verified: uv run ruff check . && uv run ruff format --check . && uv run pyright && uv run pytest -q — all green (717 passed, 34 skipped = main's skip set); coverage on touched modules at parity or better (coerce.py/biolink.py 100%).
…C alias
- advanced-example.md edge JSON: use {:.4g}-accurate values ("9", "0.85")
- tcode ordering test also asserts study-size coercion precedes effect-size
- add limma/edgeR "logFC" alias for log2_fold_change value coercion
- clarify ALAMV6 fixture keeps old annotation names as legacy-compat case
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThe PR adds canonical statistical edge annotations, defines 25 Biolink effect types, coerces effect columns, integrates coercion into cleaning, updates tests, and migrates documentation and agent guidance from legacy annotation names. ChangesStatistical edge annotations
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant SourcePipeline
participant EffectCoercion
participant NumericCleaning
SourcePipeline->>EffectCoercion: coerce effect_size and effect_type
EffectCoercion-->>SourcePipeline: return canonical effect columns
SourcePipeline->>NumericCleaning: clean numeric columns
NumericCleaning-->>SourcePipeline: return formatted output
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/agent/optimized_instructions.yaml`:
- Line 92: The effect_type guidance must encode both supported sources and its
dependency on a valid numeric effect_size. In
examples/agent/optimized_instructions.yaml at line 92, revise the prompt to
specify method: column for table-provided effect types and method: value for
fixed valid Biolink effect types, emitting effect_type only alongside a numeric
effect_size; in src/tablassert/agent.py at lines 1929-1930, update the
corresponding guidance so effect_type is not described only as a source column
and preserves the same contract.
In `@src/tablassert/coerce.py`:
- Around line 526-531: Update the column-selection logic in
src/tablassert/coerce.py#L526-L531 to return the existing effect_size column
without fuzzy ranking or renaming; only rank aliases when the canonical column
is absent. Apply the same change to effect_type at
src/tablassert/coerce.py#L669-L675. Add regression cases at
tests/test_lib.py#L1659-L1680 and tests/test_lib.py#L1747-L1752 covering both
canonical and spaced alias columns, verifying coercion succeeds and preserves
the canonical columns.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 94b532f6-52b5-4596-b614-eba71940c6f5
📒 Files selected for processing (13)
docs/configuration/advanced-example.mddocs/configuration/table.mddocs/examples.mddocs/examples/tutorial-table.yamldocs/tutorial.mdexamples/agent/optimized_instructions.yamlsrc/tablassert/agent.pysrc/tablassert/biolink.pysrc/tablassert/coerce.pysrc/tablassert/lib.pytests/test_agent_derive.pytests/test_biolink.pytests/test_lib.py
… contract - all four column coercions keep an existing canonical column instead of fuzzy-picking a higher-scoring spaced alias onto it (polars DuplicateError) - agent.py + optimized_instructions.yaml now encode the full effect_type contract: method: column OR method: value with a fixed valid Biolink effect type, emitted only alongside an effect_size annotation - +4 regression tests (effect_size/effect size, effect_type/effect type, p_value/p value, supporting_study_size/supporting study size)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds support for the new Biolink
effect size/effect typeassociation slots (biolink/biolink-model#1774) across the clean pipeline and final edges, using the same rapidfuzz coercion tactics asp_value/supporting_study_size. The pinnedbiolink-model4.4.3 predates the PR, so the enum is defined locally until a release ships it.Column coercion (
src/tablassert/coerce.py)coerce_effect_size_columns: renames effect-size-like columns (effect size,ES,beta,log2FC,odds ratio,hazard ratio, correlation/rho-style statistics, and the oldrelationship_strength) to canonicaleffect_size, picking the single best rapidfuzz candidate — matching thecoerce_study_size_columnspattern.coerce_effect_type_columns: renames effect-type-like columns toeffect_type, then coerces VALUES into the 25 permissibleEffectTypeEnumvalues via a case/separator-insensitive 37-entry alias table + rapidfuzz fallback (>= 80); unmatched values become null (Biolink range is the enum).effect typemay only be populated wheneffect sizeis —effect_typeis nulled whereeffect_sizeis missing or non-numeric, documented the waysig()documents its rule.Vocabulary + final edges (
src/tablassert/biolink.py,src/tablassert/lib.py)EFFECT_TYPE_VALUES/EffectTypes: the 25 values from the #1774 diff verbatim (mappings intentionally omitted), built with_build_str_enumin theKnowledgeLevelspattern; a comment marks the switch to_biolink_enum_values(_bm.EffectTypeEnum)oncebiolink-modelships it.effect_size/effect_typejoinTABLASERT_EDGE_EXTRASso they survivefold_unknown_to_supporting_textas top-level edge fields; supersededrelationship_strengthis removed from the extras and instead coerced toeffect_size._source_opsaftercoerce_study_size_columnsand beforeclean_numeric;numeric_columnsexact set is now{"effect_size", "supporting_study_size"}— the old namessample_size/relationship_strengthare replaced everywhere they appeared in the final-edges path.Docs + agent prompts
docs/configuration/advanced-example.md,docs/examples.md,docs/tutorial.md,docs/configuration/table.md, andexamples/tutorial-table.yamlnow use the new names (JSON examples use{:.4g}-accurate values).src/tablassert/agent.pyINSTRUCTIONS andexamples/agent/optimized_instructions.yamlsteerp_value/supporting_study_size/effect_size+effect_type.Design
_association_model_fields()picks the slots up automatically and the extras stay harmless duplicates.relationship_strength/sample_sizestill work — coercion renames at build time before numeric cleaning (the ALAMV6 agent fixture deliberately keeps the old names as the end-to-end legacy case).rho/correlationeffect-size candidates mirror the documented p-value/study-size heuristic (they can catch an unrelated header only when it is the sole candidate); the shared duplicate-name-when-canonical-exists behavior of the existing coercions is preserved as-is. Deferred: further value aliases (R^2, bareEgger) — unmatched values null safely.Testing
uv run ruff check .— All checks passed.uv run ruff format --check .— 67 files already formatted.uv run pyright— 0 errors.uv run pytest -q— 717 passed, 34 skipped (identical skip set to main); +28 new tests (rename / best-fuzzy-pick / no-ops, alias + fuzzy + unmatched value coercion, class-rule nulling both branches, fold survival, tcode ordering, enum drift guards). Independent CODE_REVIEWER pass: no Blockers; all Should-fixes applied.Questions for the reviewer
logFCalias. limma/edgeRlogFCmaps tolog2_fold_change(base-2 by convention) — keep, or drop until a real ingest needs it?Summary by CodeRabbit
New Features
effect_size,effect_type, andsupporting_study_sizeannotations.Documentation
Bug Fixes