Update models according to changes during CECAM 2020 meeting (#350)

* Added nperiodic_dimensions to all test inputs

* Added nperiodic_dimensions field to structure model

* Added bad structure data for mismatched nperiodic_dimensions/dimension_types

* Updated adapter tests with new field

* Add attached and nattached + site_attachments

site_attachments is a flag under structure_features.
attached and nattached are fields for a species.

* Add implicit_atoms to structure_features

Currently it tests whether a species is represented in species_at_sites,
however, implicit_atoms MUST also be set for some assemblies, this
validation should be added.

* unknown_positions is no longer

It used to be a flag in structure_features, but has been replaced by
implicit_atoms and site_attachments.

* Add tests, remove handling of unknown positions

Several conversion functions for the StructureAdapter specifically
handled None values in cartesian_site_positions, which is no longer an
option, hence, these extra steps have been removed from the affected
conversion functions.

* Add aggregate and no_aggregate_reason

These are optional properties for a LinksResource.

* Add type to entry listing info properties

* Add type field to all properties

* Use Enums for list of literal values

* Use enumerations for other literal values

* Add tests for DataType methods

* Add more "bad" structures for testing validators

* Add new EOL for update OpenAPI tasks

* Use test_more_structures.json

Using again test_more_structures.json revealed some errors with the new
validators, as well as some updates that needed to be done to existing
validators.

In order to properly validate `species_at_sites` it has to be
instantiated _after_ `species`.

* Use Aggregate Enum for correct property ...

* Restructure tests/models

Move to pytest.
Add fixtures for loading test data.

* Add LinksResource tests

Co-authored-by: Casper Welzel Andersen <casper.andersen@epfl.ch>

openapi/index_openapi.json

@@ -1012,8 +1012,29 @@
           },
           "link_type": {
             "title": "Link Type",
-            "type": "string",
+            "enum": [
+              "child",
+              "root",
+              "external",
+              "providers"
+            ],
             "description": "The link type of the represented resource in relation to this implementation. MUST be one of these values: 'child', 'root', 'external', 'providers'."
+          },
+          "aggregate": {
+            "title": "Aggregate",
+            "enum": [
+              "ok",
+              "test",
+              "staging",
+              "no"
+            ],
+            "description": "A string indicating whether a client that is following links to aggregate results from different OPTIMADE implementations should follow this link or not.\nThis flag SHOULD NOT be indicated for links where :property:`link_type` is not :val:`child`.\n\nIf not specified, clients MAY assume that the value is :val:`ok`.\nIf specified, and the value is anything different than :val:`ok`, the client MUST assume that the server is suggesting not to follow the link during aggregation by default (also if the value is not among the known ones, in case a future specification adds new accepted values).\n\nSpecific values indicate the reason why the server is providing the suggestion.\nA client MAY follow the link anyway if it has reason to do so (e.g., if the client is looking for all test databases, it MAY follow the links marked with :property:`aggregate`=:val:`test`).\n\nIf specified, it MUST be one of the values listed in section Link Aggregate Options.",
+            "default": "ok"
+          },
+          "no_aggregate_reason": {
+            "title": "No Aggregate Reason",
+            "type": "string",
+            "description": "An OPTIONAL human-readable string indicating the reason for suggesting not to aggregate results following the link.\nIt SHOULD NOT be present if :property:`aggregate`=:val:`ok`."
           }
         },
         "description": "Links endpoint resource object attributes"

optimade/adapters/structures/aiida.py

@@ -1,6 +1,6 @@
 from optimade.models import StructureResource as OptimadeStructure
 
-from optimade.adapters.structures.utils import pad_cell, pad_positions
+from optimade.adapters.structures.utils import pad_cell
 
 try:
     from aiida.orm.nodes.data.structure import StructureData, Kind, Site
@@ -54,16 +54,13 @@ def get_aiida_structure_data(optimade_structure: OptimadeStructure) -> Structure
             Kind(symbols=symbols, weights=concentration, mass=mass, name=kind.name)
         )
 
-    # Convert null/None values to float("nan")
-    cartesian_site_positions, _ = pad_positions(attributes.cartesian_site_positions)
-
     # Add Sites
     for index in range(attributes.nsites):
         # range() to ensure 1-to-1 between kind and site
         structure.append_site(
             Site(
                 kind_name=attributes.species_at_sites[index],
-                position=cartesian_site_positions[index],
+                position=attributes.cartesian_site_positions[index],
             )
         )
 

