Add Mammalian Phenotype Ontology - #300
Closed
gaurav wants to merge 29 commits into
Closed
Conversation
Mouse-Human Ontology Mapping Initiative (https://github.com/mapping-commons/mh_mapping_initiative)
Collaborator
Author
|
…requirements.lock Main replaced requirements.txt/requirements.lock with pyproject.toml. This commit brings the branch in sync: adds sssom (the new dependency for parsing HP-MP SSSOM mappings) to pyproject.toml and removes the old files that no longer exist on main. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…g.json Main replaced config.json with config.yaml. This commit ports the MP-related additions from this branch into the new format and removes config.json, which no longer exists on main. Changes applied to config.yaml: - ubergraph_ontologies: add MP (after HP) - disease_labelsandsynonyms: add MP (after HP) - disease_ids: add MP (after HP) - disease_concords: add HP_MP (before Manual) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Main reformatted prefixes.py (single→double quotes, consistent spacing, new prefixes). This commit takes main's reformatted version and adds MP = "MP" to eliminate the formatting conflict on merge. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Main enforces ruff and snakefmt formatting. Pre-applying it here reduces merge conflicts to semantic content only. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds Mammalian Phenotype Ontology (MP) support to the disease/phenotype compendium build, including HP↔MP mappings sourced from SSSOM files, and updates configuration/dependencies to incorporate MP into the pipeline.
Changes:
- Add MP prefix support in UberGraph SPARQL query templates and introduce
MPas a recognized prefix constant. - Add an HP↔MP concord generation step using SSSOM mappings, and include MP/HP_MP in disease pipeline config lists.
- Add
sssomas a project dependency to parse SSSOM mapping files.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
src/ubergraph.py |
Adds MP: prefix declarations to relevant SPARQL templates so MP CURIEs can be used in queries. |
src/snakefiles/diseasephenotype.snakefile |
Adds a new rule to generate an HP_MP concord file from SSSOM mapping URLs. |
src/prefixes.py |
Introduces the MP prefix constant. |
src/createcompendia/diseasephenotype.py |
Adds MP ID writing, HP↔MP concord extraction from SSSOM, and integrates MP into clique/typing logic. |
pyproject.toml |
Adds sssom dependency for parsing SSSOM TSVs. |
config.yaml |
Adds MP and HP_MP to disease IDs/labels/concord configuration and enables MP in UberGraph ontologies list. |
Comments suppressed due to low confidence (2)
config.yaml:212
MPwas added todisease_labelsandsynonyms, which makes the pipeline require${download_directory}/MP/labelsand/MP/synonyms(seedisease_compendiainputs). Currentlyconfig.generate_dirs_for_labels_and_synonyms_prefixesdoes not includeMP, soget_obo_labels/get_obo_synonymswon’t generate those per-prefix files and Snakemake will fail. Please either addMPtogenerate_dirs_for_labels_and_synonyms_prefixes(config.yaml) or refactor the disease pipeline to read labels/synonyms from the common UberGraph files instead of per-prefix files.
disease_labelsandsynonyms:
- MONDO
- DOID
- Orphanet
- HP
- MP
- MESH
- NCIT
- UMLS
- SNOMEDCT
- EFO
config.yaml:224
MPwas added todisease_ids, which meansdisease_compendiawill require${intermediate_directory}/disease/ids/MP. There is a newwrite_mp_ids()helper, but no Snakemake rule creates this ids file insrc/snakefiles/diseasephenotype.snakefile, so the workflow will be missing an input. Add adisease_mp_idsrule (callingdiseasephenotype.write_mp_ids) or removeMPfromdisease_idsuntil it’s wired up.
disease_ids:
- MONDO
- DOID
- Orphanet
- HP
- MP
- MESH
- NCIT
- UMLS
- OMIM
- EFO
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+195
to
+198
| def build_hp_mp_concords(hp_mp_sssom_urls, outfile, threshold=0.8, acceptable_predicates=["skos:exactMatch"]): | ||
| # We rely on the files from the | ||
| # Mouse-Human Ontology Mapping Initiative (https://github.com/mapping-commons/mh_mapping_initiative) | ||
|
|
Comment on lines
+179
to
+205
| rule get_hp_mp_concord: | ||
| output: | ||
| outfile=config["intermediate_directory"] + "/disease/concords/HP_MP", | ||
| run: | ||
| hp_mp_sssom_urls = [ | ||
| # https://github.com/mapping-commons/mh_mapping_initiative/blob/master/mappings/mp_hp_eye_impc.sssom.tsv | ||
| "https://raw.githubusercontent.com/mapping-commons/mh_mapping_initiative/master/mappings/mp_hp_eye_impc.sssom.tsv", | ||
| # https://github.com/mapping-commons/mh_mapping_initiative/blob/master/mappings/mp_hp_hwt_impc.sssom.tsv | ||
| "https://raw.githubusercontent.com/mapping-commons/mh_mapping_initiative/master/mappings/mp_hp_hwt_impc.sssom.tsv", | ||
| # https://github.com/mapping-commons/mh_mapping_initiative/blob/master/mappings/mp_hp_mgi_all.sssom.tsv | ||
| "https://raw.githubusercontent.com/mapping-commons/mh_mapping_initiative/master/mappings/mp_hp_mgi_all.sssom.tsv", | ||
| # https://github.com/mapping-commons/mh_mapping_initiative/blob/master/mappings/mp_hp_owt_impc.sssom.tsv | ||
| "https://raw.githubusercontent.com/mapping-commons/mh_mapping_initiative/master/mappings/mp_hp_owt_impc.sssom.tsv", | ||
| # https://github.com/mapping-commons/mh_mapping_initiative/blob/master/mappings/mp_hp_pat_impc.sssom.tsv | ||
| "https://raw.githubusercontent.com/mapping-commons/mh_mapping_initiative/master/mappings/mp_hp_pat_impc.sssom.tsv", | ||
| # https://github.com/mapping-commons/mh_mapping_initiative/blob/master/mappings/mp_hp_pistoia.sssom.tsv | ||
| "https://raw.githubusercontent.com/mapping-commons/mh_mapping_initiative/master/mappings/mp_hp_pistoia.sssom.tsv", | ||
| # https://github.com/mapping-commons/mh_mapping_initiative/blob/master/mappings/mp_hp_xry_impc.sssom.tsv | ||
| "https://raw.githubusercontent.com/mapping-commons/mh_mapping_initiative/master/mappings/mp_hp_xry_impc.sssom.tsv", | ||
| ] | ||
| diseasephenotype.build_hp_mp_concords( | ||
| hp_mp_sssom_urls, | ||
| output.outfile, | ||
| threshold=0.8, | ||
| acceptable_predicates=["skos:exactMatch", "skos:closeMatch", "skos:relatedMatch"], | ||
| ) | ||
|
|
Comment on lines
+183
to
+197
| hp_mp_sssom_urls = [ | ||
| # https://github.com/mapping-commons/mh_mapping_initiative/blob/master/mappings/mp_hp_eye_impc.sssom.tsv | ||
| "https://raw.githubusercontent.com/mapping-commons/mh_mapping_initiative/master/mappings/mp_hp_eye_impc.sssom.tsv", | ||
| # https://github.com/mapping-commons/mh_mapping_initiative/blob/master/mappings/mp_hp_hwt_impc.sssom.tsv | ||
| "https://raw.githubusercontent.com/mapping-commons/mh_mapping_initiative/master/mappings/mp_hp_hwt_impc.sssom.tsv", | ||
| # https://github.com/mapping-commons/mh_mapping_initiative/blob/master/mappings/mp_hp_mgi_all.sssom.tsv | ||
| "https://raw.githubusercontent.com/mapping-commons/mh_mapping_initiative/master/mappings/mp_hp_mgi_all.sssom.tsv", | ||
| # https://github.com/mapping-commons/mh_mapping_initiative/blob/master/mappings/mp_hp_owt_impc.sssom.tsv | ||
| "https://raw.githubusercontent.com/mapping-commons/mh_mapping_initiative/master/mappings/mp_hp_owt_impc.sssom.tsv", | ||
| # https://github.com/mapping-commons/mh_mapping_initiative/blob/master/mappings/mp_hp_pat_impc.sssom.tsv | ||
| "https://raw.githubusercontent.com/mapping-commons/mh_mapping_initiative/master/mappings/mp_hp_pat_impc.sssom.tsv", | ||
| # https://github.com/mapping-commons/mh_mapping_initiative/blob/master/mappings/mp_hp_pistoia.sssom.tsv | ||
| "https://raw.githubusercontent.com/mapping-commons/mh_mapping_initiative/master/mappings/mp_hp_pistoia.sssom.tsv", | ||
| # https://github.com/mapping-commons/mh_mapping_initiative/blob/master/mappings/mp_hp_xry_impc.sssom.tsv | ||
| "https://raw.githubusercontent.com/mapping-commons/mh_mapping_initiative/master/mappings/mp_hp_xry_impc.sssom.tsv", |
…utable default - Replace df.size (rows×cols) with len(df) in log messages — was inflating row counts by ~20× - Fix mutable default acceptable_predicates=[] → None with inner guard - Replace slow index-based DataFrame iteration with vectorized mask + to_csv - Extract "sssom:NoTermFound" to a named module-level constant - Remove redundant comment in write_mp_ids that restated the function name - Restore double-quote style in disease_mesh_ids snakefile rule (style regression) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…data Three pre-existing wiring bugs blocked PR #300 from running end-to-end: the disease_compendia rule referenced ${intermediate_directory}/disease/ids/MP but no rule produced it (write_mp_ids() existed but was never called); MP was in disease_labelsandsynonyms but missing from generate_dirs_for_labels_and_synonyms_prefixes, so MP labels/synonyms were never generated either; and 'from sssom import parsers' failed at import time because linkml 1.9.3 referenced Format.JSON which was removed in linkml-runtime 1.10.0. Adds the disease_mp_ids rule, adds MP to the labels/synonyms prefix list, and pins linkml>=1.10 (which downgrades sssom to 0.4.11). uv run snakemake --dry-run disease now resolves cleanly. build_hp_mp_concords now applies its three filters (confidence threshold, sssom:NoTermFound, predicate allowlist) in named stages and emits a metadata-HP_MP.yaml capturing per-input row totals, drops at each stage with per-predicate breakdowns, and a top-level concord-walk. The combined_from mechanism in src/metadata/provenance.py is reused; write_concord_metadata gains a combined_from parameter so concord-walk counts and per-input provenance can share one file. The first run revealed that all 1671 Pistoia rows are dropped (1670 below 0.8 confidence, 1 owl:equivalentClass not in the predicate set) and that 750 MGI broad/narrow rows are correctly rejected as asymmetric. Adds three documentation files: docs/sources/MP/README.md (durable provenance and pipeline-wiring reference, with master URLs and commit-pinned permalinks); docs/sources/MP/ValidationFindings.md (the wiring fixes, the PR author's two mapping-quality concerns, and the surprising filter outcomes the metadata.yaml made visible); and docs/AddingSources/README.md, a new directory seeded for cross-cutting source-validation conventions (DAG smoke test in CI, metadata.yaml as the source of truth for counts, predicate policy, per-source doc requirement). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The metadata.yaml format that Babel emits alongside concords, downloads, and compendia was previously only documented by example. With the addition of per-input filter-stage counts in the HP-MP concord (using the also_combined_from pattern at the concord level rather than just the compendium level), the schema deserves a single reference. Captures the four helpers in src/metadata/provenance.py, the on-disk schema, the four established type values (download / concord / compendium / transform), the recursive combined_from pattern, and conventions and known gaps. Linked from docs/README.md, docs/AddingSources/README.md, and CLAUDE.md. CLAUDE.md also gains a Conventions entry for source additions covering the two PR-blocking traps from PR #300: a prefix list edit needs a matching producing rule, and ontology prefixes need to be in generate_dirs_for_labels_and_synonyms_prefixes for UberGraph-driven labels and synonyms to materialise. The dry-run smoke test catches both. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Collaborator
Author
|
Superceded by PR #790. Closing. |
gaurav
added a commit
that referenced
this pull request
Jun 23, 2026
Mirror EMAPA's anatomy wiring on the disease/phenotype side: - build_disease_obo_relationships() now also writes disease/concords/MP. Uses UberGraph build_sets() with set_type="xref" from the MP root MP:0000001; the default subClassOf walk is correct (unlike EMAPA which needed part_of). - create_typed_sets()'s prefix-priority loop extended from [MONDO, HP] to [MONDO, HP, MP] so pure-new MP cliques (those without a MONDO or HP partner) are typed as PhenotypicFeature directly via prefix authority rather than falling through to the majority-vote path. MP is always last in the list, so MONDO and HP keep typing priority for any clique that contains them. - New disease_mp_ids Snakemake rule with retries: 10 (UberGraph backed). - get_disease_obo_relationships gets the MP concord output + metadata-MP.yaml, passes "MP": output.mp_metadata_yaml through to build_disease_obo_relationships. The whole rule now also carries retries: 10, matching anatomy's equivalent — this was missing before and is the same UberGraph backend that motivated the retry there. SSSOM-derived MP↔HP/NCIT mappings are deliberately not loaded; see the docs commit later in this branch for the rationale and the prior PR #300 cases that made that the safer default for now. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
gaurav
added a commit
that referenced
this pull request
Jun 23, 2026
write_mp_ids() and the MP concord branch of build_disease_obo_relationships both build SPARQL queries that reference MP:0000001. The query templates in src/ubergraph.py declare a fixed set of CURIE prefixes (UBERON, CL, EMAPA, GO, CHEBI, MONDO, HP, NCIT, PR, EFO) and MP was missing, so the queries failed with QueryBadFormed against the UberGraph endpoint. Add `prefix MP: <http://purl.obolibrary.org/obo/MP_>` to each of the five SPARQL templates that declare a prefix block. PR #300 added the same declaration when it tried to add MP as a disease/phenotype source. This should logically have landed with commit 902072f ("Wire MP into the disease/phenotype pipeline") but slipped through; pulling it out as its own commit keeps the fault and its fix clearly attributable. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
gaurav
added a commit
that referenced
this pull request
Jun 23, 2026
Five MP source docs paralleling docs/sources/EMAPA/: - README.md, download.md, filtering.md — standard structure for a UberGraph- backed OBO source. - mappings.md — the SME-facing document. Explicitly records that SSSOM mappings from mh_mapping_initiative were considered and intentionally not used in this PR, and names the two PR-#300 failure cases (MP:0003342↔HP:0001748 wrong, MP:0001914↔NCIT:C26791 missed via EFO) as the concrete reasons. Points at the impact report for the artefact that should drive SME conversation about whether to revisit SSSOM. - impact-report.md — auto-generated by `uv run source-impact-report --source MP` after a full `uv run snakemake -c all disease` build. Key findings: - 14,634 MP ids; 663 UberGraph xref rows. - 14,325 pure-new MP-only cliques; 214 existing cliques expanded; 0 cliques merged — UberGraph alone introduces no controversial bridges across previously-separate cliques. This is the key SME-facing signal: the historically-controversial SSSOM merges from PR #300 do not occur on the UberGraph-only path. - Only 2 HP partner-prefix rows in the MP concord file — confirms PR #300's observation that UberGraph has essentially no MP↔HP coverage. Bridging MP to HP requires SSSOM (or another mapping source) by design. - Partner-prefix breakdown contains a small number of https/http rows (18 total) — these look like raw URLs that were not normalised into CURIEs by build_sets(). Worth a follow-up but not blocking. - Section 2 shows 93 MP ids landed in Disease.txt rather than PhenotypicFeature.txt — surfaces MP terms that got pulled into a MONDO/UMLS-dominated clique typed as Disease. Worth SME review case-by- case. The full local build was run with one workaround: the get_omim rule's HTTP fetch returned 403 (OMIM gates the default Python urllib user-agent). The file downloads fine with curl using a browser UA; I dropped it into babel_downloads/OMIM/ manually and Snakemake picked it up. Fixing the UA in pull_via_urllib is a separate, repo-wide concern and worth its own commit/PR. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
gaurav
added a commit
that referenced
this pull request
Jun 30, 2026
Mirror EMAPA's anatomy wiring on the disease/phenotype side: - build_disease_obo_relationships() now also writes disease/concords/MP. Uses UberGraph build_sets() with set_type="xref" from the MP root MP:0000001; the default subClassOf walk is correct (unlike EMAPA which needed part_of). - create_typed_sets()'s prefix-priority loop extended from [MONDO, HP] to [MONDO, HP, MP] so pure-new MP cliques (those without a MONDO or HP partner) are typed as PhenotypicFeature directly via prefix authority rather than falling through to the majority-vote path. MP is always last in the list, so MONDO and HP keep typing priority for any clique that contains them. - New disease_mp_ids Snakemake rule with retries: 10 (UberGraph backed). - get_disease_obo_relationships gets the MP concord output + metadata-MP.yaml, passes "MP": output.mp_metadata_yaml through to build_disease_obo_relationships. The whole rule now also carries retries: 10, matching anatomy's equivalent — this was missing before and is the same UberGraph backend that motivated the retry there. SSSOM-derived MP↔HP/NCIT mappings are deliberately not loaded; see the docs commit later in this branch for the rationale and the prior PR #300 cases that made that the safer default for now. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
gaurav
added a commit
that referenced
this pull request
Jun 30, 2026
write_mp_ids() and the MP concord branch of build_disease_obo_relationships both build SPARQL queries that reference MP:0000001. The query templates in src/ubergraph.py declare a fixed set of CURIE prefixes (UBERON, CL, EMAPA, GO, CHEBI, MONDO, HP, NCIT, PR, EFO) and MP was missing, so the queries failed with QueryBadFormed against the UberGraph endpoint. Add `prefix MP: <http://purl.obolibrary.org/obo/MP_>` to each of the five SPARQL templates that declare a prefix block. PR #300 added the same declaration when it tried to add MP as a disease/phenotype source. This should logically have landed with commit 902072f ("Wire MP into the disease/phenotype pipeline") but slipped through; pulling it out as its own commit keeps the fault and its fix clearly attributable. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
gaurav
added a commit
that referenced
this pull request
Jun 30, 2026
Five MP source docs paralleling docs/sources/EMAPA/: - README.md, download.md, filtering.md — standard structure for a UberGraph- backed OBO source. - mappings.md — the SME-facing document. Explicitly records that SSSOM mappings from mh_mapping_initiative were considered and intentionally not used in this PR, and names the two PR-#300 failure cases (MP:0003342↔HP:0001748 wrong, MP:0001914↔NCIT:C26791 missed via EFO) as the concrete reasons. Points at the impact report for the artefact that should drive SME conversation about whether to revisit SSSOM. - impact-report.md — auto-generated by `uv run source-impact-report --source MP` after a full `uv run snakemake -c all disease` build. Key findings: - 14,634 MP ids; 663 UberGraph xref rows. - 14,325 pure-new MP-only cliques; 214 existing cliques expanded; 0 cliques merged — UberGraph alone introduces no controversial bridges across previously-separate cliques. This is the key SME-facing signal: the historically-controversial SSSOM merges from PR #300 do not occur on the UberGraph-only path. - Only 2 HP partner-prefix rows in the MP concord file — confirms PR #300's observation that UberGraph has essentially no MP↔HP coverage. Bridging MP to HP requires SSSOM (or another mapping source) by design. - Partner-prefix breakdown contains a small number of https/http rows (18 total) — these look like raw URLs that were not normalised into CURIEs by build_sets(). Worth a follow-up but not blocking. - Section 2 shows 93 MP ids landed in Disease.txt rather than PhenotypicFeature.txt — surfaces MP terms that got pulled into a MONDO/UMLS-dominated clique typed as Disease. Worth SME review case-by- case. The full local build was run with one workaround: the get_omim rule's HTTP fetch returned 403 (OMIM gates the default Python urllib user-agent). The file downloads fine with curl using a browser UA; I dropped it into babel_downloads/OMIM/ manually and Snakemake picked it up. Fixing the UA in pull_via_urllib is a separate, repo-wide concern and worth its own commit/PR. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
gaurav
added a commit
that referenced
this pull request
Jun 30, 2026
Add docs/sources/MP/disjointness.md explaining the post-glom split, why unique_prefixes/concord-dropping are insufficient, and the measured impact (added/split/moved/deleted) from babel-clique-diff comparing the overlap-allowed build to the disjoint build. Commit the clique-diff CSV + summary JSON under docs/sources/MP/disjointness/. Update the MP and HP READMEs: MP/HP are now disjoint, so an MP clique carries only the Mammalia taxon and an HP clique only Homo sapiens (correcting the earlier "mixed cliques carry both taxa" note). Cross-link prior PRs (#790, #300, #883, #742/#781). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
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.
This PR adds terms from the Mammalian Phenotype Ontology as requested by CAM-KP (#240). I added all the MP identifiers as well as mappings from https://github.com/mapping-commons/mh_mapping_initiative to connect it to HP and other identifiers. I added
sssomas an explicit prerequisite so we can use it to read the SSSOM files in that GitHub repo, but that caused a lot of our other prerequisites to change (hence all the changes to requirements.lock).We end up with 13,335 cliques that consist only of an MP identifier and 588 cliques that combine MP identifiers with other identifiers. We have no cases where an
MP:identifier is chosen over other identifiers; the clique leaders we generate are:There are a bunch of mapping issues, such as:
Ordinarily I would be nervous about including MP without more/better mappings, but since this isn't going to affect autocomplete (where we specifically filter to MONDO|HP) and that MP identifiers aren't (currently) clique leaders, I think we can merge this in now and then fix cliquing issues if anybody runs into issues with them (for now, probably only CAM-KP).
Closes #240.
WIP: see how often this new information merges cliques in ways we don't expect.
Should be merged after PR #365.