[PYG-214] 🐷Missing Reverse Direct Relation Target

cognite/pygen/_core/generators.py

@@ -211,12 +211,19 @@ def __init__(
         self._logger = logger or print
         seen_views: set[dm.ViewId] = set()
         unique_views: list[dm.View] = []
+        # Used to verify that reverse direct relation's targets exist.
+        direct_relations_by_view_id: dict[dm.ViewId, set[str]] = {}
         for view in itertools.chain.from_iterable(model.views for model in data_models):
             view_id = view.as_id()
             if view_id in seen_views:
                 continue
             unique_views.append(view)
             seen_views.add(view_id)
+            direct_relations_by_view_id[view_id] = {
+                prop_name
+                for prop_name, prop in view.properties.items()
+                if isinstance(prop, dm.MappedProperty) and isinstance(prop.type, dm.DirectRelation)
+            }
 
         self.api_by_type_by_view_id = self.create_api_by_view_id_type(
             unique_views,
@@ -252,6 +259,7 @@ def __init__(
                 edge_class_by_view_id,
                 unique_views,
                 self.has_default_instance_space,
+                direct_relations_by_view_id,
                 config,
             )
 

cognite/pygen/_core/models/data_classes.py

@@ -146,6 +146,7 @@ def update_fields(
         edge_class_by_view_id: dict[dm.ViewId, EdgeDataClass],
         views: list[dm.View],
         has_default_instance_space: bool,
+        direct_relations_by_view_id: dict[dm.ViewId, set[str]],
         config: pygen_config.PygenConfig,
     ) -> None:
         """Update the fields of the data class.
@@ -164,6 +165,7 @@ def update_fields(
                 # This is the default value for pydantic_field, it will be updated later
                 pydantic_field=self.pydantic_field,
                 has_default_instance_space=has_default_instance_space,
+                direct_relations_by_view_id=direct_relations_by_view_id,
             )
             if field_ is None:
                 # Reverse direct relations are skipped
@@ -418,6 +420,11 @@ def has_dependencies(self) -> bool:
         """Check if the data class has any dependencies."""
         return bool(self.dependencies)
 
+    @property
+    def has_dependencies_not_self(self) -> bool:
+        """Check if the data class has any dependencies that are not itself."""
+        return any(dependency != self for dependency in self.dependencies)
+
     @property
     def has_edges_or_direct_relations(self) -> bool:
         """Whether the data class has any fields that are edges or direct relations."""
@@ -664,6 +671,7 @@ def update_fields(
         edge_class_by_view_id: dict[dm.ViewId, EdgeDataClass],
         views: list[dm.View],
         has_default_instance_space: bool,
+        direct_relations_by_view_id: dict[dm.ViewId, set[str]],
         config: pygen_config.PygenConfig,
     ):
         """Update the fields of the data class."""
@@ -699,5 +707,11 @@ def update_fields(
         )
         self.fields.append(self._end_node_field)
         super().update_fields(
-            properties, node_class_by_view_id, edge_class_by_view_id, views, has_default_instance_space, config
+            properties,
+            node_class_by_view_id,
+            edge_class_by_view_id,
+            views,
+            has_default_instance_space,
+            direct_relations_by_view_id,
+            config,
         )

cognite/pygen/_core/models/fields/base.py

@@ -57,6 +57,7 @@ def from_property(
         view_id: dm.ViewId,
         pydantic_field: Literal["Field", "pydantic.Field"],
         has_default_instance_space: bool,
+        direct_relations_by_view_id: dict[dm.ViewId, set[str]],
     ) -> Field | None:
         from .cdf_reference import CDFExternalField
         from .connections import BaseConnectionField
@@ -86,7 +87,14 @@ def from_property(
             isinstance(prop, dm.MappedProperty) and isinstance(prop.type, dm.DirectRelation)
         ):
             return BaseConnectionField.load(
-                base, prop, variable, node_class_by_view_id, edge_class_by_view_id, has_default_instance_space
+                base,
+                prop,
+                variable,
+                node_class_by_view_id,
+                edge_class_by_view_id,
+                has_default_instance_space,
+                view_id,
+                direct_relations_by_view_id,
             )
         elif isinstance(prop, dm.MappedProperty) and isinstance(prop.type, dm.CDFExternalIdReference):
             return CDFExternalField.load(base, prop, variable)

cognite/pygen/_core/models/fields/connections.py

@@ -16,6 +16,8 @@
     SingleReverseDirectRelation,
 )
 
+from cognite.pygen.warnings import MissingReverseDirectRelationTargetWarning
+
 from .base import Field
 
 if TYPE_CHECKING:
@@ -231,10 +233,25 @@ def load(
         node_class_by_view_id: dict[dm.ViewId, NodeDataClass],
         edge_class_by_view_id: dict[dm.ViewId, EdgeDataClass],
         has_default_instance_space: bool,
+        view_id: dm.ViewId,
+        direct_relations_by_view_id: dict[dm.ViewId, set[str]],
     ) -> Field | None:
         """Load a connection field from a property"""
         if not isinstance(prop, (dm.EdgeConnection, dm.MappedProperty, ReverseDirectRelation)):
             return None
+        if isinstance(prop, ReverseDirectRelation):
+            field_string = f"{view_id}.{base.prop_name}"
+            target = (
+                direct_relations_by_view_id.get(prop.through.source)
+                if isinstance(prop.through.source, dm.ViewId)
+                else None
+            )
+            if target is None or prop.through.property not in target:
+                target_str = (
+                    f"{prop.through.source}.{prop.through.property}" if target is not None else str(prop.through.source)
+                )
+                warnings.warn(MissingReverseDirectRelationTargetWarning(target_str, field_string), stacklevel=2)
+                return None
         edge_type = prop.type if isinstance(prop, dm.EdgeConnection) else None
         direction: Literal["outwards", "inwards"]
         if isinstance(prop, dm.EdgeConnection):

cognite/pygen/_core/templates/data_class_edge.py.jinja

@@ -29,7 +29,7 @@ from ._core import ({% if has_default_instance_space %}
 ){% for class_ in unique_start_classes %}
 from .{{class_.file_name }} import {{ class_.write_name }}{% endfor %}{% for classes in unique_end_classes %}
 from .{{classes.file_name }} import {{ classes.read_name }}, {{ classes.graphql_name }}, {{ classes.write_name }}{% endfor %}
-{% if data_class.has_dependencies %}
+{% if data_class.has_dependencies_not_self %}
 if TYPE_CHECKING:{% for dependency_class in data_class.dependencies %}{% if dependency_class.file_name != data_class.file_name %}
     from .{{ dependency_class.file_name }} import {{ dependency_class.read_name }}, {{ dependency_class.graphql_name }}{% if dependency_class.is_writable or dependency_class.is_interface %}, {{ dependency_class.write_name }}{% endif %}{% endif %}{% endfor %}
 {% endif %}

cognite/pygen/_core/templates/data_class_node.py.jinja

@@ -57,7 +57,7 @@ from ._core import ({% if has_default_instance_space %}
     {{ data_class.filtering_import }}{% endif %}
 ){% if data_class.implements %}{% for implements in data_class.implements %}
 from .{{ implements.file_name }} import {{ implements.read_name }}{% if data_class.is_writable or data_class.is_interface %}, {{ implements.write_name }}{% endif %}{% endfor %}{% endif %}
-{% if data_class.has_dependencies %}
+{% if data_class.has_dependencies_not_self %}
 if TYPE_CHECKING:{% for dependency_class in data_class.dependencies %}{% if dependency_class.file_name != data_class.file_name %}
     from .{{ dependency_class.file_name }} import {{ dependency_class.read_name }}, {{ dependency_class.graphql_name }}{% if dependency_class.is_writable or dependency_class.is_interface %}, {{ dependency_class.write_name }}{% endif %}{% endif %}{% endfor %}
 {% endif %}
@@ -193,7 +193,7 @@ class {{ data_class.read_name }}({{ data_class.read_base_class }}{% if is_pydant
         instances: dict[dm.NodeId{% if has_default_instance_space %} | str{% endif %}, {{ data_class.read_name }}],  # type: ignore[override]
         nodes_by_id: dict[dm.NodeId{% if has_default_instance_space %} | str{% endif %}, DomainModel],
         edges_by_source_node: dict[dm.NodeId, list[dm.Edge | DomainRelation]],
-    ) -> None:{% if data_class.has_dependencies %}{% for dependency_class in data_class.dependencies %}{% if dependency_class.file_name != data_class.file_name %}
+    ) -> None:{% if data_class.has_dependencies_not_self %}{% for dependency_class in data_class.dependencies %}{% if dependency_class.file_name != data_class.file_name %}
         from .{{ dependency_class.file_name }} import {{ dependency_class.read_name }}{% endif %}{% endfor %}
 {% endif %}{% if data_class.has_edges_or_direct_relations %}
         for instance in instances.values():{% for field in data_class.one_to_one_direct_relations_with_source %}

cognite/pygen/_generator.py

@@ -5,6 +5,7 @@
 import shutil
 import sys
 import tempfile
+import warnings
 from collections.abc import Sequence
 from pathlib import Path
 from typing import Any, Callable, Literal, Optional, Union, cast, overload
@@ -22,6 +23,7 @@
 from cognite.pygen.config import PygenConfig
 from cognite.pygen.exceptions import DataModelNotFound
 from cognite.pygen.utils.text import to_pascal, to_snake
+from cognite.pygen.warnings import InvalidCodeGenerated
 
 DataModel = Union[DataModelIdentifier, dm.DataModel[dm.View]]
 
@@ -422,7 +424,11 @@ def write_sdk_to_disk(
             path.unlink()
         path.parent.mkdir(parents=True, exist_ok=True)
         if format_code:
-            file_content = formatter.format_code(file_content)
+            try:
+                file_content = formatter.format_code(file_content)
+            except Exception as e:
+                warnings.warn(InvalidCodeGenerated(file_path, str(e)), stacklevel=2)
+                continue
         # Encoding and newline are set to ensure consistent file writing across platforms
         with path.open("w", encoding="utf-8", newline="\n") as f:
             f.write(file_content)

cognite/pygen/_version.py

@@ -1 +1 @@
-__version__ = "0.99.32"
+__version__ = "0.99.33"

cognite/pygen/warnings.py

@@ -1,6 +1,7 @@
 from __future__ import annotations
 
 import warnings
+from pathlib import Path
 
 from cognite.client.data_classes.data_modeling import ViewId
 
@@ -59,3 +60,23 @@ def __str__(self) -> str:
             f"Name collision detected. The following filter parameter {self.word!r} name is used by pygen."
             "An underscore will be added to this parameter to avoid name collision."
         )
+
+
+class MissingReverseDirectRelationTargetWarning(PygenWarning, UserWarning):
+
+    def __init__(self, target: str, field: str) -> None:
+        self.target = target
+        self.field = field
+
+    def __str__(self) -> str:
+        return f"Target {self.target} does not exists. Skipping reverse direct relation {self.field}."
+
+
+class InvalidCodeGenerated(PygenWarning, UserWarning):
+
+    def __init__(self, filepath: str | Path, error_message: str) -> None:
+        self.filepath = filepath
+        self.error_message = error_message
+
+    def __str__(self) -> str:
+        return f"Invalid code generated in {self.filepath}. {self.error_message}"

docs/CHANGELOG.md

@@ -13,7 +13,7 @@ Changes are grouped as follows
 - `Fixed` for any bug fixes.
 - `Security` in case of vulnerabilities.
 
-## TBD
+## [0.99.33] - 24-08-21
 ### Added
 - Support for `files` and `sequences` in the generated SDK. This includes the ability to create and retrieve
   `files` and `sequences` in the generated SDK.
@@ -26,6 +26,9 @@ Changes are grouped as follows
 ### Fixed
 - In the generated SDK, fields of `TimeSeries` as now set to `TimeSeriesWrite` in the write Data Class of the generated
   SDK.
+- If an input view had a reverse direct relation, that points to a non-existing target, the generated SDK would raise
+  a `ValueError`. `pygen` now gracefully handles this case by raising a warning instead.
+- If a view only had a dependency on itself, `pygen` would generate invalid code. This is now fixed.
 
 ## [0.99.32] - 24-08-17
 ### Added

docs/quickstart/cdf_streamlit.md

@@ -13,7 +13,7 @@ adding `cognite-pygen` to the installed packages under `settings`.
 pyodide-http==0.2.1
 cognite-sdk==7.54.13
 pydantic==1.10.7
-cognite-pygen==0.99.32
+cognite-pygen==0.99.33
 ```
 
 Note that we also set `pydantic` to a specific version. This is because `pygen` supports both `pydantic` `v1` and `v2`, but

examples-pydantic-v1/equipment_unit_pydantic_v1/_api_client.py

@@ -23,7 +23,7 @@ class EquipmentUnitClient:
     EquipmentUnitClient
 
     Generated with:
-        pygen = 0.99.32
+        pygen = 0.99.33
         cognite-sdk = 7.54.13
         pydantic = 1.10.7
 
@@ -41,7 +41,7 @@ def __init__(self, config_or_client: CogniteClient | ClientConfig):
         else:
             raise ValueError(f"Expected CogniteClient or ClientConfig, got {type(config_or_client)}")
         # The client name is used for aggregated logging of Pygen Usage
-        client.config.client_name = "CognitePygen:0.99.32"
+        client.config.client_name = "CognitePygen:0.99.33"
 
         self._client = client
 

examples-pydantic-v1/omni_multi_pydantic_v1/_api_client.py

@@ -111,7 +111,7 @@ class OmniMultiClient:
     OmniMultiClient
 
     Generated with:
-        pygen = 0.99.32
+        pygen = 0.99.33
         cognite-sdk = 7.54.13
         pydantic = 1.10.7
 
@@ -125,7 +125,7 @@ def __init__(self, config_or_client: CogniteClient | ClientConfig):
         else:
             raise ValueError(f"Expected CogniteClient or ClientConfig, got {type(config_or_client)}")
         # The client name is used for aggregated logging of Pygen Usage
-        client.config.client_name = "CognitePygen:0.99.32"
+        client.config.client_name = "CognitePygen:0.99.33"
 
         self.omni_multi_a = OmniMultiAAPIs(client)
         self.omni_multi_b = OmniMultiBAPIs(client)

examples-pydantic-v1/omni_pydantic_v1/_api_client.py

@@ -41,7 +41,7 @@ class OmniClient:
     OmniClient
 
     Generated with:
-        pygen = 0.99.32
+        pygen = 0.99.33
         cognite-sdk = 7.54.13
         pydantic = 1.10.7
 
@@ -59,7 +59,7 @@ def __init__(self, config_or_client: CogniteClient | ClientConfig):
         else:
             raise ValueError(f"Expected CogniteClient or ClientConfig, got {type(config_or_client)}")
         # The client name is used for aggregated logging of Pygen Usage
-        client.config.client_name = "CognitePygen:0.99.32"
+        client.config.client_name = "CognitePygen:0.99.33"
 
         self._client = client
 

examples-pydantic-v1/omni_sub_pydantic_v1/_api_client.py

@@ -23,7 +23,7 @@ class OmniSubClient:
     OmniSubClient
 
     Generated with:
-        pygen = 0.99.32
+        pygen = 0.99.33
         cognite-sdk = 7.54.13
         pydantic = 1.10.7
 
@@ -41,7 +41,7 @@ def __init__(self, config_or_client: CogniteClient | ClientConfig):
         else:
             raise ValueError(f"Expected CogniteClient or ClientConfig, got {type(config_or_client)}")
         # The client name is used for aggregated logging of Pygen Usage
-        client.config.client_name = "CognitePygen:0.99.32"
+        client.config.client_name = "CognitePygen:0.99.33"
 
         self._client = client