From e73134ede1cd5087ee8f8cb73eae72cbbb8b2c4d Mon Sep 17 00:00:00 2001 From: Kevin Schaper Date: Fri, 8 May 2026 20:35:05 -0700 Subject: [PATCH 1/2] Fix obograph_source prefix-map destruction and predicate mapping Two related bugs in the obograph TSV pipeline that together caused obojson IRIs to land as URIs (instead of CURIEs) in TSV output and well-mapped RO relations like RO:0002162 (in_taxon) to be demoted to biolink:related_to. - TsvSource and SssomSource set_prefix_map now use update_prefix_map (additive) rather than set_prefix_map (destructive). The base PrefixManager loads ~600 default JSON-LD prefix mappings in __init__; the transformer's source.set_prefix_map({}) call was wiping them. - ObographSource.read_edge passes the already-computed CURIE (not the IRI) to bmt's get_element_by_mapping, so RO:* relations with biolink slot mappings resolve correctly instead of falling through to related_to. Adjusted test_read_obograph1/test_read_jsonl2 expected edge count from 205 to 206: previously, two distinct relations between the same node pair (BFO:0000050 and RO:0002211) were both demoted to biolink:related_to and produced colliding deterministic edge ids that conflated in the test's dict. With BFO:0000050 now mapped correctly to biolink:part_of, all three edges are kept distinct. Closes #546. Closes #547. --- kgx/source/obograph_source.py | 7 +- kgx/source/sssom_source.py | 5 +- kgx/source/tsv_source.py | 11 ++- .../obograph_curie_and_predicate.json | 41 +++++++++++ .../unit/test_source/test_obograph_source.py | 72 ++++++++++++++++++- 5 files changed, 131 insertions(+), 5 deletions(-) create mode 100644 tests/resources/obograph_curie_and_predicate.json diff --git a/kgx/source/obograph_source.py b/kgx/source/obograph_source.py index edaf44e8..a2e2d750 100644 --- a/kgx/source/obograph_source.py +++ b/kgx/source/obograph_source.py @@ -203,7 +203,12 @@ def read_edge(self, edge: Dict) -> Optional[Tuple]: element = get_biolink_element(curie) if not element: try: - mapping = self.toolkit.get_element_by_mapping(edge["pred"]) + # bmt's get_element_by_mapping recognizes CURIE form + # (e.g. "RO:0002162"), not IRIs. The CURIE is computed + # above as `curie`; passing the IRI here silently falls + # back to biolink:related_to for relations that have a + # perfectly good biolink slot mapping. + mapping = self.toolkit.get_element_by_mapping(curie) if mapping: element = self.toolkit.get_element(mapping) diff --git a/kgx/source/sssom_source.py b/kgx/source/sssom_source.py index 3812ad91..0a610137 100644 --- a/kgx/source/sssom_source.py +++ b/kgx/source/sssom_source.py @@ -51,7 +51,10 @@ def set_prefix_map(self, m: Dict) -> None: Prefix to IRI map """ - self.prefix_manager.set_prefix_map(m) + # See TsvSource.set_prefix_map for the rationale: use update (additive) + # rather than set (destructive) so the JSON-LD context defaults aren't + # discarded when a caller passes no overrides. + self.prefix_manager.update_prefix_map(m) def set_reverse_prefix_map(self, m: Dict) -> None: """ diff --git a/kgx/source/tsv_source.py b/kgx/source/tsv_source.py index 2028359c..7c4e5a78 100644 --- a/kgx/source/tsv_source.py +++ b/kgx/source/tsv_source.py @@ -38,7 +38,16 @@ def set_prefix_map(self, m: Dict) -> None: Prefix to IRI map """ - self.prefix_manager.set_prefix_map(m) + # Use update (additive) rather than set (destructive). The base + # PrefixManager loads ~600 default prefix mappings from the JSON-LD + # context (HGNC, NCBIGene, MONDO, …) in __init__; calling + # `set_prefix_map({})` here — as the transformer does when no + # caller-supplied prefix_map is provided — would discard all of them + # and leave only the hardcoded biolink/owlstar/MONARCH triplet. + # That broke contraction of identifiers.org/hgnc/, identifiers.org/ncbigene/, + # and many other URI prefixes for every TSV-derived source (including + # ObographSource, which inherits from this). + self.prefix_manager.update_prefix_map(m) def set_reverse_prefix_map(self, m: Dict) -> None: """ diff --git a/tests/resources/obograph_curie_and_predicate.json b/tests/resources/obograph_curie_and_predicate.json new file mode 100644 index 00000000..b0d628b3 --- /dev/null +++ b/tests/resources/obograph_curie_and_predicate.json @@ -0,0 +1,41 @@ +{ + "graphs": [ + { + "id": "test", + "nodes": [ + { + "id": "http://identifiers.org/ncbigene/698782", + "lbl": "MLH1", + "type": "CLASS" + }, + { + "id": "http://identifiers.org/hgnc/5", + "lbl": "A1BG", + "type": "CLASS" + }, + { + "id": "http://purl.obolibrary.org/obo/NCBITaxon_9544", + "lbl": "Macaca mulatta", + "type": "CLASS" + }, + { + "id": "http://purl.obolibrary.org/obo/NCBITaxon_9606", + "lbl": "Homo sapiens", + "type": "CLASS" + } + ], + "edges": [ + { + "sub": "http://identifiers.org/ncbigene/698782", + "pred": "http://purl.obolibrary.org/obo/RO_0002162", + "obj": "http://purl.obolibrary.org/obo/NCBITaxon_9544" + }, + { + "sub": "http://identifiers.org/hgnc/5", + "pred": "http://purl.obolibrary.org/obo/RO_0002162", + "obj": "http://purl.obolibrary.org/obo/NCBITaxon_9606" + } + ] + } + ] +} diff --git a/tests/unit/test_source/test_obograph_source.py b/tests/unit/test_source/test_obograph_source.py index 3b1d180b..2d03e427 100644 --- a/tests/unit/test_source/test_obograph_source.py +++ b/tests/unit/test_source/test_obograph_source.py @@ -28,7 +28,12 @@ def test_read_obograph1(): nodes[rec[0]] = rec[1] assert len(nodes) == 176 - assert len(edges) == 205 + # Was 205 prior to the predicate-mapping fix: two distinct relations + # (BFO:0000050 and RO:0002211) between GO:0007165 and GO:0008150 were both + # silently demoted to biolink:related_to and produced colliding deterministic + # edge ids, conflating them in the dict. With BFO:0000050 now correctly + # mapped to biolink:part_of, all three edges are kept distinct. + assert len(edges) == 206 n1 = nodes["GO:0003677"] assert n1["id"] == "GO:0003677" @@ -91,7 +96,12 @@ def test_read_jsonl2(): nodes[rec[0]] = rec[1] assert len(nodes) == 176 - assert len(edges) == 205 + # Was 205 prior to the predicate-mapping fix: two distinct relations + # (BFO:0000050 and RO:0002211) between GO:0007165 and GO:0008150 were both + # silently demoted to biolink:related_to and produced colliding deterministic + # edge ids, conflating them in the dict. With BFO:0000050 now correctly + # mapped to biolink:part_of, all three edges are kept distinct. + assert len(edges) == 206 n1 = nodes["GO:0003677"] assert n1["id"] == "GO:0003677" @@ -267,6 +277,64 @@ def test_error_detection(): t.write_report(None, "Warning") +def test_identifiers_org_uris_contracted_via_transform(): + """ + Regression: when running through ``transform()``, the prefix manager's + ~600 default JSON-LD context mappings used to be wiped out by + ``TsvSource.set_prefix_map({})``, leaving non-OBO IRIs uncontracted in the + TSV output. After the fix, identifiers.org/{hgnc,ncbigene} URIs land as + proper CURIEs. + """ + output_basename = os.path.join(TARGET_DIR, "obograph_curie_and_predicate") + transform( + inputs=[os.path.join(RESOURCE_DIR, "obograph_curie_and_predicate.json")], + input_format="obojson", + output=output_basename, + output_format="tsv", + stream=False, + ) + + tin = Transformer() + g = TsvSource(tin).parse( + filename=output_basename + "_nodes.tsv", format="tsv" + ) + nodes = {} + for rec in g: + if rec and len(rec) != 4: + nodes[rec[0]] = rec[1] + + # Node ids are CURIEs, not IRIs. The original IRI is preserved in `iri`. + assert "NCBIGene:698782" in nodes + assert nodes["NCBIGene:698782"]["iri"] == "http://identifiers.org/ncbigene/698782" + assert "HGNC:5" in nodes + assert nodes["HGNC:5"]["iri"] == "http://identifiers.org/hgnc/5" + + +def test_obograph_predicate_mapping_uses_curie(): + """ + Regression: ``ObographSource.read_edge`` used to pass the IRI to + ``bmt.Toolkit.get_element_by_mapping``, which only recognizes CURIE form. + The lookup silently returned None and well-mapped RO predicates fell + through to the catch-all biolink:related_to. After the fix, RO:0002162 + resolves to its proper biolink slot (in_taxon). + """ + t = Transformer() + s = ObographSource(t) + g = s.parse(os.path.join(RESOURCE_DIR, "obograph_curie_and_predicate.json")) + + edges = [] + for rec in g: + if rec and len(rec) == 4: + edges.append(rec[3]) + + taxon_edges = [e for e in edges if e.get("relation") == "RO:0002162"] + assert len(taxon_edges) == 2 + for e in taxon_edges: + assert e["predicate"] == "biolink:in_taxon", ( + f"expected biolink:in_taxon for RO:0002162, got {e['predicate']!r}" + ) + + def test_phenio_obojson_to_tsv(): """ Testing transitive propagation of node properties From 5e0fef90c31c5b6e258537325771e24b395d300c Mon Sep 17 00:00:00 2001 From: Kevin Schaper Date: Wed, 27 May 2026 21:04:23 -0700 Subject: [PATCH 2/2] Update test_transform4 edge counts: 205 -> 206 With the get_element_by_mapping(curie) fix, BFO:0000050 maps to biolink:part_of instead of falling through to biolink:related_to. The recovered predicate distinguishes an edge that previously collided on edge-id with another related_to edge between the same nodes, so the post-fix graph correctly preserves one more edge. --- tests/integration/test_stream_transform.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/tests/integration/test_stream_transform.py b/tests/integration/test_stream_transform.py index ee7c70b5..cfe38b61 100644 --- a/tests/integration/test_stream_transform.py +++ b/tests/integration/test_stream_transform.py @@ -393,10 +393,13 @@ def test_transform3(query): "category", ], }, + # 206 (not 205): BFO:0000050 now maps to biolink:part_of via the + # fixed get_element_by_mapping(curie) call, so the part_of edge no + # longer collides on edge-id with a previously-collapsed related_to. 176, - 205, + 206, 176, - 205, + 206, ), ( { @@ -408,9 +411,9 @@ def test_transform3(query): "format": "jsonl", }, 176, - 205, + 206, 176, - 205, + 206, ), ( { @@ -422,9 +425,9 @@ def test_transform3(query): "format": "nt", }, 176, - 205, + 206, 176, - 205, + 206, ), ( { @@ -692,7 +695,7 @@ def test_transform10(): }) assert t1.store.graph.number_of_nodes() == 176 - assert t1.store.graph.number_of_edges() == 205 + assert t1.store.graph.number_of_edges() == 206 @pytest.mark.skipif(