From bdec073f67c08171b0c2995704b85fcf60e91467 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Genevi=C3=A8ve=20Fleury?= <113036747+gefleury@users.noreply.github.com> Date: Thu, 16 Jul 2026 14:44:53 +0200 Subject: [PATCH 1/5] Deduplicate by_name(all=True) results by_name(..., all=True) could return the same instance more than once, for two reasons: - an instance's own name-like properties (including synonyms) sharing a value (e.g. MolecularEntity.propofol) - match="contains" reaching the same instance via several distinct keys (e.g. searching SovereignState for "FR" matches France via both its "FR" and "FRA" synonyms) Deduplication preserves the original order, so the first match (all=False) is unaffected. --- .../src/additional_methods/by_name.py.txt | 4 ++-- pipeline/tests/test_regressions.py | 21 +++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/pipeline/src/additional_methods/by_name.py.txt b/pipeline/src/additional_methods/by_name.py.txt index 1dcd7cd7..c1a8974f 100644 --- a/pipeline/src/additional_methods/by_name.py.txt +++ b/pipeline/src/additional_methods/by_name.py.txt @@ -46,9 +46,9 @@ matches.extend(instances) else: raise ValueError("'match' must be either 'equals' or 'contains'") - if not matches: + if not matches: return None elif all: - return matches + return list(dict.fromkeys(matches)) else: return matches[0] \ No newline at end of file diff --git a/pipeline/tests/test_regressions.py b/pipeline/tests/test_regressions.py index c172166a..43ecd2db 100644 --- a/pipeline/tests/test_regressions.py +++ b/pipeline/tests/test_regressions.py @@ -638,3 +638,24 @@ def test_issue0094_resolve_links_tolerates_missing_id(om): assert parents[0] is present # resolvable link replaced by the typed object assert isinstance(parents[1], Link) # unresolvable link kept, no KeyError assert parents[1].identifier == "http://example.org/pe/missing" + + +@pytest.mark.parametrize("om", [openminds.latest]) +def test_by_name_deduplicates_matches(om): + # by_name(..., all=True) must not return the same instance more than once + # Two cases: + # (a) an instance whose name-like properties (which include "synonyms") share + # the same value gets indexed twice under that value's key while the lookup + # is built (e.g. MolecularEntity "propofol" lists "propofol" itself as one + # of its own synonyms). + # (b) match="contains" can find the same instance through several distinct, + # overlapping keys (e.g. SovereignState "France" has synonyms "FR" and + # "FRA", both of which contain the substring "FR"). + + MolecularEntity = om.controlled_terms.MolecularEntity + propofol_matches = MolecularEntity.by_name("propofol", all=True) + assert len(propofol_matches) == 1 + + SovereignState = om.controlled_terms.SovereignState + fr_matches = SovereignState.by_name("FR", match="contains", all=True) + assert len(set(m.id for m in fr_matches)) == len(fr_matches) # no instance repeated From af66572905445d4c6326ce1e3d7e41a536e6d6fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Genevi=C3=A8ve=20Fleury?= <113036747+gefleury@users.noreply.github.com> Date: Thu, 16 Jul 2026 15:41:09 +0200 Subject: [PATCH 2/5] Fix by_name() crash on unset properties - hasattr(instance, prop_name) only checks whether the class defines the property, not whether this instance actually set it (every instance shares the same __init__ signature, with unset properties defaulting to None). - As a result, an unset property (e.g. ParcellationEntity without an abbreviation) was still indexed, leaving None as a lookup key. - match="contains" then crashed with TypeError, since `name in key` is invalid when key is None. - The fix only appends a name-like property to the lookup keys when its value is not None. --- pipeline/src/additional_methods/by_name.py.txt | 5 +++-- pipeline/tests/test_regressions.py | 11 +++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/pipeline/src/additional_methods/by_name.py.txt b/pipeline/src/additional_methods/by_name.py.txt index c1a8974f..7f4750e4 100644 --- a/pipeline/src/additional_methods/by_name.py.txt +++ b/pipeline/src/additional_methods/by_name.py.txt @@ -27,8 +27,9 @@ for instance in cls.instances(): keys = [] for prop_name in namelike_properties: - if hasattr(instance, prop_name): - keys.append(getattr(instance, prop_name)) + value = getattr(instance, prop_name, None) + if value is not None: + keys.append(value) if hasattr(instance, "synonyms"): for synonym in instance.synonyms or []: keys.append(synonym) diff --git a/pipeline/tests/test_regressions.py b/pipeline/tests/test_regressions.py index 43ecd2db..42b47f17 100644 --- a/pipeline/tests/test_regressions.py +++ b/pipeline/tests/test_regressions.py @@ -659,3 +659,14 @@ def test_by_name_deduplicates_matches(om): SovereignState = om.controlled_terms.SovereignState fr_matches = SovereignState.by_name("FR", match="contains", all=True) assert len(set(m.id for m in fr_matches)) == len(fr_matches) # no instance repeated + + +@pytest.mark.parametrize("om", [openminds.latest]) +def test_by_name_tolerates_unset_namelike_properties(om): + # ParcellationEntity has real instances that leave "abbreviation" unset. + # Before the fix, an unset property was still indexed, leaving None as + # a lookup key, which crashed match="contains" (`name in None`). + ParcellationEntity = om.sands.ParcellationEntity + + ParcellationEntity.by_name("brain", match="contains", all=True) # would previously raise TypeError + assert None not in ParcellationEntity._instance_lookup From 70ce3e8027d38aeb8723a47d587259d76101777a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Genevi=C3=A8ve=20Fleury?= <113036747+gefleury@users.noreply.github.com> Date: Thu, 16 Jul 2026 17:33:15 +0200 Subject: [PATCH 3/5] Add case_sensitive option to by_name() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - by_name() gains a case_sensitive parameter (default True, preserving current behavior). - When False, equals/contains comparisons use casefold() instead of lower(), so Unicode variants like the micro sign "µ" and the Greek letter "μ" are also treated as equivalent. - Case-insensitive matching can merge results from genuinely different instances that only differ by case (e.g. MolecularEntity "pentobarbital" vs "pentobarbital sodium"). - Add tests reusing test_issue0069's License examples with flipped case, plus the MolecularEntity merge case and the Unicode casefold case. --- .../src/additional_methods/by_name.py.txt | 16 +++++++- pipeline/tests/test_regressions.py | 38 +++++++++++++++++++ 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/pipeline/src/additional_methods/by_name.py.txt b/pipeline/src/additional_methods/by_name.py.txt index 7f4750e4..2b904fe3 100644 --- a/pipeline/src/additional_methods/by_name.py.txt +++ b/pipeline/src/additional_methods/by_name.py.txt @@ -8,6 +8,7 @@ name: str, match: str = "equals", all: bool = False, + case_sensitive: bool = True, ): """ Search for instances in the openMINDS instance library based on their name. @@ -20,6 +21,7 @@ name (str): a string to search for. match (str, optional): either "equals" (exact match - default) or "contains". all (bool, optional): Whether to return all objects that match the name, or only the first. Defaults to False. + case_sensitive (bool, optional): Whether the search should be case-sensitive. Defaults to True. """ namelike_properties = ("name", "lookup_label", "family_name", "full_name", "short_name", "abbreviation") if cls._instance_lookup is None: @@ -38,12 +40,22 @@ cls._instance_lookup[key].append(instance) else: cls._instance_lookup[key] = [instance] + + def normalize(s): + return s if case_sensitive else s.casefold() + if match == "equals": - matches = cls._instance_lookup.get(name, []) + if case_sensitive: + matches = cls._instance_lookup.get(name, []) + else: + matches = [] + for key, instances in cls._instance_lookup.items(): + if key.casefold() == name.casefold(): + matches.extend(instances) elif match == "contains": matches = [] for key, instances in cls._instance_lookup.items(): - if name in key: + if normalize(name) in normalize(key): matches.extend(instances) else: raise ValueError("'match' must be either 'equals' or 'contains'") diff --git a/pipeline/tests/test_regressions.py b/pipeline/tests/test_regressions.py index 42b47f17..ff2242bf 100644 --- a/pipeline/tests/test_regressions.py +++ b/pipeline/tests/test_regressions.py @@ -670,3 +670,41 @@ def test_by_name_tolerates_unset_namelike_properties(om): ParcellationEntity.by_name("brain", match="contains", all=True) # would previously raise TypeError assert None not in ParcellationEntity._instance_lookup + + +@pytest.mark.parametrize("om", [openminds.latest]) +def test_by_name_case_sensitive(om): + # by_name(..., case_sensitive=False) must match regardless of case, while + # the default (case_sensitive=True) stays exact, as in test_issue0069. + License = om.core.License + + # match="equals" + assert License.by_name("cc-by-4.0") is None + result = License.by_name("cc-by-4.0", case_sensitive=False) + assert result is License.by_name("CC-BY-4.0") + + # match="contains" + assert License.by_name("creative commons", match="contains", all=True) is None + results = License.by_name("creative commons", match="contains", all=True, case_sensitive=False) + expected = License.by_name("Creative Commons", match="contains", all=True) + assert set(r.id for r in results) == set(r.id for r in expected) + + # case-insensitive matching can also merge results from genuinely different + # instances, not just recover a single missed match: MolecularEntity + # "pentobarbital sodium" is its own instance's name, while "pentobarbital + # Sodium" (capital S) is a synonym of a different instance ("pentobarbital"). + MolecularEntity = om.controlled_terms.MolecularEntity + exact_matches = MolecularEntity.by_name("pentobarbital sodium", all=True) + assert set(m.name for m in exact_matches) == {"pentobarbital sodium"} + merged_matches = MolecularEntity.by_name("pentobarbital sodium", all=True, case_sensitive=False) + assert set(m.name for m in merged_matches) == {"pentobarbital", "pentobarbital sodium"} + + # case-insensitive matching uses casefold(), not lower(), so it also unifies + # Unicode variants such as the micro sign "µ" (U+00B5) and the Greek letter + # "μ" (U+03BC) - e.g. UnitOfMeasurement "microampere" has synonym "µA" (micro + # sign), which a search for the Greek-mu spelling should still find. + UnitOfMeasurement = om.controlled_terms.UnitOfMeasurement + greek_mu_a = "μA" # Greek mu, not the micro sign used in the real data + assert UnitOfMeasurement.by_name(greek_mu_a) is None + match = UnitOfMeasurement.by_name(greek_mu_a, case_sensitive=False) + assert match.name == "microampere" From 056e80ef09306bc9ee5ba485cc024c250909b137 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Genevi=C3=A8ve=20Fleury?= <113036747+gefleury@users.noreply.github.com> Date: Thu, 16 Jul 2026 18:34:37 +0200 Subject: [PATCH 4/5] Add match="contained" option to by_name() by_name() gains match="contained", the reverse of "contains": finds instances whose name-like properties are substrings of the given string. Useful for NWB files: their fields often combine a canonical name with extra free text (e.g. "Mus musculus - House mouse"), embedding the openMINDS instance name inside a longer, non-canonical string. --- pipeline/src/additional_methods/by_name.py.txt | 11 +++++++++-- pipeline/tests/test_regressions.py | 16 ++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/pipeline/src/additional_methods/by_name.py.txt b/pipeline/src/additional_methods/by_name.py.txt index 2b904fe3..363a9ee7 100644 --- a/pipeline/src/additional_methods/by_name.py.txt +++ b/pipeline/src/additional_methods/by_name.py.txt @@ -19,7 +19,9 @@ Args: name (str): a string to search for. - match (str, optional): either "equals" (exact match - default) or "contains". + match (str, optional): either "equals" (exact match - default), "contains" + (the name-like property contains the given string), or "contained" + (the given string contains the name-like property). all (bool, optional): Whether to return all objects that match the name, or only the first. Defaults to False. case_sensitive (bool, optional): Whether the search should be case-sensitive. Defaults to True. """ @@ -57,8 +59,13 @@ for key, instances in cls._instance_lookup.items(): if normalize(name) in normalize(key): matches.extend(instances) + elif match == "contained": + matches = [] + for key, instances in cls._instance_lookup.items(): + if normalize(key) in normalize(name): + matches.extend(instances) else: - raise ValueError("'match' must be either 'equals' or 'contains'") + raise ValueError("'match' must be either 'equals', 'contains', or 'contained'") if not matches: return None elif all: diff --git a/pipeline/tests/test_regressions.py b/pipeline/tests/test_regressions.py index ff2242bf..3dff77ba 100644 --- a/pipeline/tests/test_regressions.py +++ b/pipeline/tests/test_regressions.py @@ -708,3 +708,19 @@ def test_by_name_case_sensitive(om): assert UnitOfMeasurement.by_name(greek_mu_a) is None match = UnitOfMeasurement.by_name(greek_mu_a, case_sensitive=False) assert match.name == "microampere" + + +@pytest.mark.parametrize("om", [openminds.latest]) +def test_by_name_match_contained(om): + # match="contained" is the reverse of match="contains": it looks for + # instances whose name-like properties are substrings of the given + # (typically longer/composite) search string, rather than the other way around. + Species = om.controlled_terms.Species + + result = Species.by_name("Mus musculus - House mouse", match="contained") + assert result.name == "Mus musculus" + + # Several macaque species' full names contain "Macaca" as a substring, + # but none of those full names is itself a substring of "Macaca". + assert Species.by_name("Macaca", match="contains", all=True) is not None + assert Species.by_name("Macaca", match="contained", all=True) is None From 9bfaf7dab38a5944d369a746a3cff3b0c298f088 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Genevi=C3=A8ve=20Fleury?= <113036747+gefleury@users.noreply.github.com> Date: Fri, 17 Jul 2026 15:36:10 +0200 Subject: [PATCH 5/5] Rename by_name() tests to match the test_pr convention --- pipeline/tests/test_regressions.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/pipeline/tests/test_regressions.py b/pipeline/tests/test_regressions.py index 3dff77ba..a6f8212a 100644 --- a/pipeline/tests/test_regressions.py +++ b/pipeline/tests/test_regressions.py @@ -641,7 +641,8 @@ def test_issue0094_resolve_links_tolerates_missing_id(om): @pytest.mark.parametrize("om", [openminds.latest]) -def test_by_name_deduplicates_matches(om): +def test_pr0100_by_name_deduplicates_matches(om): + # https://github.com/openMetadataInitiative/openMINDS_Python/pull/100 # by_name(..., all=True) must not return the same instance more than once # Two cases: # (a) an instance whose name-like properties (which include "synonyms") share @@ -662,7 +663,8 @@ def test_by_name_deduplicates_matches(om): @pytest.mark.parametrize("om", [openminds.latest]) -def test_by_name_tolerates_unset_namelike_properties(om): +def test_pr0100_by_name_tolerates_unset_namelike_properties(om): + # https://github.com/openMetadataInitiative/openMINDS_Python/pull/100 # ParcellationEntity has real instances that leave "abbreviation" unset. # Before the fix, an unset property was still indexed, leaving None as # a lookup key, which crashed match="contains" (`name in None`). @@ -673,7 +675,8 @@ def test_by_name_tolerates_unset_namelike_properties(om): @pytest.mark.parametrize("om", [openminds.latest]) -def test_by_name_case_sensitive(om): +def test_pr0100_by_name_case_sensitive(om): + # https://github.com/openMetadataInitiative/openMINDS_Python/pull/100 # by_name(..., case_sensitive=False) must match regardless of case, while # the default (case_sensitive=True) stays exact, as in test_issue0069. License = om.core.License @@ -711,7 +714,8 @@ def test_by_name_case_sensitive(om): @pytest.mark.parametrize("om", [openminds.latest]) -def test_by_name_match_contained(om): +def test_pr0100_by_name_match_contained(om): + # https://github.com/openMetadataInitiative/openMINDS_Python/pull/100 # match="contained" is the reverse of match="contains": it looks for # instances whose name-like properties are substrings of the given # (typically longer/composite) search string, rather than the other way around.