Skip to content

Commit d02abae

Browse files
lucapinelloclaude
andcommitted
Make "I want to add an example" a documented path instead of a reverse-engineering exercise
CONTRIBUTING opened with "this guide will walk you through the process of implementing a new oracle", and Current Priorities listed three kinds of contribution, all of them models. Read literally, someone with a variant they care about and no intention of porting a neural network is told in sentence one that they are in the wrong document — and adding a worked example is the cheapest useful contribution chorus can receive. It was also genuinely undocumented. Four regeneration scripts own different example types, and the matrix mapping example -> script -> valid --oracle -> conda env lived in **CLAUDE.md** (a file for Claude sessions), the CHANGELOG, and a May audit report. None of those is where a contributor looks. That matrix exists because getting it wrong is the single most common way a regeneration silently does nothing: * `regenerate_multioracle.py` accepts chrombpnet / cherimoya / legnet / alphagenome and pointedly **not** enformer, whose report comes from `regenerate_examples.py` instead; * it also has no `--gpu`, while the other two do — passing one is an argparse error that scrolls past in a tailed log; * the wrong conda env does not fail fast. A missing oracle package logs `Failed to load <track>` once per track and carries on, so a run can spend an hour loading nothing. So: a routing table in the first screen (oracle / example / bug fix, with sizes), a "Contributing an example or walkthrough" section carrying the verified matrix, the real entry shape copied from `ENFORMER_EXAMPLES`, the four traps, and the six guard tests to expect — all of which collect (138 tests), unlike the browser recipe that shipped collecting 0. Priorities now name examples, doc corrections and tests alongside models. `examples/*/README.md` point at the one canonical section rather than restating it. Guard: 8 tests pinning the table to the scripts' argparse definitions rather than to a proofread — every documented `--oracle` value must be a real choice (negations excluded and covered separately), the no-enformer exception must remain true in both directions, the `--gpu` asymmetry must stay correctly described, the notebook generator must stay argument-free, and every test file the section tells a contributor to run must exist. Fast suite 1,993 passed / 30 skipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 98acb31 commit d02abae

4 files changed

Lines changed: 280 additions & 1 deletion

File tree

CONTRIBUTING.md

Lines changed: 106 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,19 @@
11
# Contributing to Chorus
22

