From e96bc51d57a80829459bd8570996d1930fbc4d0f Mon Sep 17 00:00:00 2001 From: jdsika Date: Tue, 20 Oct 2026 23:24:14 +0200 Subject: [PATCH 1/3] perf: skip quadratic curies duplicate check for valid contexts Context.as_converter() let curies run its strict duplicate check, which compares every pair of records. For each merged context (4,273 records) that takes 15-25 seconds, so load_converter("merged") is slow for every downstream user (see #75 and monarch-app#819). Scan the extended prefix map for shared prefixes or URI prefixes in linear time, and build the converter with strict=False when there are none. When duplicates do exist, keep strict=True so curies still raises DuplicatePrefixes or DuplicateURIPrefixes with its usual message. load_converter("merged") now takes about 0.3 seconds, and converters for all bundled contexts build in about one second in total. Add tests that every bundled context builds a converter, and that each kind of shared prefix or URI prefix (canonical or synonym) is still rejected. Fixes #75 Signed-off-by: jdsika --- src/prefixmaps/datamodel/context.py | 29 ++++++++++++- tests/test_core/test_curies.py | 66 ++++++++++++++++++++++++++++- 2 files changed, 93 insertions(+), 2 deletions(-) diff --git a/src/prefixmaps/datamodel/context.py b/src/prefixmaps/datamodel/context.py index e286f60..a7f9648 100644 --- a/src/prefixmaps/datamodel/context.py +++ b/src/prefixmaps/datamodel/context.py @@ -381,7 +381,11 @@ def as_converter(self) -> curies.Converter: :return: """ extended_prefix_map = self.as_extended_prefix_map() - return curies.Converter.from_extended_prefix_map(extended_prefix_map) + # curies' strict check compares every pair of records, which takes 15-25 seconds + # per merged context, see https://github.com/linkml/prefixmaps/issues/75. + # Only run it when a linear scan finds duplicates, so curies raises its usual error. + strict = _has_duplicates(extended_prefix_map) + return curies.Converter.from_extended_prefix_map(extended_prefix_map, strict=strict) def validate(self, canonical_only=True) -> List[str]: """ @@ -398,3 +402,26 @@ def validate(self, canonical_only=True) -> List[str]: # namespaces = self.namespaces(lower=True) # prefixes = self.namespaces(lower=True) return messages + + +def _has_duplicates(records: List[curies.Record]) -> bool: + """ + Check whether any two records share a prefix or a URI prefix. + + This finds the same duplicates as curies' strict check, but in linear time. + + :param records: records of an extended prefix map + :return: True if a prefix or URI prefix appears in more than one record + """ + prefixes = set() + uri_prefixes = set() + for record in records: + record_prefixes = {record.prefix, *record.prefix_synonyms} + record_uri_prefixes = {record.uri_prefix, *record.uri_prefix_synonyms} + if not prefixes.isdisjoint(record_prefixes): + return True + if not uri_prefixes.isdisjoint(record_uri_prefixes): + return True + prefixes |= record_prefixes + uri_prefixes |= record_uri_prefixes + return False diff --git a/tests/test_core/test_curies.py b/tests/test_core/test_curies.py index b41edfd..0b6d773 100644 --- a/tests/test_core/test_curies.py +++ b/tests/test_core/test_curies.py @@ -2,9 +2,11 @@ import unittest -from curies import Converter +from curies import Converter, DuplicatePrefixes, DuplicateURIPrefixes import prefixmaps +from prefixmaps.data import context_paths +from prefixmaps.datamodel.context import Context, PrefixExpansion, StatusType class TestCuries(unittest.TestCase): @@ -37,3 +39,65 @@ def test_load(self): self.assertIn(uri_prefix_2, converter.reverse_prefix_map) self.assertEqual(prefix, converter.reverse_prefix_map[uri_prefix_1]) self.assertEqual(prefix, converter.reverse_prefix_map[uri_prefix_2]) + + def test_load_all(self): + """Test that every bundled context can be loaded into a converter.""" + for key in context_paths: + with self.subTest(key=key): + self.assertIsInstance(prefixmaps.load_converter(key), Converter) + + def test_duplicate_uri_prefixes(self): + """Test that two records sharing a URI prefix are still rejected.""" + context = Context("test") + context.prefix_expansions = [ + PrefixExpansion("test", "a", "https://example.org/a/", StatusType.canonical), + PrefixExpansion("test", "b", "https://example.org/a/", StatusType.canonical), + ] + with self.assertRaises(DuplicateURIPrefixes): + context.as_converter() + + def test_duplicate_prefixes(self): + """Test that two records sharing a prefix synonym are still rejected.""" + context = Context("test") + context.prefix_expansions = [ + PrefixExpansion("test", "a", "https://example.org/a/", StatusType.canonical), + PrefixExpansion("test", "b", "https://example.org/b/", StatusType.canonical), + PrefixExpansion("test", "c", "https://example.org/a/", StatusType.namespace_alias), + PrefixExpansion("test", "c", "https://example.org/b/", StatusType.namespace_alias), + ] + with self.assertRaises(DuplicatePrefixes): + context.as_converter() + + def test_duplicate_uri_prefix_synonyms(self): + """Test that a URI prefix synonym shared with another record is still rejected.""" + context = Context("test") + context.prefix_expansions = [ + PrefixExpansion("test", "a", "https://example.org/a/", StatusType.canonical), + PrefixExpansion("test", "b", "https://example.org/b/", StatusType.canonical), + PrefixExpansion("test", "b", "https://example.org/a/", StatusType.prefix_alias), + ] + with self.assertRaises(DuplicateURIPrefixes): + context.as_converter() + + def test_duplicate_canonical_prefix_synonym(self): + """Test that a prefix synonym equal to another record's prefix is still rejected.""" + context = Context("test") + context.prefix_expansions = [ + PrefixExpansion("test", "a", "https://example.org/a/", StatusType.canonical), + PrefixExpansion("test", "b", "https://example.org/b/", StatusType.canonical), + PrefixExpansion("test", "a", "https://example.org/b/", StatusType.namespace_alias), + ] + with self.assertRaises(DuplicatePrefixes): + context.as_converter() + + def test_duplicate_shared_uri_prefix_synonym(self): + """Test that two records sharing a URI prefix synonym are still rejected.""" + context = Context("test") + context.prefix_expansions = [ + PrefixExpansion("test", "a", "https://example.org/a/", StatusType.canonical), + PrefixExpansion("test", "b", "https://example.org/b/", StatusType.canonical), + PrefixExpansion("test", "a", "https://example.org/z/", StatusType.prefix_alias), + PrefixExpansion("test", "b", "https://example.org/z/", StatusType.prefix_alias), + ] + with self.assertRaises(DuplicateURIPrefixes): + context.as_converter() From 1b829fe0c147204f31eeee66f1d8c30961818b66 Mon Sep 17 00:00:00 2001 From: jdsika Date: Tue, 20 Oct 2026 23:24:15 +0200 Subject: [PATCH 2/3] test: check canonical uniqueness in merged contexts too The integrity tests skipped merged, merged.monarch and merged.oak entirely. Their canonical expansions are bijective, so run the two canonical-uniqueness tests on every bundled context. The alias tests still skip the merged contexts, because those still contain aliases without a matching canonical record (#26). Follow-up to the discussion in #11. Signed-off-by: jdsika --- tests/test_core/test_integrity.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/tests/test_core/test_integrity.py b/tests/test_core/test_integrity.py index 0bd4c91..d8ffbb4 100644 --- a/tests/test_core/test_integrity.py +++ b/tests/test_core/test_integrity.py @@ -14,11 +14,12 @@ class TestIntegrity(unittest.TestCase): def setUp(self) -> None: """Set up the test case with all contexts.""" - # We don't expect some merged contexts to pass all tests, see: - # See: https://github.com/linkml/prefixmaps/issues/26 + self.contexts: Mapping[str, Context] = {key: load_context(key) for key in context_paths} + # We don't expect merged contexts to pass the alias tests, see: + # https://github.com/linkml/prefixmaps/issues/26 skip = {"merged", "merged.oak", "merged.monarch"} - self.contexts: Mapping[str, Context] = { - key: load_context(key) for key, path in context_paths.items() if key not in skip + self.alias_contexts: Mapping[str, Context] = { + key: context for key, context in self.contexts.items() if key not in skip } def test_unique_canonical_prefix(self): @@ -56,7 +57,7 @@ def test_unique_canonical_namespace(self): def test_prefix_aliases(self): """Test that namespace aliases have a valid prefix.""" - for key, context in self.contexts.items(): + for key, context in self.alias_contexts.items(): with self.subTest(key=key): canonical_prefixes = { expansion.prefix @@ -78,7 +79,7 @@ def test_prefix_aliases(self): def test_namespace_aliases(self): """Test that prefix aliases have a valid namespace.""" - for key, context in self.contexts.items(): + for key, context in self.alias_contexts.items(): with self.subTest(key=key): canonical_namespaces = { expansion.namespace: expansion.prefix From eb909fb4ef3b0fc4d1ff36466985a77ce13542b0 Mon Sep 17 00:00:00 2001 From: jdsika Date: Tue, 20 Oct 2026 23:24:15 +0200 Subject: [PATCH 3/3] docs: build converters with Context.as_converter() The examples built converters with Converter.from_extended_prefix_map(context.as_extended_prefix_map()), which runs the slow strict duplicate check. Use as_converter() instead. This also fixes examples that assigned ctxt but read context, one line repeated three times, and the missing load_context import. Signed-off-by: jdsika --- docs/intro.md | 23 +++++++---------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/docs/intro.md b/docs/intro.md index 8161ec8..5682205 100644 --- a/docs/intro.md +++ b/docs/intro.md @@ -28,12 +28,10 @@ pip install prefixmaps to use in combination with [curies](https://github.com/cthoyt/curies) library: ```python -from prefixmaps.io.parser import load_multi_context -from curies import Converter +from prefixmaps.io.parser import load_context, load_multi_context context = load_multi_context(["obo", "bioregistry.upper", "linked_data", "prefixcc"]) -extended_prefix_map = context.as_extended_prefix_map() -converter = Converter.from_extended_prefix_map(extended_prefix_map) +converter = context.as_converter() >>> converter.expand("CHEBI:1") 'http://purl.obolibrary.org/obo/CHEBI_1' @@ -55,8 +53,7 @@ If we prioritize prefix.cc the OBO prefix is ignored: ```python >>> ctxt = load_multi_context(["prefixcc", "obo"]) ->>> extended_prefix_map = context.as_extended_prefix_map() ->>> converter = Converter.from_extended_prefix_map(extended_prefix_map) +>>> converter = ctxt.as_converter() >>> converter.expand("GEO:1") >>> converter.expand("geo:1") 'http://www.opengis.net/ont/geosparql#1' @@ -68,8 +65,7 @@ If we push bioregistry at the start of the list then GEOGEO can be used as the p ```python >>> ctxt = load_multi_context(["bioregistry", "prefixcc", "obo"]) ->>> extended_prefix_map = context.as_extended_prefix_map() ->>> converter = Converter.from_extended_prefix_map(extended_prefix_map) +>>> converter = ctxt.as_converter() >>> converter.expand("geo:1") 'http://identifiers.org/geo/1' >>> converter.expand("GEO:1") @@ -83,10 +79,7 @@ We get similar results using the upper-normalized variant of bioregistry: ```python >>> ctxt = load_multi_context(["bioregistry.upper", "prefixcc", "obo"]) ->>> extended_prefix_map = context.as_extended_prefix_map() ->>> converter = Converter.from_extended_prefix_map(extended_prefix_map) ->>> converter = Converter.from_extended_prefix_map(extended_prefix_map) ->>> converter = Converter.from_extended_prefix_map(extended_prefix_map) +>>> converter = ctxt.as_converter() >>> converter.expand("GEO:1") 'http://identifiers.org/geo/1' >>> converter.expand("geo:1") @@ -98,8 +91,7 @@ Users of OBO ontologies will want to place OBO at the start of the list: ```python >>> ctxt = load_multi_context(["obo", "bioregistry.upper", "prefixcc"]) ->>> extended_prefix_map = context.as_extended_prefix_map() ->>> converter = Converter.from_extended_prefix_map(extended_prefix_map) +>>> converter = ctxt.as_converter() >>> converter.expand("geo:1") >>> converter.expand("GEO:1") 'http://purl.obolibrary.org/obo/GEO_1' @@ -114,8 +106,7 @@ You can use the ready-made "merged" prefix set, which prioritizes OBO: ```python >>> ctxt = load_context("merged") ->>> extended_prefix_map = context.as_extended_prefix_map() ->>> converter = Converter.from_extended_prefix_map(extended_prefix_map) +>>> converter = ctxt.as_converter() >>> converter.expand("GEOGEO:1") >>> converter.expand("GEO:1") 'http://purl.obolibrary.org/obo/GEO_1'