optimade/adapters/structures/ase.py

@@ -3,6 +3,7 @@
 
 from optimade.models import Species as OptimadeStructureSpecies
 from optimade.models import StructureResource as OptimadeStructure
+from optimade.models import StructureFeatures
 
 from optimade.adapters.exceptions import ConversionError
 
@@ -31,7 +32,7 @@ def get_ase_atoms(optimade_structure: OptimadeStructure) -> Atoms:
     attributes = optimade_structure.attributes
 
     # Cannot handle partial occupancies
-    if "disorder" in attributes.structure_features:
+    if StructureFeatures.DISORDER in attributes.structure_features:
         raise ConversionError(
             "ASE cannot handle structures with partial occupancies, sorry."
         )

optimade/adapters/structures/cif.py

@@ -5,7 +5,6 @@
 
 from optimade.adapters.structures.utils import (
     cell_to_cellpar,
-    pad_positions,
     fractional_coordinates,
 )
 
@@ -78,9 +77,9 @@ def get_cif(  # pylint: disable=too-many-locals,too-many-branches
         # Since some structure viewers are having issues with cartesian coordinates,
         # we calculate the fractional coordinates if this is a 3D structure and we have all the necessary information.
         if not hasattr(attributes, "fractional_site_positions"):
-            sites, _ = pad_positions(attributes.cartesian_site_positions)
             attributes.fractional_site_positions = fractional_coordinates(
-                cell=attributes.lattice_vectors, cartesian_positions=sites
+                cell=attributes.lattice_vectors,
+                cartesian_positions=attributes.cartesian_site_positions,
             )
 
     # NOTE: This is otherwise a bit ahead of its time, since this OPTIMADE property is part of an open PR.
@@ -102,9 +101,9 @@ def get_cif(  # pylint: disable=too-many-locals,too-many-branches
     )
 
     if coord_type == "fract":
-        sites, _ = pad_positions(attributes.fractional_site_positions)
+        sites = attributes.fractional_site_positions
     else:
-        sites, _ = pad_positions(attributes.cartesian_site_positions)
+        sites = attributes.cartesian_site_positions
 
     species: Dict[str, OptimadeStructureSpecies] = {
         species.name: species for species in attributes.species

optimade/adapters/structures/jarvis.py

@@ -1,7 +1,7 @@
 from warnings import warn
 from optimade.models import StructureResource as OptimadeStructure
+from optimade.models import StructureFeatures
 from optimade.adapters.exceptions import ConversionError
-from optimade.adapters.structures.utils import pad_positions
 
 try:
     from jarvis.core.atoms import Atoms
@@ -28,16 +28,14 @@ def get_jarvis_atoms(optimade_structure: OptimadeStructure) -> Atoms:
     attributes = optimade_structure.attributes
 
     # Cannot handle partial occupancies
-    if "disorder" in attributes.structure_features:
+    if StructureFeatures.DISORDER in attributes.structure_features:
         raise ConversionError(
             "jarvis-tools cannot handle structures with partial occupancies."
         )
 
-    cartesian_site_positions, _ = pad_positions(attributes.cartesian_site_positions)
-
     return Atoms(
         lattice_mat=attributes.lattice_vectors,
         elements=[specie.name for specie in attributes.species],
-        coords=cartesian_site_positions,
+        coords=attributes.cartesian_site_positions,
         cartesian=True,
     )

optimade/adapters/structures/proteindatabank.py

@@ -16,7 +16,6 @@
     cell_to_cellpar,
     cellpar_to_cell,
     fractional_coordinates,
-    pad_positions,
     scaled_cell,
 )
 
@@ -80,9 +79,9 @@ def get_pdbx_mmcif(  # pylint: disable=too-many-locals
         # Since some structure viewers are having issues with cartesian coordinates,
         # we calculate the fractional coordinates if this is a 3D structure and we have all the necessary information.
         if not hasattr(attributes, "fractional_site_positions"):
-            sites, _ = pad_positions(attributes.cartesian_site_positions)
             attributes.fractional_site_positions = fractional_coordinates(
-                cell=attributes.lattice_vectors, cartesian_positions=sites
+                cell=attributes.lattice_vectors,
+                cartesian_positions=attributes.cartesian_site_positions,
             )
 
     # TODO: The following lines are perhaps needed to create a "valid" PDBx/mmCIF file.
@@ -134,9 +133,9 @@ def get_pdbx_mmcif(  # pylint: disable=too-many-locals
     )
 
     if coord_type == "fract":
-        sites, _ = pad_positions(attributes.fractional_site_positions)
+        sites = attributes.fractional_site_positions
     else:
-        sites, _ = pad_positions(attributes.cartesian_site_positions)
+        sites = attributes.cartesian_site_positions
 
     species: Dict[str, OptimadeStructureSpecies] = {
         species.name: species for species in attributes.species
@@ -216,8 +215,7 @@ def get_pdb(  # pylint: disable=too-many-locals
         species.name: species for species in attributes.species
     }
 
-    cartesian_site_positions, _ = pad_positions(attributes.cartesian_site_positions)
-    sites = np.asarray(cartesian_site_positions)
+    sites = np.asarray(attributes.cartesian_site_positions)
     if rotation is not None:
         sites = sites.dot(rotation)
 

optimade/adapters/structures/pymatgen.py

@@ -3,8 +3,6 @@
 from optimade.models import Species as OptimadeStructureSpecies
 from optimade.models import StructureResource as OptimadeStructure
 
-from optimade.adapters.structures.utils import pad_positions
-
 try:
     from pymatgen import Structure, Molecule
 
@@ -40,16 +38,14 @@ def _get_structure(optimade_structure: OptimadeStructure) -> Structure:
 
     attributes = optimade_structure.attributes
 
-    cartesian_site_positions, _ = pad_positions(attributes.cartesian_site_positions)
-
     return Structure(
         lattice=attributes.lattice_vectors,
         species=_pymatgen_species(
             nsites=attributes.nsites,
             species=attributes.species,
             species_at_sites=attributes.species_at_sites,
         ),
-        coords=cartesian_site_positions,
+        coords=attributes.cartesian_site_positions,
         coords_are_cartesian=True,
     )
 
@@ -59,15 +55,13 @@ def _get_molecule(optimade_structure: OptimadeStructure) -> Molecule:
 
     attributes = optimade_structure.attributes
 
-    cartesian_site_positions, _ = pad_positions(attributes.cartesian_site_positions)
-
     return Molecule(
         species=_pymatgen_species(
             nsites=attributes.nsites,
             species=attributes.species,
             species_at_sites=attributes.species_at_sites,
         ),
-        coords=cartesian_site_positions,
+        coords=attributes.cartesian_site_positions,
     )
 
 

optimade/adapters/structures/utils.py

@@ -228,15 +228,6 @@ def _pad_iter_of_iters(
     return iterable, padded_iterable
 
 
-def pad_positions(
-    positions: List[Vector3D], padding: float = None
-) -> Tuple[List[Vector3D], bool]:
-    """Turn any null/None values into a float in given list of positions"""
-    return _pad_iter_of_iters(
-        iterable=positions, padding=padding, outer=list, inner=tuple,
-    )
-
-
 def pad_cell(
     lattice_vectors: Tuple[Vector3D, Vector3D, Vector3D], padding: float = None
 ) -> Tuple[Tuple[Vector3D, Vector3D, Vector3D], bool]:

optimade/models/entries.py

@@ -4,7 +4,7 @@
 from pydantic import BaseModel, Field, validator  # pylint: disable=no-name-in-module
 
 from .jsonapi import Relationships, Attributes, Resource
-from .optimade_json import Relationship
+from .optimade_json import Relationship, DataType
 
 
 __all__ = (
@@ -150,6 +150,11 @@ class EntryInfoProperty(BaseModel):
         "If the entry listing endpoint supports sorting, this key MUST be present for sortable properties with value `true`.",
     )
 
+    type: Optional[DataType] = Field(
+        None,
+        description="Data type of value. Must equal a valid OPTIMADE data type as listed and defined under 'Data types'.",
+    )
+
 
 class EntryInfoResource(BaseModel):
 

optimade/models/index_metadb.py

@@ -1,5 +1,7 @@
 # pylint: disable=no-self-argument
-from pydantic import Field, BaseModel, validator  # pylint: disable=no-name-in-module
+from enum import Enum
+
+from pydantic import Field, BaseModel  # pylint: disable=no-name-in-module
 from typing import Union, Dict
 
 from .jsonapi import BaseResource
@@ -14,6 +16,12 @@
 )
 
 
+class DefaultRelationship(Enum):
+    """Enumeration of key(s) for relationship dictionary in IndexInfoResource"""
+
+    DEFAULT = "default"
+
+
 class IndexInfoAttributes(BaseInfoAttributes):
     """Attributes for Base URL Info endpoint for an Index Meta-Database"""
 
@@ -46,18 +54,10 @@ class IndexInfoResource(BaseInfoResource):
     """Index Meta-Database Base URL Info endpoint resource"""
 
     attributes: IndexInfoAttributes = Field(...)
-    relationships: Union[None, Dict[str, IndexRelationship]] = Field(
+    relationships: Union[None, Dict[DefaultRelationship, IndexRelationship]] = Field(
         ...,
         description="Reference to the child identifier object under the links endpoint "
         "that the provider has chosen as their 'default' OPTIMADE API database. "
         "A client SHOULD present this database as the first choice when an end-user "
         "chooses this provider.",
     )
-
-    @validator("relationships")
-    def relationships_key_must_be_default(cls, value):
-        if value is not None and all([key != "default" for key in value]):
-            raise ValueError(
-                "if the relationships value is a dict, the key MUST be 'default'"
-            )
-        return value

optimade/models/links.py

@@ -1,11 +1,12 @@
 # pylint: disable=no-self-argument
+from enum import Enum
+
 from pydantic import (  # pylint: disable=no-name-in-module
     Field,
     AnyUrl,
-    validator,
     root_validator,
 )
-from typing import Union
+from typing import Union, Optional
 
 from .jsonapi import Link, Attributes
 from .entries import EntryResource
@@ -17,6 +18,24 @@
 )
 
 
+class LinkType(Enum):
+    """Enumeration of link_type values"""
+
+    CHILD = "child"
+    ROOT = "root"
+    EXTERNAL = "external"
+    PROVIDERS = "providers"
+
+
+class Aggregate(Enum):
+    """Enumeration of aggregate values"""
+
+    OK = "ok"
+    TEST = "test"
+    STAGING = "staging"
+    NO = "no"
+
+
 class LinksResourceAttributes(Attributes):
     """Links endpoint resource object attributes"""
 
@@ -40,18 +59,30 @@ class LinksResourceAttributes(Attributes):
         description="JSON API links object, pointing to a homepage URL for this implementation",
     )
 
-    link_type: str = Field(
+    link_type: LinkType = Field(
         ...,
         description="The link type of the represented resource in relation to this implementation. MUST be one of these values: 'child', 'root', 'external', 'providers'.",
     )
 
-    @validator("link_type")
-    def link_type_must_be_in_specific_set(cls, value):
-        if value not in {"child", "root", "external", "providers"}:
-            raise ValueError(
-                "link_type MUST be either 'child, 'root', 'external', or 'providers'"
-            )
-        return value
+    aggregate: Optional[Aggregate] = Field(
+        "ok",
+        description="""A string indicating whether a client that is following links to aggregate results from different OPTIMADE implementations should follow this link or not.
+This flag SHOULD NOT be indicated for links where :property:`link_type` is not :val:`child`.
+
+If not specified, clients MAY assume that the value is :val:`ok`.
+If specified, and the value is anything different than :val:`ok`, the client MUST assume that the server is suggesting not to follow the link during aggregation by default (also if the value is not among the known ones, in case a future specification adds new accepted values).
+
+Specific values indicate the reason why the server is providing the suggestion.
+A client MAY follow the link anyway if it has reason to do so (e.g., if the client is looking for all test databases, it MAY follow the links marked with :property:`aggregate`=:val:`test`).
+
+If specified, it MUST be one of the values listed in section Link Aggregate Options.""",
+    )
+
+    no_aggregate_reason: Optional[str] = Field(
+        None,
+        description="""An OPTIONAL human-readable string indicating the reason for suggesting not to aggregate results following the link.
+It SHOULD NOT be present if :property:`aggregate`=:val:`ok`.""",
+    )
 
 
 class LinksResource(EntryResource):