Handle unknown data types in data modeling client (#1723)

CHANGELOG.md

@@ -17,10 +17,14 @@ Changes are grouped as follows
 - `Fixed` for any bug fixes.
 - `Security` in case of vulnerabilities.
 
+## [7.37.0] - 2024-04-16
+### Fixed
+- Now handle unknown data types in DM
+
 ## [7.36.0] - 2024-04-16
 ### Fixed
-- Now handle unknown filter types
-- Add support for the "invalid" filter type in DMS
+- Now handle unknown filter types in DM
+- Add support for the "invalid" filter type in DM
 
 ## [7.35.0] - 2024-04-16
 ### Added

cognite/client/_version.py

@@ -1,4 +1,4 @@
 from __future__ import annotations
 
-__version__ = "7.36.0"
+__version__ = "7.37.0"
 __api_subversion__ = "20230101"

cognite/client/data_classes/data_modeling/data_types.py

@@ -1,21 +1,19 @@
 from __future__ import annotations
 
-import inspect
 import logging
-from abc import ABC, abstractmethod
-from dataclasses import asdict, dataclass
-from typing import Any, ClassVar, cast
+from abc import ABC
+from dataclasses import asdict, dataclass, field
+from typing import Any, ClassVar
 
 from typing_extensions import Self
 
 from cognite.client.data_classes.data_modeling.ids import ContainerId
 from cognite.client.utils._auxiliary import rename_and_exclude_keys
-from cognite.client.utils._text import convert_all_keys_recursive, convert_all_keys_to_snake_case
+from cognite.client.utils._text import convert_all_keys_recursive
 
 logger = logging.getLogger(__name__)
 
-_PROPERTY_ALIAS = {"list": "isList"}
-_PROPERTY_ALIAS_INV = {"isList": "list", "is_list": "list"}
+_PROPERTY_ALIAS = {"isList": "list", "is_list": "list"}
 
 
 @dataclass(frozen=True)
@@ -48,78 +46,98 @@ def as_tuple(self) -> tuple[str, str]:
 @dataclass
 class PropertyType(ABC):
     _type: ClassVar[str]
+    is_list: bool = False
 
     def dump(self, camel_case: bool = True) -> dict[str, Any]:
         output = asdict(self)
         output["type"] = self._type
         for key in list(output.keys()):
             if output[key] is None:
                 output.pop(key)
-        output = rename_and_exclude_keys(output, aliases=_PROPERTY_ALIAS_INV)
+        output = rename_and_exclude_keys(output, aliases=_PROPERTY_ALIAS)
         return convert_all_keys_recursive(output, camel_case)
 
+    @staticmethod
+    def __load_unit_ref(data: dict) -> UnitReference | None:
+        unit: UnitReference | None = None
+        if (unit_raw := data.get("unit")) and isinstance(unit_raw, dict):
+            unit = UnitReference.load(unit_raw)
+        return unit
+
     @classmethod
     def load(cls, data: dict) -> Self:
-        if "type" not in data:
-            raise ValueError("Property types are required to have a type")
         type_ = data["type"]
-        data = convert_all_keys_to_snake_case(rename_and_exclude_keys(data, aliases=_PROPERTY_ALIAS, exclude={"type"}))
-
-        if type_cls := _TYPE_LOOKUP.get(type_):
-            if issubclass(type_cls, LoadablePropertyType):
-                return cast(Self, type_cls.load(data))
-            try:
-                return cast(Self, type_cls(**data))
-            except TypeError:
-                not_supported = set(data).difference(inspect.signature(type_cls).parameters) - {"type"}
-                logger.warning(
-                    f"For '{type_cls.__name__}', the following properties are not yet supported in the SDK (ignored): "
-                    f"{not_supported}. Try updating to the latest SDK version, or create an issue on Github!"
-                )
-                return cast(Self, type_cls(**rename_and_exclude_keys(data, exclude=not_supported)))
-
-        raise ValueError(f"Invalid type {type_}.")
+        obj: Any
+        if type_ == "text":
+            obj = Text(is_list=data["list"], collation=data["collation"])
+        elif type_ == "boolean":
+            obj = Boolean(is_list=data["list"])
+        elif type_ == "float32":
+            obj = Float32(is_list=data["list"], unit=cls.__load_unit_ref(data))
+        elif type_ == "float64":
+            obj = Float64(is_list=data["list"], unit=cls.__load_unit_ref(data))
+        elif type_ == "int32":
+            obj = Int32(is_list=data["list"], unit=cls.__load_unit_ref(data))
+        elif type_ == "int64":
+            obj = Int64(is_list=data["list"], unit=cls.__load_unit_ref(data))
+        elif type_ == "timestamp":
+            obj = Timestamp(is_list=data["list"])
+        elif type_ == "date":
+            obj = Date(is_list=data["list"])
+        elif type_ == "json":
+            obj = Json(is_list=data["list"])
+        elif type_ == "timeseries":
+            obj = TimeSeriesReference(is_list=data["list"])
+        elif type_ == "file":
+            obj = FileReference(is_list=data["list"])
+        elif type_ == "sequence":
+            obj = SequenceReference(is_list=data["list"])
+        elif type_ == "direct":
+            obj = DirectRelation(
+                container=ContainerId.load(container) if (container := data.get("container")) else None,
+                is_list=data["list"],
+            )
+        else:
+            logger.warning(f"Unknown property type: {type_}")
+            obj = UnknownPropertyType(_data=data)
+        return obj
 
 
 @dataclass
-class LoadablePropertyType(ABC):
-    @classmethod
-    @abstractmethod
-    def load(cls, data: dict) -> Self: ...
+class UnknownPropertyType:
+    _data: dict[str, Any] = field(default_factory=dict)
 
-
-@dataclass
-class ListablePropertyType(PropertyType, ABC):
-    is_list: bool = False
+    def dump(self, camel_case: bool = True) -> dict[str, Any]:
+        return convert_all_keys_recursive(self._data, camel_case=camel_case)
 
 
 @dataclass
-class Text(ListablePropertyType):
+class Text(PropertyType):
     _type = "text"
     collation: str = "ucs_basic"
 
 
 @dataclass
-class Primitive(ListablePropertyType, ABC): ...
+class Primitive(PropertyType, ABC): ...
 
 
 @dataclass
-class Boolean(ListablePropertyType):
+class Boolean(PropertyType):
     _type = "boolean"
 
 
 @dataclass
-class Timestamp(ListablePropertyType):
+class Timestamp(PropertyType):
     _type = "timestamp"
 
 
 @dataclass
-class Date(ListablePropertyType):
+class Date(PropertyType):
     _type = "date"
 
 
 @dataclass
-class Json(ListablePropertyType):
+class Json(PropertyType):
     _type = "json"
 
 
@@ -155,20 +173,9 @@ def load(cls, data: dict) -> UnitSystemReference:
 
 
 @dataclass
-class ListablePropertyTypeWithUnit(ListablePropertyType, LoadablePropertyType, ABC):
+class PropertyTypeWithUnit(PropertyType, ABC):
     unit: UnitReference | None = None
 
-    @classmethod
-    def load(cls, data: dict) -> Self:
-        data = convert_all_keys_to_snake_case(rename_and_exclude_keys(data, aliases=_PROPERTY_ALIAS, exclude={"type"}))
-        unit: UnitReference | None = None
-        if (unit_raw := data.get("unit")) and isinstance(unit_raw, dict):
-            unit = UnitReference.load(unit_raw)
-        return cls(
-            is_list=data["is_list"],
-            unit=unit,
-        )
-
     def dump(self, camel_case: bool = True) -> dict[str, Any]:
         output = super().dump(camel_case)
         if self.unit:
@@ -177,27 +184,27 @@ def dump(self, camel_case: bool = True) -> dict[str, Any]:
 
 
 @dataclass
-class Float32(ListablePropertyTypeWithUnit):
+class Float32(PropertyTypeWithUnit):
     _type = "float32"
 
 
 @dataclass
-class Float64(ListablePropertyTypeWithUnit):
+class Float64(PropertyTypeWithUnit):
     _type = "float64"
 
 
 @dataclass
-class Int32(ListablePropertyType):
+class Int32(PropertyTypeWithUnit):
     _type = "int32"
 
 
 @dataclass
-class Int64(ListablePropertyType):
+class Int64(PropertyTypeWithUnit):
     _type = "int64"
 
 
 @dataclass
-class CDFExternalIdReference(ListablePropertyType, ABC): ...
+class CDFExternalIdReference(PropertyType, ABC): ...
 
 
 @dataclass
@@ -216,7 +223,7 @@ class SequenceReference(CDFExternalIdReference):
 
 
 @dataclass
-class DirectRelation(PropertyType, LoadablePropertyType):
+class DirectRelation(PropertyType):
     _type = "direct"
     container: ContainerId | None = None
 
@@ -228,24 +235,3 @@ def dump(self, camel_case: bool = True) -> dict:
             elif output["container"] is None:
                 output.pop("container")
         return output
-
-    @classmethod
-    def load(cls, data: dict) -> Self:
-        return cls(container=ContainerId.load(container) if (container := data.get("container")) else None)
-
-
-_TYPE_LOOKUP: dict[str, type[PropertyType]] = {
-    "text": Text,
-    "boolean": Boolean,
-    "float32": Float32,
-    "float64": Float64,
-    "int32": Int32,
-    "int64": Int64,
-    "timestamp": Timestamp,
-    "date": Date,
-    "json": Json,
-    "timeseries": TimeSeriesReference,
-    "file": FileReference,
-    "sequence": SequenceReference,
-    "direct": DirectRelation,
-}

pyproject.toml

@@ -1,7 +1,7 @@
 [tool.poetry]
 name = "cognite-sdk"
 
-version = "7.36.0"
+version = "7.37.0"
 description = "Cognite Python SDK"
 readme = "README.md"
 documentation = "https://cognite-sdk-python.readthedocs-hosted.com"

tests/tests_unit/test_data_classes/test_data_models/test_containers.py

@@ -31,7 +31,7 @@ class TestContainerProperty:
     @pytest.mark.parametrize(
         "data",
         [
-            {"type": {"type": "direct"}},
+            {"type": {"type": "direct", "list": False}},
             # List is not required, but gets set and thus will be dumped
             {"type": {"type": "int32", "list": False}},
             {"type": {"type": "text", "list": False, "collation": "ucs_basic"}},

tests/tests_unit/test_data_classes/test_data_models/test_data_types.py

@@ -1,11 +1,11 @@
 import pytest
 
 from cognite.client.data_classes.data_modeling.data_types import (
-    DirectRelation,
     DirectRelationReference,
-    Float32,
     PropertyType,
+    PropertyTypeWithUnit,
     UnitReference,
+    UnknownPropertyType,
 )
 
 
@@ -21,8 +21,21 @@ class TestPropertyType:
         "data",
         [
             {"type": "text", "collation": "ucs_basic", "list": False},
+            {"type": "boolean", "list": False},
+            {"type": "float32", "list": False},
+            {"type": "float64", "list": False},
             {"type": "int32", "list": True},
+            {"type": "int64", "list": True},
             {"type": "timeseries", "list": False},
+            {"type": "file", "list": False},
+            {"type": "sequence", "list": False},
+            {"type": "json", "list": False},
+            {"type": "timestamp", "list": False},
+            {
+                "type": "direct",
+                "container": {"space": "mySpace", "externalId": "myId", "type": "container"},
+                "list": True,
+            },
         ],
     )
     def test_load_dump(self, data: dict) -> None:
@@ -31,23 +44,30 @@ def test_load_dump(self, data: dict) -> None:
         assert data == actual
 
     def test_load_ignore_unknown_properties(self) -> None:
-        data = {"type": "float64", "list": True, "unit": {"externalId": "known"}, "super_unit": "unknown"}
+        data = {"type": "float64", "list": True, "unit": {"externalId": "known"}, "unknownProperty": "unknown"}
         actual = PropertyType.load(data).dump(camel_case=True)
-        data.pop("super_unit")
-
+        data.pop("unknownProperty")
         assert data == actual
 
-
-class TestDirectRelation:
-    def test_load_dump_container_direct_relation(self) -> None:
-        data = {"type": "direct", "container": {"space": "mySpace", "externalId": "myId", "type": "container"}}
-
-        assert data == DirectRelation.load(data).dump(camel_case=True)
+    def test_load_dump_unkown_property(self) -> None:
+        data = {"type": "unknowngibberish", "list": True, "unit": {"externalId": "known"}}
+        obj = PropertyType.load(data)
+        assert isinstance(obj, UnknownPropertyType)
+        actual = obj.dump(camel_case=True)
+        assert data == actual
 
 
 class TestUnitSupport:
-    def test_load_dump_property_with_unit(self) -> None:
-        data = {"type": "float32", "list": False, "unit": {"externalId": "temperature:celsius"}}
-        property = Float32.load(data)
+    @pytest.mark.parametrize(
+        "data",
+        [
+            {"type": "float32", "list": False, "unit": {"externalId": "temperature:celsius"}},
+            {"type": "float64", "list": False, "unit": {"externalId": "temperature:celsius"}},
+            {"type": "int32", "list": True, "unit": {"externalId": "temperature:celsius"}},
+            {"type": "int64", "list": True, "unit": {"externalId": "temperature:celsius"}},
+        ],
+    )
+    def test_load_dump_property_with_unit(self, data: dict) -> None:
+        property = PropertyTypeWithUnit.load(data)
         assert isinstance(property.unit, UnitReference)
         assert data == property.dump(camel_case=True)

tests/tests_unit/test_data_classes/test_data_models/test_views.py

@@ -27,6 +27,7 @@ def test_load_dumped_mapped_property_for_read(self) -> None:
             "type": {
                 "type": "direct",
                 "source": {"type": "view", "space": "mySpace", "externalId": "myExternalId", "version": "myVersion"},
+                "list": False,
             },
             "container": {"space": "mySpace", "externalId": "myExternalId", "type": "container"},
             "containerPropertyIdentifier": "name",
@@ -51,6 +52,7 @@ def test_load_dumped_mapped_property_for_read(self) -> None:
             "type": {
                 "type": "direct",
                 "source": {"external_id": "myExternalId", "space": "mySpace", "version": "myVersion"},
+                "list": False,
             },
         }