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/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( 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