Repository navigation
Add conservation-score tracks and a unified annotation catalog - #246
lorenzoruggerii wants to merge 6 commits into
Conversation
GPN-Star, PhyloP 20-way, and PhastCons 7-way become optional IGV tracks on variant reports (analyze_variant_multilayer(..., show_conservation=True)), bulk-downloaded and cached like oracle weights rather than streamed per-region. A new AnnotationStore unifies these with GENCODE GTF annotations and user-added custom entries behind one list/describe/download interface (CLI `chorus conservation` / `chorus annotation`, and matching MCP tools), with downloaded bigwigs' declared genome build physically verified against the file's own chromosome-1 length. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Fixes from reviewing #246. Nine defects, each with a test that fails without the fix, plus the documentation the new surface was missing. **Silent wrong data** * Conservation tracks were plotted onto non-hg38 reports. Every source is hg38, and a bigwig read against another assembly returns values rather than erroring, so an mm10 report showed human conservation against mouse coordinates. Now raises, checked before any work is done. * No coverage rendered as MAXIMUM conservation: NaN -> 0.0, then invert -> clip(1-0,0,1) = 1.0, drawn as a solid full-height bar with autoscale off. Uncovered positions are dropped; genuine zeros still plot (both cases tested). * The sequence-logo track sat one base left of every coolbox track above it -- coolbox passes 0-based half-open, the height function reads 1-based inclusive. The existing test encoded the wrong expectation (4 values for a 3-base range); corrected, and pinned with a property test that the logo covers exactly the span coolbox does. **Data loss** * `chorus annotation remove --delete-file` unlinked the user's own registered file for kind="local" entries -- files chorus never downloaded, on a flag whose help says "the downloaded file". A test asserted this behaviour; it now asserts the opposite, with a second test for the case the flag is actually for. **Reproducibility** * The GPN-Star downloads passed no revision=, fetching songlab/gpn-star-scores at its head. Pinned to a7b13bbf. The existing pin guard reads only normalization.py, so it could not see the new sites; it now covers the artefact-download modules, and a new guard checks the track configs rather than the call site (the call moved into a shared helper, so a call-site scan would pass while the pin was None). Note: widening that guard package-wide flags 13 pre-existing unpinned model-weight sites (alphagenome_pt, cherimoya, chrombpnet, epinformerseq, legnet, sei, utils/annotations, _igv_report). Left alone deliberately -- a real question, but a separate policy call, not this PR's job. **Crashes on ordinary input** * A `custom_annotations.yaml` with an empty `annotations:` key took down every listing path with an AttributeError naming neither the file nor the key. setdefault only fires when a key is absent. * A source with no derivable filename was accepted at registration, then failed at download with `'NoneType' object has no attribute 'exists'`. Now raises with advice at the point of use. * A 0-byte file printed the optimistic size estimate rather than its real size -- the one case a user needs to see -- because the check was truthiness rather than `is not None`. * The assembly check never ran on the path that reads the data: it was called from describe_annotation/add_annotation, which a report never touches. _bigwig_path verifies after download, with a confident mismatch raising and an unreadable file warning (the second half matters -- an earlier version of this fix turned a stub fixture into a hard failure of the download path). **Duplication** * One `hf_download_flat` helper replaces two line-for-line copies of the HF download-and-flatten block. They differed only in whether they passed revision= -- which is exactly how the unpinned download shipped. **Documentation, which was absent** * Nothing user-facing mentioned any of this: `show_conservation`, `chorus conservation`, `chorus annotation`, `AnnotationStore` and the three MCP tools appeared in 0 files across README, docs/ and examples/. README gains a Conservation tracks section, its MCP count goes 24 -> 27 with the new tools in the catalogue, and docs/API_DOCUMENTATION.md gains Annotations & Conservation. * Three copies of the blurb (build_variant_report, analyze_variant_multilayer, and the text rendered into every report's HTML) described the logo track as IGV's dynseq showing clip(1-entropy,0,1). It is chorus's own stacked-logo track showing p(base)*(2-H) on a 0-2 bit scale. The same copies said ~25 GB of downloads; with the logo track it is ~70 GB. **Recorded, not changed:** `igv.min.js` was replaced (1.35 -> 1.50 MB, dropping bundled jQuery 3.3.1) with no mention in the changelog. The 19 committed example reports still inline the OLD bundle, and the browser check renders committed reports -- so the new bundle currently ships with no CI coverage at all. Also left for the author's judgement: add_annotation validating genome_build against a 5-entry chr1-fingerprint table (rejects dm6/ce11, which chorus otherwise supports), _write_html_report re-deriving a path instead of learning it from to_html, and discover_variant rendering the report twice. Suite on this branch: 2,257 passed / 38 skipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…eview-fixes Review fixes for the conservation / annotation-store work
…alf the igv.min.js CI gap, ship the conservation notebook Addresses items PR #247 explicitly left open rather than fixed: - resolve_html_path: VariantReport and CausalResult now expose it, and both to_html and the MCP _write_html_report helper call it instead of re-deriving the output path independently. They used to disagree for an output_dir like /data/run.v2, where the dotted .v2 makes Path.suffix non-empty. Regression tests included. - discover_variant no longer renders and writes its HTML report twice. It now builds the AnalysisRequest before calling discover_variant_effects (which already accepted one for exactly this) instead of stamping it on after the first write and writing again. Regression test included. - Regenerated rs12740374_SORT1_chrombpnet_report.html, one of the two reports CHORUS_BROWSER_SMOKE=1 renders, so it embeds the new igv.min.js bundle (1.50 MB, no jQuery) instead of the old one. Verified with Playwright/Chromium against test_committed_reports_render_in_a_browser.py: the CI smoke subset and the full 19-report corpus both pass. The other smoke report (cherimoya) and 17 more still carry the old bundle -- regenerating them needs envs unavailable in this session. - single_oracle_quickstart.ipynb gains a Conservation Tracks section (show_conservation on the notebook's own GATA1 variant, plus an AnnotationStore listing), executed with real data rather than left unexecuted -- show_conservation, chorus conservation, chorus annotation and AnnotationStore appeared in no notebook before this. Left alone, documented rather than patched: add_annotation's genome_build check rejects dm6/ce11 even though chorus supports those genomes elsewhere, but the underlying chr1-fingerprint verification scheme can't apply to them at all (no chromosome named chr1) -- a real design decision, not a quick fix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Follow-up commit responding to #247's review: 3d0b6b1. Merged #247 in full first (fast-forward, no changes). This commit then addresses the items it explicitly left open: Fixed (both were "left for your judgement" in #247, not required fixes):
Partly closed — the
Shipped — the doc gap:
Left alone, documented rather than patched:
Full suite: 2182 passed (a pre-existing, unrelated set of ~40 environment-specific failures — stale background/provenance fixtures on this machine — confirmed identical with and without this commit's changes). |
|
Could you add a worked example for this before it lands? Right now the feature has no user-facing I've opened #247 against this branch with the prose half — a README Conservation tracks section, Either of these would do it; no need for both: a) A section in an existing notebook — b) A minimal walkthrough — What would make either genuinely useful rather than a smoke test: a variant where conservation Two things to know if you go the notebook route, both from #247:
|
…hlighting a strong predicted effect in a poorly conserved region
|
Hi Luca! Thank you for pointing this out. I decided to add a minimal walkthrough.
Committed via a new |
GPN-Star, PhyloP 20-way, and PhastCons 7-way become optional IGV tracks on variant reports (analyze_variant_multilayer(..., show_conservation=True)), bulk-downloaded and cached like oracle weights rather than streamed per-region. A new AnnotationStore unifies these with GENCODE GTF annotations and user-added custom entries behind one list/describe/download interface (CLI
chorus conservation/chorus annotation, and matching MCP tools), with downloaded bigwigs' declared genome build physically verified against the file's own chromosome-1 length.