From f64d1b9f059fccee8ec538d93379365b3baf881d Mon Sep 17 00:00:00 2001 From: Andrew Davison Date: Fri, 16 Oct 2026 11:04:58 +0200 Subject: [PATCH 1/2] Fix expand_filter() dropping filters that share a prefix When several filters shared a prefix, e.g. `developers__given_name` and `developers__family_name`, each one replaced the nested dict created by the previous one, so all but the last were silently dropped. The nested dicts are now merged. --- fairgraph/utility/jsonld.py | 3 +-- test/test_utility.py | 8 ++++++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/fairgraph/utility/jsonld.py b/fairgraph/utility/jsonld.py index 0a55e813..a7b3ce72 100644 --- a/fairgraph/utility/jsonld.py +++ b/fairgraph/utility/jsonld.py @@ -219,7 +219,6 @@ def expand_filter(filter_dict: Dict[str, Any]): local_path = expanded parts = key.split("__") for part in parts[:-1]: - local_path[part] = {} - local_path = local_path[part] + local_path = local_path.setdefault(part, {}) local_path[parts[-1]] = value return expanded diff --git a/test/test_utility.py b/test/test_utility.py index 36f5208f..a31c600f 100644 --- a/test/test_utility.py +++ b/test/test_utility.py @@ -27,6 +27,14 @@ def test_expand_filter(): assert result == expected +def test_expand_filter_keeps_siblings(): + filter = { + "developers__given_name": "Bilbo", + "developers__family_name": "Baggins", + } + assert expand_filter(filter) == {"developers": {"given_name": "Bilbo", "family_name": "Baggins"}} + + def test_compact_uri(): context = {"foaf": "http://xmlns.com/foaf/0.1/"} uri_list = "http://xmlns.com/foaf/0.1/Person" From c35d41d7b25fe8b25bd9fe1de13f3fbf89827490 Mon Sep 17 00:00:00 2001 From: Andrew Davison Date: Fri, 16 Oct 2026 11:04:58 +0200 Subject: [PATCH 2/2] Support lists of values in query filters A list of filter values, e.g. `SoftwareVersion.list(client, developers__family_name=["Baggins", "Oakenshield"])`, is now expressed in the generated KG query by repeating the top-level property once per value, each copy being required and carrying a single value. A result must therefore match every value, in any order. Previously the list was sent as the value of a single filter, which the KG rejects with a 500 error, and a list of linked objects raised an AttributeError. A one-item list is treated as a single value. An empty list, or more than one list within the same top-level property, raises a ValueError. A list of ID strings is now accepted for a link property, as a single ID string already was. --- fairgraph/node.py | 20 +++++++++- fairgraph/queries.py | 53 +++++++++++++++++++++--- test/test_queries.py | 95 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 160 insertions(+), 8 deletions(-) diff --git a/fairgraph/node.py b/fairgraph/node.py index 1969084b..6f699d31 100644 --- a/fairgraph/node.py +++ b/fairgraph/node.py @@ -26,7 +26,14 @@ from .base import Resolvable, ErrorHandling, Releasable from .kgproxy import KGProxy from .kgquery import KGQuery -from .queries import Equals, QueryProperty, get_query_properties, get_query_filter_property, get_filter_value +from .queries import ( + Equals, + QueryProperty, + get_query_properties, + get_query_filter_property, + get_filter_value, + to_single_value_filters, +) from .errors import ResolutionFailure, CannotBuildExistenceQuery from .utility import ( as_list, # temporary for backwards compatibility (a lot of code imports it from here) @@ -303,7 +310,16 @@ def generate_query_filter_properties( properties = [] for prop in cls.all_properties: if prop.name in filters: - properties.append(get_query_filter_property(prop, cls.context, filters[prop.name])) + # the KG accepts one value per filter, so a filter containing a list of values + # becomes one copy of the property per value, each of which must match + single_value_filters = to_single_value_filters(filters[prop.name]) + if len(single_value_filters) == 1: + properties.append(get_query_filter_property(prop, cls.context, single_value_filters[0])) + else: + for i, single_value_filter in enumerate(single_value_filters, start=1): + properties.append( + get_query_filter_property(prop, cls.context, single_value_filter, name=f"Q{prop.name}{i}") + ) return properties @classmethod diff --git a/fairgraph/queries.py b/fairgraph/queries.py index 2888289a..76ce2c5c 100644 --- a/fairgraph/queries.py +++ b/fairgraph/queries.py @@ -75,7 +75,8 @@ class Equals(str): The KG ignores case when matching, but not whitespace, so `Equals("FOO")` does match "foo", but not "FOO ". - Only a single value is supported: a list of `Equals` values is filtered with "CONTAINS". + As with any filter value, a list of `Equals` values matches only if every item is matched, + each with "EQUALS". Args: value (str): the value the property must be equal to. @@ -484,12 +485,50 @@ def get_query_properties( return properties -def get_query_filter_property(property, context, filter: Any) -> QueryProperty: +def to_single_value_filters(filter: Any) -> List[Any]: + """ + Return one or more filters equivalent to `filter`, each with a single value at every leaf. + + A filter that contains no lists is returned unchanged, as the only element of the result. + The KG query API accepts only a single value per filter, so a filter containing a list of values + is expressed by repeating the property once per item, each copy being required: in that case + one filter is returned per item. The list may be nested within a dict of filters for a linked + or embedded node, in which case each copy of the dict contains one of the items. + + Example: + >>> to_single_value_filters({"family_name": ["Baggins", "Oakenshield"], "given_name": "A"}) + [{"family_name": "Baggins", "given_name": "A"}, {"family_name": "Oakenshield", "given_name": "A"}] + """ + if isinstance(filter, (list, tuple)): + if len(filter) == 0: + raise ValueError("Cannot filter on an empty list of values") + return list(filter) + if isinstance(filter, dict): + split = {key: to_single_value_filters(value) for key, value in filter.items()} + keys_with_lists = [key for key, values in split.items() if len(values) > 1] + if len(keys_with_lists) > 1: + raise ValueError( + f"Only one list of filter values is supported within a single property, not {len(keys_with_lists)}" + ) + base = {key: values[0] for key, values in split.items()} + if keys_with_lists: + key = keys_with_lists[0] + return [{**base, key: item} for item in split[key]] + return [base] + return [filter] + + +def get_query_filter_property(property, context, filter: Any, name: Optional[str] = None) -> QueryProperty: """ Generate a QueryProperty instance containing a filter, for use in constructing a KG query definition. + + The filter must contain a single value: use `to_single_value_filters()` for filters containing lists. + The name of the QueryProperty defaults to "Q" followed by the property name. """ assert filter is not None + if name is None: + name = f"Q{property.name}" if isinstance(filter, dict): # we pass the filter through to the next level filter_obj = None @@ -509,7 +548,7 @@ def get_query_filter_property(property, context, filter: Any) -> QueryProperty: if any(issubclass(_type, Node) for _type in property.types): assert all(issubclass(_type, Node) for _type in property.types) - prop = QueryProperty(expanded_path, name=f"Q{property.name}", required=True, reverse=property.reverse) + prop = QueryProperty(expanded_path, name=name, required=True, reverse=property.reverse) if filter_obj: if filter_obj.value.startswith("https://kg.ebrains.eu/api/instances"): filter_path = "@id" @@ -526,7 +565,7 @@ def get_query_filter_property(property, context, filter: Any) -> QueryProperty: prop.properties.extend(child_properties) break else: - prop = QueryProperty(expanded_path, name=f"Q{property.name}", filter=filter_obj, required=True) + prop = QueryProperty(expanded_path, name=name, filter=filter_obj, required=True) return prop @@ -565,7 +604,9 @@ def is_valid(val): isinstance(val, KGProxy) and not set(val.classes).isdisjoint(property.types) ) - if isinstance(value, list) and len(value) > 0: + if isinstance(value, (list, tuple)) and len(value) == 0: + raise ValueError(f"Cannot filter {property.name} on an empty list of values") + if isinstance(value, list): valid_type = all(is_valid(item) for item in value) have_multiple = True else: @@ -574,7 +615,7 @@ def is_valid(val): if not valid_type: if property.name == "hash": # bit of a hack filter_value = value - elif isinstance(value, str) and value.startswith("http"): # for @id + elif all(isinstance(item, str) and item.startswith("http") for item in as_list(value)): # for @id filter_value = value else: raise TypeError("{} must be of type {}, not {}".format(property.name, property.types, type(value))) diff --git a/test/test_queries.py b/test/test_queries.py index 1b029056..1fcdf072 100644 --- a/test/test_queries.py +++ b/test/test_queries.py @@ -845,3 +845,98 @@ def test_existence_query_filters_strings_with_contains_if_requested(mock_client) def test_existence_query_rejects_an_invalid_match(): with pytest.raises(ValueError, match="existence_match"): omcore.Dataset(short_name="FOO")._build_existence_query("fuzzy") + + +def _filter_properties(query, path): + return [prop for prop in query["structure"] if prop.get("path") == path and prop.get("required")] + + +def test_list_filter_repeats_the_property_once_per_value(mock_client): + # each value must be matched by some developer, as in a hand-written KG query + query = omcore.SoftwareVersion.generate_query( + client=mock_client, + space=None, + filters={"developers__family_name": [Equals("Baggins"), Equals("Oakenshield")]}, + ) + props = _filter_properties(query, "https://openminds.om-i.org/props/developer") + assert [prop["propertyName"] for prop in props] == ["Qdevelopers1", "Qdevelopers2"] + assert [prop["structure"] for prop in props] == [ + [ + { + "path": "https://openminds.om-i.org/props/familyName", + "propertyName": "Qfamily_name", + "filter": {"op": "EQUALS", "value": family_name}, + "required": True, + } + ] + for family_name in ("Baggins", "Oakenshield") + ] + + +def test_list_filter_keeps_sibling_filters_in_each_copy(mock_client): + query = omcore.SoftwareVersion.generate_query( + client=mock_client, + space=None, + filters={"developers__given_name": "Bilbo", "developers__family_name": ["Baggins", "Oakenshield"]}, + ) + props = _filter_properties(query, "https://openminds.om-i.org/props/developer") + assert len(props) == 2 + for prop, family_name in zip(props, ("Baggins", "Oakenshield")): + filters = {sub_prop["propertyName"]: sub_prop["filter"]["value"] for sub_prop in prop["structure"]} + assert filters == {"Qgiven_name": "Bilbo", "Qfamily_name": family_name} + + +@pytest.mark.parametrize("as_objects", [True, False]) +def test_list_filter_on_links(mock_client, as_objects): + import fairgraph.openminds.publications as ompub + + ids = [f"https://kg.ebrains.eu/api/instances/00000000-0000-0000-0000-00000000000{i}" for i in (1, 2)] + if as_objects: + values = [omcore.Person(id=id, given_name="A", family_name="B") for id in ids] + else: + values = ids + query = ompub.Chapter.generate_query(client=mock_client, space=None, filters={"authors": values}) + props = _filter_properties(query, "https://openminds.om-i.org/props/author") + assert [prop["propertyName"] for prop in props] == ["Qauthors1", "Qauthors2"] + assert [prop["structure"][0]["path"] for prop in props] == ["@id", "@id"] + assert [prop["structure"][0]["filter"]["value"] for prop in props] == ids + + +def test_list_filter_with_one_value_is_the_same_as_a_single_value(mock_client): + single = omcore.SoftwareVersion.generate_query( + client=mock_client, space=None, filters={"developers__family_name": "Baggins"} + ) + as_list = omcore.SoftwareVersion.generate_query( + client=mock_client, space=None, filters={"developers__family_name": ["Baggins"]} + ) + assert as_list == single + + +def test_list_filter_rejects_an_empty_list(mock_client): + with pytest.raises(ValueError, match="empty list"): + omcore.SoftwareVersion.generate_query(client=mock_client, space=None, filters={"developers__family_name": []}) + + +def test_list_filter_rejects_two_lists_within_one_property(mock_client): + with pytest.raises(ValueError, match="Only one list"): + omcore.SoftwareVersion.generate_query( + client=mock_client, + space=None, + filters={"developers__family_name": ["Baggins", "Oakenshield"], "developers__given_name": ["A", "B"]}, + ) + + +@skip_if_no_connection +def test_execute_query_with_list_filter(kg_client): + # software versions developed by both people should be those found by filtering on each separately + family_names = ["Davison", "Appukuttan"] + + def software_version_ids(family_name): + software_versions = omcore.SoftwareVersion.list( + kg_client, release_status="released", size=1000, developers__family_name=family_name + ) + return {software_version.id for software_version in software_versions} + + expected_ids = set.intersection(*(software_version_ids(Equals(family_name)) for family_name in family_names)) + assert len(expected_ids) > 0 + assert software_version_ids([Equals(family_name) for family_name in family_names]) == expected_ids