diff --git a/CHANGELOG.md b/CHANGELOG.md index b66b70aac7..e58caba4aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,12 @@ Changes are grouped as follows - `Fixed` for any bug fixes. - `Security` in case of vulnerabilities. +## [7.63.3] - 2024-10-13 +### Fixed +- NodeList and EdgeList (and subclasses) now support using `.get` with an `external_id` as a shortcut over + using `instance_id`. When the `external_id` is ambiguous (non-unique, multiple spaces), a ValueError + is raised. Thus, using `external_id` is no longer deprecated. + ## [7.63.2] - 2024-10-11 ### Fixed - Setting up interactive `OAuthInteractive` sessions no longer raises `TypeError` as the lower bound for the `msal` @@ -29,7 +35,7 @@ Changes are grouped as follows ## [7.63.0] - 2024-10-10 ### Removed -- Removed support for Python 3.8 and 3.9. +- Dropped support for Python 3.8 and 3.9. ## [7.62.8] - 2024-10-07 ### Added diff --git a/cognite/client/_version.py b/cognite/client/_version.py index 2e1d91f2aa..7c16d77305 100644 --- a/cognite/client/_version.py +++ b/cognite/client/_version.py @@ -1,4 +1,4 @@ from __future__ import annotations -__version__ = "7.63.2" +__version__ = "7.63.3" __api_subversion__ = "20230101" diff --git a/cognite/client/data_classes/data_modeling/instances.py b/cognite/client/data_classes/data_modeling/instances.py index 0e30e30c17..7f8b565ce8 100644 --- a/cognite/client/data_classes/data_modeling/instances.py +++ b/cognite/client/data_classes/data_modeling/instances.py @@ -61,7 +61,7 @@ ViewId, ViewIdentifier, ) -from cognite.client.utils._auxiliary import flatten_dict +from cognite.client.utils._auxiliary import exactly_one_is_not_none, find_duplicates, flatten_dict from cognite.client.utils._identifier import InstanceId from cognite.client.utils._importing import local_import from cognite.client.utils._text import convert_all_keys_to_snake_case, to_camel_case @@ -935,41 +935,55 @@ def as_ids(self) -> list[NodeId]: class DataModelingInstancesList(WriteableCogniteResourceList[T_WriteClass, T_Instance], ABC): def _build_id_mappings(self) -> None: self._instance_id_to_item = {(inst.space, inst.external_id): inst for inst in self.data} - # TODO: Remove when we ditch PY3.8 (Oct, 2024), reason: ambiguous without space: + # We allow fast lookup via external ID when not ambiguous: + self._ambiguous_xids = find_duplicates(inst.external_id for inst in self.data) self._ext_id_to_item = {inst.external_id: inst for inst in self.data} - def get( + def get( # type: ignore [override] self, - id: InstanceId | tuple[str, str] | None = None, # type: ignore [override] + instance_id: InstanceId | tuple[str, str] | None = None, external_id: str | None = None, + *, + id: InstanceId | tuple[str, str] | None = None, ) -> T_Instance | None: """Get an instance from this list by instance ID. Args: - id (InstanceId | tuple[str, str] | None): The instance ID to get. A tuple on the form (space, external_id) is also accepted. - external_id (str | None): DEPRECATED (reason: ambiguous). The external ID of the instance to return. + instance_id (InstanceId | tuple[str, str] | None): The instance ID to get. A tuple on the form (space, external_id) is also accepted. + external_id (str | None): The external ID of the instance to return. Will raise ValueError when ambiguous (in presence of multiple spaces). + id (InstanceId | tuple[str, str] | None): (DEPRECATED) Backwards-compatible alias for instance_id. Will be removed in the next major version. Returns: T_Instance | None: The requested instance if present, else None """ - # TODO: Remove when we ditch PY3.8 - if external_id is not None: + if not exactly_one_is_not_none(instance_id, external_id, id): + raise ValueError( + "Pass exactly one of 'instance_id' or 'external_id' ('id' is a deprecated alias for 'instance_id'). " + "Using an external ID requires all instances to be from the same space." + ) + if id is not None: + instance_id = id warnings.warn( - "Calling .get with an external ID is deprecated due to being ambiguous in the absense of 'space', and " - "will be removed as of Oct, 2024. Pass an instance ID instead (or a tuple of (space, external_id)).", + "Calling .get using `id` is deprecated and will be removed in the next major version. " + "Use 'instance_id' instead", UserWarning, ) + elif external_id is not None: + if external_id in self._ambiguous_xids: + raise ValueError( + f"{external_id=} is ambiguous (multiple spaces are present). Pass 'instance_id' instead." + ) return self._ext_id_to_item.get(external_id) - if isinstance(id, InstanceId): - id = id.as_tuple() - return self._instance_id_to_item.get(id) # type: ignore [arg-type] + if isinstance(instance_id, InstanceId): + instance_id = instance_id.as_tuple() + return self._instance_id_to_item.get(instance_id) # type: ignore [arg-type] def extend(self, other: Iterable[Any]) -> None: other_res_list = type(self)(other) # See if we can accept the types if self._instance_id_to_item.keys().isdisjoint(other_res_list._instance_id_to_item): self.data.extend(other_res_list.data) - self._instance_id_to_item.update(other_res_list._instance_id_to_item) + self._build_id_mappings() else: raise ValueError("Unable to extend as this would introduce duplicates") @@ -1088,7 +1102,7 @@ def extend(self, other: NodeListWithCursor) -> None: # type: ignore[override] other_res_list = type(self)(other, other.cursor) # See if we can accept the types if self._instance_id_to_item.keys().isdisjoint(other_res_list._instance_id_to_item): self.data.extend(other.data) - self._instance_id_to_item.update(other_res_list._instance_id_to_item) + self._build_id_mappings() self.cursor = other.cursor else: raise ValueError("Unable to extend as this would introduce duplicates") @@ -1184,7 +1198,7 @@ def extend(self, other: EdgeListWithCursor) -> None: # type: ignore[override] other_res_list = type(self)(other, other.cursor) # See if we can accept the types if self._instance_id_to_item.keys().isdisjoint(other_res_list._instance_id_to_item): self.data.extend(other.data) - self._instance_id_to_item.update(other_res_list._instance_id_to_item) + self._build_id_mappings() self.cursor = other.cursor else: raise ValueError("Unable to extend as this would introduce duplicates") diff --git a/pyproject.toml b/pyproject.toml index 3e91d4d85a..23aeefdd24 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,7 +1,7 @@ [tool.poetry] name = "cognite-sdk" -version = "7.63.2" +version = "7.63.3" description = "Cognite Python SDK" readme = "README.md" documentation = "https://cognite-sdk-python.readthedocs-hosted.com" diff --git a/tests/tests_integration/test_api/test_data_modeling/test_instances.py b/tests/tests_integration/test_api/test_data_modeling/test_instances.py index 7c9aff7f06..e34bc3d4ff 100644 --- a/tests/tests_integration/test_api/test_data_modeling/test_instances.py +++ b/tests/tests_integration/test_api/test_data_modeling/test_instances.py @@ -813,8 +813,8 @@ def test_aggregate_invalid_view_id(self, cognite_client: CogniteClient) -> None: assert "One or more views do not exist: " in error.value.message assert view_id.as_source_identifier() in error.value.message - def test_dump_json_serialize_load_node(self, movie_nodes: NodeList) -> None: - node = movie_nodes.get(external_id="movie:pulp_fiction") + def test_dump_json_serialize_load_node(self, movie_nodes: NodeList, integration_test_space: Space) -> None: + node = movie_nodes.get((integration_test_space.space, "movie:pulp_fiction")) assert node is not None, "Pulp fiction movie not found, please recreate it" node_dumped = node.dump(camel_case=True) @@ -823,8 +823,8 @@ def test_dump_json_serialize_load_node(self, movie_nodes: NodeList) -> None: assert node == node_loaded - def test_dump_json_serialize_load_edge(self, movie_edges: EdgeList) -> None: - edge = movie_edges.get(external_id="person:quentin_tarantino:director:quentin_tarantino") + def test_dump_json_serialize_load_edge(self, movie_edges: EdgeList, integration_test_space: Space) -> None: + edge = movie_edges.get((integration_test_space.space, "person:quentin_tarantino:director:quentin_tarantino")) assert edge is not None, "Relation between Quentin Tarantino person and director not found, please recreate it" edge_dumped = edge.dump(camel_case=True) diff --git a/tests/tests_unit/test_data_classes/test_data_models/test_resource_class_methods.py b/tests/tests_unit/test_data_classes/test_data_models/test_resource_class_methods.py index ff957be7ef..8880822594 100644 --- a/tests/tests_unit/test_data_classes/test_data_models/test_resource_class_methods.py +++ b/tests/tests_unit/test_data_classes/test_data_models/test_resource_class_methods.py @@ -5,6 +5,7 @@ from __future__ import annotations +from copy import deepcopy from typing import Any import pytest @@ -68,9 +69,22 @@ def test_get_instance_lists(node_lst: NodeList, edge_lst: EdgeList, space: str, inst = inst_lst.get((space, "constant")) # pass by tuple assert inst.space == space # type: ignore [union-attr] - # Since ext.id is ambiguous, we always get the last (deprecated): - inst = inst_lst.get(external_id="constant") - assert inst.space == "foo2" # type: ignore [union-attr] + # Since ext.id is ambiguous, lookup should raise: + with pytest.raises(ValueError, match=r"^external_id='constant' is ambiguous.*Pass 'instance_id' instead\.$"): + inst = inst_lst.get(external_id="constant") + + lst1 = node_lst[:1] + lst2, lst3 = deepcopy(lst1), deepcopy(lst1) + lst2[0].external_id = "different" + lst3[0].space = "different" + assert lst1.get(external_id="constant") is not None + + lst1.extend(lst2) + assert lst1.get(external_id="constant") is not None + + lst1.extend(lst3) # there are now nodes in two spaces with the same external_id: + with pytest.raises(ValueError, match=r"^external_id='constant' is ambiguous.*Pass 'instance_id' instead\.$"): + lst1.get(external_id="constant") @pytest.mark.parametrize("space", ["foo0", "foo1", "foo2"])