Repository: Materials-Consortia/optimade-python-tools (python)

Issue:
Deserialization regressions: cannot resolve child databases in client
In "v1", the client can no longer resolve child databases, since when deserializing a `LinksResponse`, the `LinkType` enum is not present in the final model so a filter that checks `link["attributes"]["link_type"] == LinkType.CHILD` does not trigger.

I'm slightly worried that this opens us up to many more occurrences of this, so I would say this is a blocking issue. Our tests did not run into it as we do test the client on "live" provider data (maybe we should -- the test could just be whether there are ANY accessible OPTIMADE APIs out there, if this test fails for non-code reasons then we can probably roll up this package anyway 🙃)

Pull request description:
Fix incorrect 'smart' union discrimination
Closes #1843.

pydantic v2 made "smart" union discrimination by default, which seems to favour dicts over models.

For now I've made it so that any `Union[Model, dict[str, Any]]` fields use the newly-added workaround in pydantic v2.3, `union_mode = 'left_to_right'`. Unfortunately we can't set this globally at the level of `StrictField` as it crashes out in schema generation if the schema type is not `'union'`. I might make an issue about this on pydantic.

@CasperWA you may have thoughts -- will probably merge but feel free to do a post-review!

Patch:
diff --git a/optimade/models/responses.py b/optimade/models/responses.py
index a3716f5e..9cc4e1b4 100644
--- a/optimade/models/responses.py
+++ b/optimade/models/responses.py
@@ -70,12 +70,19 @@ class InfoResponse(Success):
 
 
 class EntryResponseOne(Success):
-    data: Optional[Union[EntryResource, dict[str, Any]]] = None  # type: ignore[assignment]
+    data: Annotated[
+        Optional[Union[EntryResource, dict[str, Any]]],
+        StrictField(
+            description="The single entry resource returned by this query.",
+            union_mode="left_to_right",
+        ),
+    ] = None  # type: ignore[assignment]
     included: Annotated[
         Optional[Union[list[EntryResource], list[dict[str, Any]]]],
         StrictField(
             description="A list of unique included OPTIMADE entry resources.",
             uniqueItems=True,
+            union_mode="left_to_right",
         ),
     ] = None  # type: ignore[assignment]
 
@@ -86,6 +93,7 @@ class EntryResponseMany(Success):
         StrictField(
             description="List of unique OPTIMADE entry resource objects.",
             uniqueItems=True,
+            union_mode="left_to_right",
         ),
     ]
     included: Annotated[
@@ -93,6 +101,7 @@ class EntryResponseMany(Success):
         StrictField(
             description="A list of unique included OPTIMADE entry resources.",
             uniqueItems=True,
+            union_mode="left_to_right",
         ),
     ] = None  # type: ignore[assignment]
 
@@ -103,6 +112,7 @@ class LinksResponse(EntryResponseMany):
         StrictField(
             description="List of unique OPTIMADE links resource objects.",
             uniqueItems=True,
+            union_mode="left_to_right",
         ),
     ]
 
@@ -110,7 +120,10 @@ class LinksResponse(EntryResponseMany):
 class StructureResponseOne(EntryResponseOne):
     data: Annotated[
         Optional[Union[StructureResource, dict[str, Any]]],
-        StrictField(description="A single structures entry resource."),
+        StrictField(
+            description="A single structures entry resource.",
+            union_mode="left_to_right",
+        ),
     ]
 
 
@@ -120,6 +133,7 @@ class StructureResponseMany(EntryResponseMany):
         StrictField(
             description="List of unique OPTIMADE structures entry resource objects.",
             uniqueItems=True,
+            union_mode="left_to_right",
         ),
     ]
 
@@ -127,7 +141,10 @@ class StructureResponseMany(EntryResponseMany):
 class ReferenceResponseOne(EntryResponseOne):
     data: Annotated[
         Optional[Union[ReferenceResource, dict[str, Any]]],
-        StrictField(description="A single references entry resource."),
+        StrictField(
+            description="A single references entry resource.",
+            union_mode="left_to_right",
+        ),
     ]
 
 
@@ -137,5 +154,6 @@ class ReferenceResponseMany(EntryResponseMany):
         StrictField(
             description="List of unique OPTIMADE references entry resource objects.",
             uniqueItems=True,
+            union_mode="left_to_right",
         ),
     ]
diff --git a/pyproject.toml b/pyproject.toml
index c7f79468..aa4cc7d7 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -26,7 +26,7 @@ classifiers = [
 requires-python = ">=3.9"
 dependencies = [
     "lark~=1.1",
-    "pydantic[email]~=2.0",
+    "pydantic[email]~=2.2",
     "pydantic-settings~=2.0",
     "requests~=2.28",
 ]


Test patch:
diff --git a/tests/models/test_links.py b/tests/models/test_links.py
index c65cc9ed..99b5947d 100644
--- a/tests/models/test_links.py
+++ b/tests/models/test_links.py
@@ -1,7 +1,8 @@
 # pylint: disable=no-member
 import pytest
 
-from optimade.models.links import LinksResource
+from optimade.models import LinksResponse
+from optimade.models.links import Aggregate, LinksResource, LinkType
 
 MAPPER = "LinksMapper"
 
@@ -9,7 +10,35 @@ MAPPER = "LinksMapper"
 def test_good_links(starting_links, mapper):
     """Check well-formed links used as example data"""
     # Test starting_links is a good links resource
-    LinksResource(**mapper(MAPPER).map_back(starting_links))
+    resource = LinksResource(**mapper(MAPPER).map_back(starting_links))
+    assert resource.attributes.link_type == LinkType.CHILD
+    assert resource.attributes.aggregate == Aggregate.TEST
+
+
+def test_edge_case_links():
+    response = LinksResponse(
+        data=[
+            {
+                "id": "aflow",
+                "type": "links",
+                "attributes": {
+                    "name": "AFLOW",
+                    "description": "The AFLOW OPTIMADE endpoint",
+                    "base_url": "http://aflow.org/API/optimade/",
+                    "homepage": "http://aflow.org",
+                    "link_type": "child",
+                },
+            },
+        ],
+        meta={
+            "query": {"representation": "/links"},
+            "more_data_available": False,
+            "api_version": "1.0.0",
+        },
+    )
+
+    assert isinstance(response.data[0], LinksResource)
+    assert response.data[0].attributes.link_type == LinkType.CHILD
 
 
 def test_bad_links(starting_links, mapper):
diff --git a/tests/models/test_structures.py b/tests/models/test_structures.py
index 828c9f3c..7301c0d3 100644
--- a/tests/models/test_structures.py
+++ b/tests/models/test_structures.py
@@ -4,6 +4,7 @@ from typing import TYPE_CHECKING
 
 import pytest
 
+from optimade.models.structures import Periodicity, StructureFeatures
 from optimade.warnings import MissingExpectedField
 
 if TYPE_CHECKING:
@@ -55,7 +56,11 @@ def test_more_good_structures(
 
     for index, structure in enumerate(good_structures):
         try:
-            StructureResource(**mapper(MAPPER).map_back(structure))
+            s = StructureResource(**mapper(MAPPER).map_back(structure))
+            if s.attributes.structure_features:
+                assert isinstance(s.attributes.structure_features[0], StructureFeatures)
+            for dim in s.attributes.dimension_types:
+                assert isinstance(dim, Periodicity)
         except ValidationError:
             # Printing to keep the original exception as is, while still being informational
             print(