Skip to content

Commit

Permalink
bugfix and regression fix for InAssetSubtree filter (#1954)
Browse files Browse the repository at this point in the history
  • Loading branch information
haakonvt authored Oct 6, 2024
1 parent 8b2b99c commit 723bde9
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 17 deletions.
8 changes: 7 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,15 @@ Changes are grouped as follows
- `Fixed` for any bug fixes.
- `Security` in case of vulnerabilities.

## [7.62.7] - 2024-10-07
### Fixed
- Several bugfixes for the filter `InAssetSubtree`:
- No longer causes `CogniteAPIError` when used, by accepting a list of values rather than a single value.
- Loading via `Filter.load` now works as expected (a regression introduced in 7.38.3).

## [7.62.6] - 2024-09-27
### Fixed
- Instances with a single property no longer fail `to_pandas()` with `TypeError`, when using expand_properties=True.
- Instances with a single property no longer fail `to_pandas()` with `TypeError`, when using `expand_properties=True`.

## [7.62.5] - 2024-09-26
### Added
Expand Down
12 changes: 10 additions & 2 deletions cognite/client/_api/documents.py
Original file line number Diff line number Diff line change
Expand Up @@ -259,12 +259,20 @@ def aggregate_count(self, query: str | None = None, filter: Filter | dict[str, A
Count the number of PDF documents in your CDF project:
>>> from cognite.client import CogniteClient
>>> from cognite.client.data_classes import filters
>>> from cognite.client.data_classes.documents import DocumentProperty
>>> client = CogniteClient()
>>> is_pdf = filters.Equals(DocumentProperty.mime_type, "application/pdf")
>>> pdf_count = client.documents.aggregate_count(filter=is_pdf)
Count the number of documents with a related asset in a subtree rooted at any of
the specified external IDs, e.g. 'Plant_1' and 'Plant_2':
>>> client.documents.aggregate_count(
... filter=filters.InAssetSubtree(
... property=DocumentProperty.asset_external_ids,
... values=['Plant_1', 'Plant_2'],
... )
... )
"""
self._validate_filter(filter)
return self._advanced_aggregate(
Expand Down
2 changes: 1 addition & 1 deletion cognite/client/_version.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from __future__ import annotations

__version__ = "7.62.6"
__version__ = "7.62.7"
__api_subversion__ = "20230101"
36 changes: 29 additions & 7 deletions cognite/client/data_classes/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,10 +174,10 @@ def load(cls, filter_: dict[str, Any]) -> Filter:
property=filter_body["property"],
geometry=Geometry.load(filter_body["geometry"]),
)
elif (filter_body := filter_.get(SpaceFilter._filter_name)) is not None:
elif (filter_body := filter_.get(InAssetSubtree._filter_name)) is not None:
return InAssetSubtree(
property=filter_body["property"],
value=_load_filter_value(filter_body["value"]),
values=cast(FilterValueList, _load_filter_value(filter_body["values"])),
)
elif (filter_body := filter_.get(Search._filter_name)) is not None:
return Search(
Expand Down Expand Up @@ -254,7 +254,7 @@ def _validate_filter(


class CompoundFilter(Filter, ABC):
_filter_name = "compound"
_filter_name: str

def __init__(self, *filters: Filter) -> None:
if not_flt := [flt for flt in filters if not isinstance(flt, Filter)]:
Expand All @@ -266,7 +266,7 @@ def _filter_body(self, camel_case_property: bool) -> list | dict:


class FilterWithProperty(Filter, ABC):
_filter_name = "propertyFilter"
_filter_name: str

def __init__(self, property: PropertyReference) -> None:
self._property = property
Expand All @@ -279,7 +279,7 @@ def _filter_body(self, camel_case_property: bool) -> dict:


class FilterWithPropertyAndValue(FilterWithProperty, ABC):
_filter_name = "propertyAndValueFilter"
_filter_name: str

def __init__(self, property: PropertyReference, value: FilterValue) -> None:
super().__init__(property)
Expand All @@ -290,7 +290,7 @@ def _filter_body(self, camel_case_property: bool) -> dict[str, Any]:


class FilterWithPropertyAndValueList(FilterWithProperty, ABC):
_filter_name = "propertyAndValueListFilter"
_filter_name: str

def __init__(self, property: PropertyReference, values: FilterValueList) -> None:
super().__init__(property)
Expand Down Expand Up @@ -790,7 +790,27 @@ class GeoJSONWithin(GeoJSON):


@final
class InAssetSubtree(FilterWithPropertyAndValue):
class InAssetSubtree(FilterWithPropertyAndValueList):
"""Filters results based on whether item/resource is connected to an asset with an ID (or external ID)
that is in the subtree rooted at any of the provided IDs.
Args:
property (PropertyReference): The property to filter on, e.g. 'assetId' or 'assetExternalId'.
values (FilterValueList): The value(s) to filter on.
Example:
Count the number of documents with a related asset in a subtree rooted at any of
the specified external IDs, e.g. 'Plant_1' and 'Plant_2':
>>> client.documents.aggregate_count(
... filter=filters.InAssetSubtree(
... property=DocumentProperty.asset_external_ids,
... values=['Plant_1', 'Plant_2'],
... )
... )
"""

_filter_name = "inAssetSubtree"


Expand Down Expand Up @@ -827,6 +847,8 @@ class SpaceFilter(FilterWithProperty):
>>> flt = SpaceFilter("space3", instance_type="edge")
"""

_filter_name = "__space"

def __init__(self, space: str | SequenceNotStr[str], instance_type: Literal["node", "edge"] = "node") -> None:
super().__init__(property=[instance_type, "space"])
space = [space] if isinstance(space, str) else list(space)
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[tool.poetry]
name = "cognite-sdk"

version = "7.62.6"
version = "7.62.7"
description = "Cognite Python SDK"
readme = "README.md"
documentation = "https://cognite-sdk-python.readthedocs-hosted.com"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from cognite.client.data_classes._base import EnumProperty
from cognite.client.data_classes.data_modeling import ViewId
from cognite.client.data_classes.data_modeling.data_types import DirectRelationReference
from cognite.client.data_classes.filters import Filter
from cognite.client.data_classes.filters import Filter, UnknownFilter
from tests.utils import all_subclasses

if TYPE_CHECKING:
Expand All @@ -18,6 +18,7 @@

def load_and_dump_equals_data() -> Iterator[ParameterSet]:
yield pytest.param(
f.And,
{
"and": [
{"in": {"property": ["tag"], "values": ["1001_1", "1001_1"]}},
Expand All @@ -28,6 +29,7 @@ def load_and_dump_equals_data() -> Iterator[ParameterSet]:
)

yield pytest.param(
f.Or,
{
"or": [
{"equals": {"property": ["name"], "value": "Quentin Tarantino"}},
Expand All @@ -43,16 +45,19 @@ def load_and_dump_equals_data() -> Iterator[ParameterSet]:
)

yield pytest.param(
f.Not,
{"not": {"equals": {"property": ["scenario"], "value": "scenario7"}}},
id="Not equals",
)

yield pytest.param(
f.Range,
{"range": {"property": ["weight"], "lte": 100, "gt": 0}},
id="Range, lte and gt",
)

yield pytest.param(
f.And,
{
"and": [
{
Expand All @@ -70,6 +75,7 @@ def load_and_dump_equals_data() -> Iterator[ParameterSet]:
)

yield pytest.param(
f.Or,
{
"or": [
{
Expand All @@ -85,6 +91,7 @@ def load_and_dump_equals_data() -> Iterator[ParameterSet]:
id="Or overlaps or containsAny",
)
yield pytest.param(
f.Nested,
{
"nested": {
"scope": ("space", "container", "prop"),
Expand All @@ -95,13 +102,16 @@ def load_and_dump_equals_data() -> Iterator[ParameterSet]:
)

yield pytest.param(
{"range": {"property": ["weight"], "gte": {"property": ["capacity"]}}}, id="Range with gte another property"
f.Range,
{"range": {"property": ["weight"], "gte": {"property": ["capacity"]}}},
id="Range with gte another property",
)

yield pytest.param(
{"prefix": {"property": ["name"], "value": {"parameter": "param1"}}}, id="prefix with parameters"
f.Prefix, {"prefix": {"property": ["name"], "value": {"parameter": "param1"}}}, id="prefix with parameters"
)
yield pytest.param(
f.Prefix,
{
"prefix": {
"property": ["cdf_cdm", "CogniteAsset/v1", "path"],
Expand All @@ -116,6 +126,7 @@ def load_and_dump_equals_data() -> Iterator[ParameterSet]:
id="prefix with list of dicts",
)
yield pytest.param(
f.Prefix,
{
"prefix": {
"property": ["cdf_cdm", "CogniteAsset/v1", "path"],
Expand All @@ -129,11 +140,22 @@ def load_and_dump_equals_data() -> Iterator[ParameterSet]:
},
id="prefix with list of objects",
)
yield pytest.param(
f.InAssetSubtree,
{
"inAssetSubtree": {
"property": ["assetIds"],
"values": [11, 22],
}
},
id="InAssetSubtree with list of asset ids",
)


@pytest.mark.parametrize("raw_data", list(load_and_dump_equals_data()))
def test_load_and_dump_equals(raw_data: dict) -> None:
@pytest.mark.parametrize("expected_filter_cls, raw_data", list(load_and_dump_equals_data()))
def test_load_and_dump_equals(expected_filter_cls: type[Filter], raw_data: dict) -> None:
parsed = Filter.load(raw_data)
assert isinstance(parsed, expected_filter_cls)
dumped = parsed.dump(camel_case_property=False)
assert dumped == raw_data

Expand Down Expand Up @@ -274,6 +296,19 @@ def test_space_filter_passes_isinstance_checks(self) -> None:
space_filter = f.SpaceFilter("myspace", "edge")
assert isinstance(space_filter, Filter)

@pytest.mark.parametrize(
"body",
(
{"property": ["edge", "space"], "value": "myspace"},
{"property": ["node", "space"], "values": ["myspace", "another"]},
),
)
def test_space_filter_loads_as_unknown(self, body: dict[str, str | list[str]]) -> None:
# Space Filter is an SDK concept, so it should load as an UnknownFilter:
dumped = {f.SpaceFilter._filter_name: body}
loaded_flt = Filter.load(dumped)
assert isinstance(loaded_flt, UnknownFilter)

@pytest.mark.parametrize(
"space_filter",
[
Expand Down

0 comments on commit 723bde9

Please sign in to comment.