3-
Thank you for your interest in contributing to Chorus! This guide will walk you through the process of implementing a new oracle (genomic sequence prediction model) step by step.
3+
Thank you for your interest in contributing to Chorus! Most of this guide is a step-by-step for
4+
implementing a new **oracle** (a genomic sequence prediction model), because that is the largest
5+
kind of contribution — but it is not the only one we want, and the smaller ones are not lesser.
6+
7+
**Start here — what are you contributing?**
8+
9+
| you want to add | go to | rough size |
10+
|---|---|---|
11+
| a new model / oracle | [Implementing a new oracle](#step-by-step-guide-to-implementing-a-new-oracle) below | a day or more; 9 registration sites, an env, a background |
12+
| a **worked example or walkthrough** | [Contributing an example](#contributing-an-example-or-walkthrough) | an hour; one list entry and one script run |
13+
| a bug fix, a doc fix, a test | [Running the tests](#running-the-tests) then open the PR | minutes |
14+
15+
You do not need a HuggingFace account, our GPU, or our HuggingFace dataset for any of these — see
16+
[You do not need our infrastructure](#you-do-not-need-our-infrastructure-to-develop-or-to-open-the-pr).
417

518
## Overview
619

@@ -432,6 +445,87 @@ BORZOI_ENV_CONFIG = {
432445

433446
6. **Documentation**: Include docstrings for all public methods
434447

448+
## Contributing an example or walkthrough
449+
450+
The fastest useful contribution. If there is a variant, locus or question you care about and chorus
451+
already ships an oracle that can answer it, adding it as a worked example is roughly one list entry
452+
plus one script run.
453+
454+
Chorus commits two different kinds of example:
455+
456+
* **Walkthroughs**`examples/walkthroughs/<category>/<name>/`. A real variant or locus, pre-run,
457+
committed together with its HTML report, JSON and TSV, so a reader sees the answer with no GPU and
458+
no install. These are **declarative**: you add a dict, a script produces everything else.
459+
* **Notebooks**`examples/notebooks/*.ipynb`, hand-written library tutorials. Note the
460+
`notebook.ipynb` *inside* each walkthrough is code-generated by
461+
`scripts/generate_walkthrough_notebooks.py` and must not be hand-edited.
462+
463+
### 1. Add the entry
464+
465+
For a variant walkthrough, append to the matching list in `scripts/regenerate_examples.py`
466+
(`ALPHAGENOME_EXAMPLES`, `ENFORMER_EXAMPLES` or `CHROMBPNET_EXAMPLES`). An entry is just:
467+
468+
```python
469+
{
470+
"name": "SORT1 rs12740374 (Enformer)",
471+
"dir": f"{BASE}/variant_analysis/SORT1_enformer",
472+
"type": "discovery",
473+
"position": "chr1:109274968",
474+
"ref": "G", "alt": "T",
475+
"gene": "SORT1",
476+
"html_name": "rs12740374_SORT1_enformer_report.html",
477+
}
478+
```
479+
480+
`ref` **must** match the reference genome at that position — `strict_ref=True` is the default, so a
481+
wrong ref raises `ReferenceAlleleMismatchError` rather than quietly substituting.
482+
483+
### 2. Run the right script, in the right env
484+
485+
This table is the part that costs people an afternoon. Getting it wrong usually does **not** fail
486+
loudly — see the traps below.
487+
488+
| what you added | script | valid `--oracle` | conda env |
489+
|---|---|---|---|
490+
| a variant walkthrough | `scripts/regenerate_examples.py` | `alphagenome`, `enformer`, `chrombpnet`, `all` | `chorus` (these use `use_environment=True`, so they spawn the per-oracle env themselves) |
491+
| a per-oracle multioracle report | `scripts/regenerate_multioracle.py --oracle X` | `chrombpnet`, `cherimoya`, `legnet`, `alphagenome`**no `enformer`** | `chorus-X` |
492+
| the unified IGV panel | `scripts/regenerate_multioracle.py --consolidate` || `chorus` |
493+
| discovery / causal / region_swap / integration / batch / TERT | `scripts/regenerate_remaining_examples.py --only all` || `chorus-alphagenome` |
494+
| the walkthrough's `notebook.ipynb` | `scripts/generate_walkthrough_notebooks.py` | — (codegen only, no GPU) | `chorus` |
495+
496+
Enformer has no `--oracle enformer` in the multioracle script; its single-oracle report comes from
497+
`regenerate_examples.py`.
498+
499+
### 3. Traps worth knowing before you start
500+
501+
* **The wrong env does not fail fast.** A missing oracle package logs `Failed to load <track>` once
502+
per track and carries on, so a long run can spend an hour loading nothing and then die at the end.
503+
* **`regenerate_multioracle.py` has no `--gpu` flag** (the other two do). Passing one is an argparse
504+
error that scrolls past in a tailed log.
505+
* **Do not write `mamba run -n X --no-capture-output ...`** — a flag after `-n` makes the wrapper die
506+
with `exec: --: invalid option` before your script starts, which reads like a completed step.
507+
* **Finish editing before you start regenerating.** Python reads source at process start, so a change
508+
made mid-run reaches only the examples generated after it, leaving a committed set that is
509+
internally inconsistent in a way no test compares.
510+
* `conda run` buffers stdout, so a long build's log stays empty until it exits. Use
511+
`conda run --no-capture-output ... python -u` when you want to watch it.
512+
513+
### 4. What will go red
514+
515+
Committed examples are guarded, deliberately — an example whose HTML no longer matches its JSON is
516+
worse than no example. Expect to run:
517+
518+
```bash
519+
pytest tests/test_committed_examples.py tests/test_json_tsv_parity.py \
520+
tests/test_walkthrough_readmes_match_artefacts.py \
521+
tests/test_summary_tables_match_their_artefact_by_label.py \
522+
tests/test_batch_rows_reconcile_with_headline.py \
523+
tests/test_rerender_refuses_to_degrade.py
524+
```
525+
526+
If a guard fails, it is usually telling you a regeneration step was skipped or run in the wrong env,
527+
not that the guard is wrong.
528+
435529
## Running the tests
436530

437531
From the `chorus` base env, at the repo root. This is the command CI runs, and a guard test enforces
@@ -509,8 +603,19 @@ chorus/
509603
## Current Priorities
510604

511605
All eight core oracles (Enformer, Borzoi, ChromBPNet/BPNet, Sei, LegNet, AlphaGenome, Cherimoya/CATv1, EPInformer-seq) are implemented — nine registered names, since AlphaGenome ships both a JAX and a PyTorch backend. We're interested in contributions for:
606+
607+
**Models**
512608
1. **Custom fine-tuned models** — models trained on specific tissues or conditions
513609
2. **Species-specific oracles** — mouse, drosophila, etc.
514610
3. **New architectures** — HyenaDNA, Evo, Nucleotide Transformer, etc.
515611

612+
**Everything else — genuinely wanted, and much smaller**
613+
4. **Worked examples** — a variant, locus or question you care about, run through an oracle we
614+
already ship. This is the most useful contribution per hour of your time: it costs one entry in a
615+
declarative list plus one script run, and it is how most people discover what chorus does. See
616+
[Contributing an example](#contributing-an-example-or-walkthrough).
617+
5. **Documentation that was wrong or missing when you read it** — including "I followed this and it
618+
failed". A copy-pasteable repro of where you got stuck is a useful issue even with no patch.
619+
6. **Bug fixes and tests**, especially a test that pins something you found the hard way.
620+
516621
Thank you for contributing to Chorus! Your implementation will help make genomic deep learning models more accessible to the research community.

examples/notebooks/README.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,3 +81,10 @@ kernel from the Kernel menu.
8181
~30 s.
8282
- **Notebook cells show `<Figure ... >` but no image** — check your
8383
matplotlib backend; `%matplotlib inline` should be in cell 1.
84+
85+
## Want to add one?
86+
87+
See [CONTRIBUTING.md § Contributing an example or walkthrough](../../CONTRIBUTING.md#contributing-an-example-or-walkthrough).
88+
Note the `notebook.ipynb` inside each `examples/walkthroughs/*/` directory is **code-generated** by
89+
`scripts/generate_walkthrough_notebooks.py` — the hand-written tutorials are the ones in this
90+
directory.

examples/walkthroughs/README.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,3 +157,10 @@ Every analysis tool produces outputs in four formats:
157157
| JSON | `report.to_dict()` | Programmatic analysis, pipelines, notebooks |
158158
| TSV | `report.to_tsv(path)` or `report.to_dataframe()` | Excel, R, pandas |
159159
| HTML | `report.to_html(path)` | Visual review with embedded IGV genome browser |
160+
161+
## Want to add one?
162+
163+
Worked examples are the smallest useful contribution to chorus — one entry in a declarative list
164+
plus one script run. The canonical step-by-step, including which regeneration script owns which
165+
kind of example and which conda env it needs, is
166+
[CONTRIBUTING.md § Contributing an example or walkthrough](../../CONTRIBUTING.md#contributing-an-example-or-walkthrough).
Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
"""CONTRIBUTING's "which script, which oracle, which env" table must match the scripts.
2+
3+
Adding a worked example is the smallest useful contribution to chorus, and for a long time it was
4+
documented nowhere a contributor would look: the script/oracle/env matrix lived in `CLAUDE.md` (for
5+
Claude sessions) and in an audit report, while `CONTRIBUTING.md` opened by saying it was a guide to
6+
implementing an oracle. Someone who wanted to add a variant walkthrough had to reverse-engineer four
7+
regeneration scripts to find which one owned it.
8+
9+
The matrix is now in CONTRIBUTING, which makes it a drift risk: `regenerate_multioracle.py` accepts
10+
four oracles and pointedly **not** `enformer`, `regenerate_examples.py` accepts a different three
11+
plus `all`, and only two of the three take `--gpu`. Every one of those asymmetries has already cost
12+
a debugging session, and a table that silently goes stale costs the next one too — so it is pinned
13+
to the argparse definitions rather than proofread.
14+
"""
15+
from __future__ import annotations
16+
17+
import ast
18+
from pathlib import Path
19+
20+
import pytest
21+
22+
REPO = Path(__file__).resolve().parent.parent
23+
CONTRIBUTING = REPO / "CONTRIBUTING.md"
24+
SCRIPTS = REPO / "scripts"
25+
26+
27+
def _arg_choices(script: Path, flag: str) -> list[str] | None:
28+
"""The `choices=[...]` of `flag` in the script's argparse setup, or None if the flag is absent."""
29+
tree = ast.parse(script.read_text())
30+
for node in ast.walk(tree):
31+
if not isinstance(node, ast.Call):
32+
continue
33+
fn = node.func
34+
if getattr(fn, "attr", None) != "add_argument":
35+
continue
36+
if not any(isinstance(a, ast.Constant) and a.value == flag for a in node.args):
37+
continue
38+
for kw in node.keywords:
39+
if kw.arg == "choices" and isinstance(kw.value, (ast.List, ast.Tuple)):
40+
return [e.value for e in kw.value.elts if isinstance(e, ast.Constant)]
41+
return [] # flag exists but takes no choices
42+
return None
43+
44+
45+
def _table() -> str:
46+
"""The example-contribution section, where the matrix lives."""
47+
text = CONTRIBUTING.read_text()
48+
i = text.index("## Contributing an example or walkthrough")
49+
return text[i:text.index("\n## ", i + 10)]
50+
51+
52+
@pytest.mark.parametrize("script,flag", [
53+
("regenerate_examples.py", "--oracle"),
54+
("regenerate_multioracle.py", "--oracle"),
55+
("regenerate_remaining_examples.py", "--only"),
56+
])
57+
def test_every_documented_choice_is_a_real_choice(script: str, flag: str):
58+
"""No value in the table that argparse would reject."""
59+
choices = _arg_choices(SCRIPTS / script, flag)
60+
assert choices is not None, f"{script} no longer defines {flag}; the CONTRIBUTING table is stale"
61+
table = _table()
62+
row = next((ln for ln in table.splitlines() if script in ln), None)
63+
assert row is not None, f"the CONTRIBUTING matrix has no row for {script}"
64+
65+
# anything rendered as `code` in that row which looks like an oracle name must be valid —
66+
# except names the row explicitly calls out as *unsupported* ("no `enformer`"), which are
67+
# covered by test_the_enformer_exception_is_still_true instead
68+
import re
69+
positive = re.sub(r"no\s+`[a-z_]+`", "", row)
70+
cited = {m for m in re.findall(r"`([a-z_]+)`", positive)}
71+
known_oracles = {"alphagenome", "enformer", "chrombpnet", "cherimoya", "legnet", "sei",
72+
"borzoi", "epinformerseq", "alphagenome_pt"}
73+
for name in cited & known_oracles:
74+
assert name in choices, (
75+
f"CONTRIBUTING's row for {script} cites `{name}`, but its {flag} choices are {choices}. "
76+
f"argparse would reject it — an invalid --oracle is an error that scrolls past in a "
77+
f"tailed log, which is exactly how a regeneration silently does nothing."
78+
)
79+
80+
81+
def test_the_enformer_exception_is_still_true():
82+
"""`regenerate_multioracle.py` deliberately has no `enformer`, and the docs say so.
83+
84+
This is the asymmetry most likely to be "helpfully" corrected by someone who assumes the table
85+
is a typo. If enformer is ever added, the sentence has to go.
86+
"""
87+
choices = _arg_choices(SCRIPTS / "regenerate_multioracle.py", "--oracle") or []
88+
table = _table()
89+
stated = "no `enformer`" in table or "no enformer" in table
90+
91+
if "enformer" in choices:
92+
assert not stated, (
93+
"regenerate_multioracle.py now accepts --oracle enformer, so CONTRIBUTING's "
94+
"'no enformer' note is wrong and must be removed"
95+
)
96+
else:
97+
assert stated, (
98+
"regenerate_multioracle.py rejects --oracle enformer and CONTRIBUTING no longer says so; "
99+
"a contributor will try it and read the argparse error as a broken script"
100+
)
101+
102+
103+
def test_the_gpu_flag_asymmetry_is_documented_correctly():
104+
"""Two of the three regeneration scripts take `--gpu`; the multioracle one does not."""
105+
has_gpu = {
106+
s: _arg_choices(SCRIPTS / s, "--gpu") is not None
107+
for s in ("regenerate_examples.py", "regenerate_multioracle.py",
108+
"regenerate_remaining_examples.py")
109+
}
110+
table = _table()
111+
if not has_gpu["regenerate_multioracle.py"]:
112+
assert "no `--gpu` flag" in table, (
113+
"regenerate_multioracle.py still has no --gpu, but CONTRIBUTING does not warn about it; "
114+
f"passing one is an argparse error. (--gpu presence: {has_gpu})"
115+
)
116+
else:
117+
pytest.fail(
118+
"regenerate_multioracle.py gained a --gpu flag — remove the warning from CONTRIBUTING"
119+
)
120+
121+
122+
def test_the_notebook_generator_is_still_codegen_only():
123+
"""Documented as taking no arguments and needing no GPU."""
124+
script = SCRIPTS / "generate_walkthrough_notebooks.py"
125+
tree = ast.parse(script.read_text())
126+
adds = [n for n in ast.walk(tree)
127+
if isinstance(n, ast.Call) and getattr(n.func, "attr", None) == "add_argument"]
128+
assert not adds, (
129+
f"generate_walkthrough_notebooks.py now takes {len(adds)} argument(s); CONTRIBUTING "
130+
f"describes it as codegen-only with no flags"
131+
)
132+
133+
134+
def test_the_guard_files_named_in_contributing_exist():
135+
"""The section tells a contributor which tests to run. All of them must be real files."""
136+
import re
137+
138+
table = _table()
139+
named = re.findall(r"tests/(test_[a-z0-9_]+)\.py", table)
140+
assert named, "the example section no longer names any guard tests to run"
141+
missing = [n for n in named if not (REPO / "tests" / f"{n}.py").is_file()]
142+
assert not missing, (
143+
f"CONTRIBUTING tells contributors to run tests that do not exist: {missing}. A documented "
144+
f"command that errors is worse than no command."
145+
)
146+
147+
148+
def test_the_routing_table_covers_the_non_oracle_paths():
149+
"""The doc used to open by saying it was about implementing an oracle, and nothing else.
150+
151+
A contributor with an example to add read sentence one and concluded they were in the wrong
152+
place. The routing table exists so that does not happen.
153+
"""
154+
head = CONTRIBUTING.read_text()[:2000].lower()
155+
for want, why in (
156+
("example", "an example contributor must see themselves in the first screen"),
157+
("bug fix", "small fixes are wanted and were never mentioned"),
158+
("oracle", "the oracle path is still the main one and must stay findable"),
159+
):
160+
assert want in head, f"CONTRIBUTING's opening does not mention {want!r}{why}"

0 commit comments

Comments
 (0)