diff --git a/kgx/utils/kgx_utils.py b/kgx/utils/kgx_utils.py index 22776127..f39cc383 100644 --- a/kgx/utils/kgx_utils.py +++ b/kgx/utils/kgx_utils.py @@ -220,6 +220,24 @@ def format_biolink_slots(s: str) -> str: return f"biolink:{formatted}" +def _most_specific(uri: str, primary: List[str], fallback: List[str]) -> List[str]: + """Return whichever candidate list consumed the longest IRI prefix. + + The length of the IRI prefix a CURIE consumed is ``len(uri) - len(local id)``, + so the candidate with the shortest local part is the most specific one. + ``primary`` wins ties, keeping the caller's prefix map canonical. + """ + + def local_len(curie: str) -> int: + return len(curie.split(":", 1)[-1]) + + if not primary: + return fallback + if min(map(local_len, fallback)) < min(map(local_len, primary)): + return fallback + return primary + + def contract( uri: str, prefix_maps: Optional[List[Dict]] = None, fallback: bool = True ) -> str: @@ -252,12 +270,17 @@ def contract( ] if prefix_maps: curie_list = contract_uri(uri, prefix_maps) - if len(curie_list) == 0: - if fallback: - curie_list = contract_uri(uri, default_curie_maps) - if curie_list: - curie = curie_list[0] - else: + if fallback: + # A catch-all namespace in prefix_maps (e.g. biolink's + # OBO -> http://purl.obolibrary.org/obo/) matches every OBO IRI, so + # a bare "did we get any match" test would shadow the far more + # specific mappings the default maps carry (DDPHENO_, FBbt_, ...). + # Consider both tiers and keep the most specific match, i.e. the one + # that consumed the longest IRI prefix; prefix_maps wins a tie. + fallback_list = contract_uri(uri, default_curie_maps) + if fallback_list: + curie_list = _most_specific(uri, curie_list, fallback_list) + if curie_list: curie = curie_list[0] else: curie_list = contract_uri(uri, default_curie_maps) diff --git a/tests/unit/test_kgx_utils.py b/tests/unit/test_kgx_utils.py index 68a9c053..f2e0fbd0 100644 --- a/tests/unit/test_kgx_utils.py +++ b/tests/unit/test_kgx_utils.py @@ -398,3 +398,64 @@ def test_sanitize_export_property(query): assert query[1] == value else: assert query[1] in value + + +# A catch-all namespace, i.e. one whose IRI is a bare prefix of many more +# specific namespaces. biolink's `OBO -> http://purl.obolibrary.org/obo/` is the +# real-world example that motivated these tests. +CATCH_ALL_PREFIX_MAP = {"OBO": "http://purl.obolibrary.org/obo/"} + + +@pytest.mark.parametrize( + "uri,expected", + [ + # obo_context has a DDPHENO_ mapping; the catch-all must not shadow it. + ("http://purl.obolibrary.org/obo/DDPHENO_0000001", "DDPHENO:0000001"), + ("http://purl.obolibrary.org/obo/FBbt_00000001", "FBbt:00000001"), + ("http://purl.obolibrary.org/obo/EMAPA_16040", "EMAPA:16040"), + # Nothing more specific exists for these, so the catch-all still applies. + ( + "http://purl.obolibrary.org/obo/fbbt#has_function_in", + "OBO:fbbt#has_function_in", + ), + ( + "http://purl.obolibrary.org/obo/go/extensions/ro_0002092", + "OBO:go/extensions/ro_0002092", + ), + ], +) +def test_contract_catch_all_does_not_shadow_specific_mapping(uri, expected): + """ + Test that a catch-all namespace in prefix_maps does not suppress the fallback. + + A catch-all matches every IRI beneath it, so gating the fallback on "did + prefix_maps match anything" means a more specific mapping in the default + maps is never reached. + """ + assert contract(uri, prefix_maps=[CATCH_ALL_PREFIX_MAP], fallback=True) == expected + + +def test_contract_prefix_maps_win_ties(): + """ + Test that prefix_maps stays canonical when it is as specific as the fallback. + """ + # obo_context maps GO -> http://purl.obolibrary.org/obo/GO_; an equally + # specific caller-supplied mapping must not be overridden by it. + curie = contract( + "http://purl.obolibrary.org/obo/GO_0008150", + prefix_maps=[{"GENE_ONTOLOGY": "http://purl.obolibrary.org/obo/GO_"}], + fallback=True, + ) + assert curie == "GENE_ONTOLOGY:0008150" + + +def test_contract_catch_all_is_kept_without_fallback(): + """ + Test that fallback=False still contracts using only the given prefix_maps. + """ + curie = contract( + "http://purl.obolibrary.org/obo/DDPHENO_0000001", + prefix_maps=[CATCH_ALL_PREFIX_MAP], + fallback=False, + ) + assert curie == "OBO:DDPHENO_0000001" diff --git a/tests/unit/test_prefix_manager.py b/tests/unit/test_prefix_manager.py index 8a4c1778..f22ee0d8 100644 --- a/tests/unit/test_prefix_manager.py +++ b/tests/unit/test_prefix_manager.py @@ -120,3 +120,38 @@ def test_prefix_manager_contract(query): """ pm = PrefixManager() assert pm.contract(query[0]) == query[1] + + +@pytest.mark.parametrize( + "query", + [ + # OBO ontologies with no entry of their own in the JSON-LD context. The + # context's catch-all `OBO -> http://purl.obolibrary.org/obo/` matches + # them, and must not suppress the specific mappings in obo_context. + ("http://purl.obolibrary.org/obo/DDPHENO_0000001", "DDPHENO:0000001"), + ("http://purl.obolibrary.org/obo/FBbt_00000001", "FBbt:00000001"), + ("http://purl.obolibrary.org/obo/EMAPA_16040", "EMAPA:16040"), + ("http://purl.obolibrary.org/obo/WBbt_0005733", "WBbt:0005733"), + ("http://purl.obolibrary.org/obo/ZFA_0000001", "ZFA:0000001"), + ("http://purl.obolibrary.org/obo/XAO_0000001", "XAO:0000001"), + ("http://purl.obolibrary.org/obo/ZFS_0000001", "ZFS:0000001"), + ("http://purl.obolibrary.org/obo/CHR_0000001", "CHR:0000001"), + # Ontologies that do have their own context entry are unaffected. + ("http://purl.obolibrary.org/obo/HP_0000001", "HP:0000001"), + ("http://purl.obolibrary.org/obo/MONDO_0000001", "MONDO:0000001"), + ("http://purl.obolibrary.org/obo/RO_0002162", "RO:0002162"), + ("http://purl.obolibrary.org/obo/BFO_0000050", "BFO:0000050"), + # IRIs beneath the OBO namespace that no per-ontology mapping covers + # keep the catch-all, as before. + ( + "http://purl.obolibrary.org/obo/fbbt#has_function_in", + "OBO:fbbt#has_function_in", + ), + ], +) +def test_prefix_manager_contract_obo_idspaces(query): + """ + Test that the OBO catch-all does not shadow per-ontology mappings. + """ + pm = PrefixManager() + assert pm.contract(query[0]) == query[1]