fix: emit number_of_cases as a JSON integer instead of source text - #139
Merged
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 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 |
SkyeAv
added a commit
that referenced
this pull request
Sep 4, 2026
Cut 16.6.2 and bump the package version in pyproject.toml, uv.lock, and CITATION.cff. Patch: two fix PRs ship. `number_of_cases` is now emitted as a JSON integer instead of raw source text — the slot is typed `int` on biolink-model but sat outside `lib.numeric_columns`, so string cells shipped on the edge NDJSON and left the graph type-inconsistent edge to edge (#139). `--release` builds no longer reuse section parquets cached by other build modes: `--head`, `--release`, and `--qc` now cache under distinct suffixes via `_section_store_path`, closing a cache-poisoning bug where release filters were silently skipped and `applied_to_treat` edges with `number_of_cases < 25` shipped (#140). The release-mode `drop_low_number_of_cases` and `drop_zero_effect_size` filters also tolerate non-numeric cells (e.g. a TSV header row read as data) instead of crashing with `InvalidOperationError` (#140). Changelog: - Versioned the Unreleased section as 16.6.2 and gave the `number_of_cases` entry its missing PR link (#139). Docs: none needed here — #140's `cli.py` change is an internal cache-key helper and #139 changes emission types only; both are fully documented in their changelog entries. Testing: - uv run pytest -q -> 1253 passed, 3 skipped (96% coverage) - uv run ruff check . && uv run ruff format --check . && uv run pyright -> clean / 0 errors - uv lock --check -> up to date - uv run mkdocs build --strict -> clean
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.
FAERS/DailyMed
applied_to_treatedges shippednumber_of_casesas a raw TSV string ("1") even though biolink-model types the slotint— the numeric coercion machinery simply missed the slot. This widensnumeric_columns()so the count leaves the pipeline as a real JSON integer, matching the slot's model typing and the Rust merge path.Fixes
STUDY_SIZE_EXEMPT_PATTERNstoppednumber_of_casesbeing renamed ontostudy_size, but nothing ever cast its values — the raw TSV cell ("1") rode through asStringonto the edge NDJSON.numeric_columns()' exact set now includesnumber_of_cases, so it rides the sameclean_numeric/format_numericmachinery asstudy_size.biolink.numeric_slot_kind("number_of_cases")reads theinttyping off the installed model (EntityToDiseaseAssociation/EntityToPhenotypicFeatureAssociation), soformat_numericemits realInt64; fractional, negative, and non-numeric counts become null, exactly like study counts.validate_kgxnever flagged this because Pydantic's lax validation coerces"1"→1for theint-ranged slot, while the Rustuuid_on_collision: mergerecompute already wrote a real int — leaving the shipped KG type-inconsistent edge to edge. Now uniform.Compatibility
drop_low_number_of_casesunchanged: the release-mode filter already cast inline toFloat64, so it works on either dtype.test_faers_case_count_rides_the_edge_as_number_of_casesacceptsint | strand anticipated this fix. Follow-up: tighten that test to requireintafter this release.Testing
uv run pytest -q→1164 passed, 44 skipped(full gate env:--group dev --extra qc --extra log+maturin develop).uv run ruff check ./uv run ruff format --check ./uv run pyright→ all clean.test_format_numeric_emits_number_of_cases_as_int:"1"/"25"→1/25asInt64;"0.42"/"-3"/"abc"/null → null.