Emit the reference DOI on bacdive metabolite-utilization edges - #776
realmarcin wants to merge 2 commits into
Conversation
BacDive attaches an @ref to every data item and lists the matching citations in each record's Reference section. The transform did not read it, so an edge could name the strain record it came from but never the paper. That linkage cannot be recovered downstream, because the edge no longer records which data item produced it. Adds reference_dois(), which maps a record's @ref ids to publication DOIs, and threads it through the metabolite-utilization path. Where a DOI is known the edge carries it alongside the existing provenance: ['infores:bacdive', 'bacdive:1', 'doi:10.1099/ijs.0.02862-0'] Two kinds of doi/url are skipped deliberately. Catalogue URLs (DSMZ, StrainInfo) are not publications, and BacDive's own record DOI (10.13145/bacdive...) points back at the record primary_knowledge_source already identifies. Either would attach confident-looking provenance that leads nowhere. _StrainProvenanceWriter.writerow takes publication as an optional keyword, so every call site that does not pass one emits exactly what it emitted before. That is what makes this landable one section at a time, and it is covered by a test rather than asserted. Scope is metabolite utilization only -- the carbon-source relations. The pattern is the same for the other sections: capture the item's @ref where the item is read, pass the resolved DOI at the write site. Tested: 51 bacdive tests pass, including 7 new ones. The three collection errors elsewhere in tests/ are a missing 'parameterized' dependency and are identical on a clean checkout. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The preceding commit landed tests/test_bacdive_reference_doi.py without the code it exercises: a stash/pop cycle during verification left the two source files unstaged, and the commit only picked up the staged test. This adds them, so the branch is self-consistent again: reference_dois() maps a record's @ref ids to publication DOIs, skipping catalogue URLs and BacDive's own record DOI REFERENCE_KEY the '@ref' constant, beside the other item keys writerow() takes an optional publication and appends it to the primary_knowledge_source list Kept as a separate commit rather than amending, since the previous one was already pushed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adversarial reviewReviewed on request; not my PR, so nothing pushed. The core idea is right and Quantitative claims were measured by replaying the PR's own Blockers
DesignThe coverage number in the PR body is measured on the wrong denominator. "300/300 sampled records resolve at least one paper DOI" is true (98,448/99,392 records, 99.0%), but per edge — which is what is emitted:
The 608,545 are not noise — by title they are Emitting only the DOI, and not the
TestsThe production wiring is entirely untested — verified by mutation. Deleting both hooks (the Also: Minor
Verified non-issues
Not concludedPost-merge behaviour of the duplicated rows in #794 (running the merge rewrites tracked artifacts); whether any DOI resolves to a real, correct paper (syntactic shape only). |
#737) (#795) * feat(bacdive): report per-reference phenotype conflicts, split by kind #737 quoted "4,278 strains get contradictory phenotype edges" from an ad-hoc measurement that was never persisted. Nothing on disk recorded which strains, or which references disagreed: the bacdive transform writes only nodes.tsv, edges.tsv and bacdive_media_links.txt, and the contradictory edges sit in edges.tsv unmarked — same primary_knowledge_source on both rows, and no slot for the `@ref` that would tell them apart. This makes the number reproducible and inspectable, and splits it three ways. Restricted to #737's eight fields the script reproduces **4,278 exactly**, which is the check that it is measuring the same thing. Splitting that total: specificity 955 values on one METPO subsumption chain contradiction 3,342 strains values that cannot both hold unresolved a value with no METPO term at all **The specificity share is far smaller than I assumed, and #737's note that `anaerobe` vs `obligate anaerobe` is "arguably not a conflict at all (2,643 of 4,278)" reads as if subsumption explains most of the problem. It does not.** All 955 specificity cases are oxygen tolerance, and there they are 36% of disagreements, 22% of the 4,278 — 854 `aerobe | obligate aerobe` and 101 `anaerobe | obligate anaerobe`. The dominant oxygen-tolerance patterns are genuine contradictions: `aerobe | facultative anaerobe` (440), `anaerobe | microaerophile` (299), `facultative anaerobe | microaerophile` (266). So ~3,342 strains really do need a resolution policy. Getting that split required resolving through METPO **synonyms**, not labels. BacDive writes `anaerobe`; METPO's label is `anaerobic` and carries `anaerobe` as a synonym. A label-only match resolves nothing and reports every oxygen-tolerance disagreement as a contradiction — my first cut did exactly that and reported 4,686 strains, all "contradiction". Third bucket is deliberate. `unresolved` means a value has no METPO term, so the relation cannot be judged; calling it a contradiction would report a curation gap as a data conflict. `colony color` is 1,196 rows, every one unresolved, because colours have no METPO terms. The script also scans four fields #737 did not — `colony color`, `colony shape`, `type of spore`, `forms multicellular complex` — adding 1,793 more disagreeing observations, of which `forms multicellular complex` (551) and `colony shape` (23) are the ones worth looking at. Deliberately not a gate: it exits 0 and reports. Wire it into CI once the counts are expected to be stable. One design note carried over from reviewing #776: `observations()` handles the list and single-dict shapes in **one** code path. Handling them per-section is how #793 happened — that PR captured `@ref` in the list branch and not the dict branch of a single section, silently unattributing 157 records. * fix: scope METPO resolution per field, so one axis cannot judge another From Codex's external review. A single flat alias index resolved every value through whichever METPO term sorted first, and 67 aliases are claimed by more than one term. The collisions land exactly on the yes/no fields: yes -> METPO:1000702 (motile) AND METPO:1000871 (spore forming) no -> METPO:1000703 (non motile) AND METPO:1000872 (non-spore forming) So spore formation, and `forms multicellular complex`, were being judged on the motility terms. It changed no verdict — the pairs are siblings on either axis, so "contradiction" came out right by luck — but one METPO edit putting two collision-sharing terms into a subsumption relation would silently downgrade real contradictions to `specificity`, discarding true statements. That is the exact failure this script exists to prevent, so luck is not good enough. `resolve()` now takes the field and intersects the alias's owners with that field's declared axis. A field with no axis accepts only an unambiguous alias and otherwise refuses. This changes one count for the better. `forms multicellular complex` has no METPO terms at all, so its 551 observations move from `contradiction` to `unresolved` — honest, since nothing in the ontology can express the concept. Contradiction strains: 3,342 -> 2,828. Unchanged: restricted to #737's eight fields the script still reproduces **4,278** exactly, and the specificity split is still 955.
1 participant
Addresses #770.
BacDive attaches an
@refto every data item and lists the matching citations in each record'sReferencesection. The transform doesn't read it, so an edge can name the strain record it came from but never the paper. That linkage can't be recovered downstream, because the edge no longer records which data item produced it.The change
reference_dois()maps a record's@refids to publication DOIs; the metabolite-utilization path threads it through. Where a DOI is known the edge carries it alongside the existing provenance:Same list-literal convention already used for the strain id, so nothing changes shape.
Two
doi/urlkinds are skipped deliberatelyCatalogue URLs (DSMZ, StrainInfo) aren't publications, and BacDive's own record DOI (
10.13145/bacdive…) points back at the recordprimary_knowledge_sourcealready identifies. Either would attach confident-looking provenance that leads nowhere.Backward compatible by construction
_StrainProvenanceWriter.writerowtakespublicationas an optional keyword, so every call site that doesn't pass one emits exactly what it emitted before. That's what makes this landable one section at a time, and there's a test asserting it rather than a claim.Scope
Metabolite utilization only — the carbon-source relations. The pattern for the rest is identical: capture the item's
@refwhere the item is read, pass the resolved DOI at the write site. I stopped at one section so the change is reviewable and the convention can be agreed before it's repeated ~8 times.Testing
reference_dois()against the real dump: 300/300 sampled records resolve at least one paper DOI.tests/are a missingparameterizeddependency and are identical on a clean checkout — not from this change.Why it matters downstream
In
kg-microbe-projectswe needed statement-level citations and had to re-parse the raw JSON in parallel, which covers 9 of ~50 referenced sections. In a BacDive-vs-assay comparison there, only 81 of 996 rows could be attributed to a paper — and the one row where two papers disagreed with each other (Alteromonas stellipolaris on NAG:10.1099/ijs.0.02862-0says "does not assimilate",10.1099/ijs.0.63521-0says "uses as carbon source") turned out to be the most informative row in the analysis. Without DOIs that's indistinguishable from a curation error.Threading
@refthrough the remaining sections would make every edge attributable and let that parallel extraction be deleted.