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

Issue:
Add validation that anonymous/reduced chemical formulae are in fact reduced
Prompted by https://github.com/aiidateam/aiida-optimade/issues/279.

We should add validators that check that the chemical formulae presented in `chemical_formula_reduced` and `chemical_formula_anonymous` are in fact reduced, e.g. that for A<sub>x</sub>B<sub>y</sub>C<sub>z</sub>, `math.gcd([x, y, z]) == 1`.

This is important to truly enable filtering on chemical formulae.

Pull request:
Add validator to check whether anonymous and reduced formulae are reduced
Closes #913.

Patch:
diff --git a/optimade/models/structures.py b/optimade/models/structures.py
index 5d9ab1aa..00c6d059 100644
--- a/optimade/models/structures.py
+++ b/optimade/models/structures.py
@@ -1,6 +1,9 @@
 # pylint: disable=no-self-argument,line-too-long,no-name-in-module
 import re
 import warnings
+import sys
+import math
+from functools import reduce
 from enum import IntEnum, Enum
 from sys import float_info
 from typing import List, Optional, Union
@@ -874,7 +877,7 @@ The properties of the species are found in the property `species`.
         elements = tuple(re.findall(r"[A-Z][a-z]*", v))
         numbers = [int(n.strip()) for n in re.split(r"[A-Z][a-z]*", v) if n.strip()]
 
-        if any(n == 1 for n in numbers):
+        if 1 in numbers:
             raise ValueError(
                 f"'chemical_formula_anonymous' {v} must omit proportion '1'"
             )
@@ -893,6 +896,32 @@ The properties of the species are found in the property `species`.
 
         return v
 
+    @validator("chemical_formula_anonymous", "chemical_formula_reduced")
+    def check_reduced_formulae(cls, value, field):
+        if value is None:
+            return value
+
+        numbers = [n.strip() or 1 for n in re.split(r"[A-Z][a-z]*", value)]
+        # Need to remove leading 1 from split and convert to ints
+        numbers = [int(n) for n in numbers[1:]]
+
+        if 0 in numbers:
+            raise ValueError(
+                f"{field.name} {value!r} cannot contain chemical proportion of 0."
+            )
+
+        if sys.version_info[1] >= 9:
+            gcd = math.gcd(*numbers)
+        else:
+            gcd = reduce(math.gcd, numbers)
+
+        if gcd != 1:
+            raise ValueError(
+                f"{field.name} {value!r} is not properly reduced: greatest common divisor was {gcd}, expected 1."
+            )
+
+        return value
+
     @validator("elements", each_item=True)
     def element_must_be_chemical_symbol(cls, v):
         if v not in CHEMICAL_SYMBOLS:
diff --git a/optimade/validator/__init__.py b/optimade/validator/__init__.py
index 2ef9a886..ccbdf226 100644
--- a/optimade/validator/__init__.py
+++ b/optimade/validator/__init__.py
@@ -1,5 +1,6 @@
 """ This module contains the ImplementationValidator class and corresponding command line tools. """
 # pylint: disable=import-outside-toplevel
+from optimade import __version__, __api_version__
 from .validator import ImplementationValidator
 from .utils import DEFAULT_CONN_TIMEOUT
 
@@ -98,6 +99,12 @@ def validate():  # pragma: no cover
         help="Additional HTTP headers to use for each request, specified as a JSON object.",
     )
 
+    parser.add_argument(
+        "--version",
+        action="version",
+        version=f"optimade-validator running from optimade-python-tools v{__version__} which implements OPTIMADE specification v{__api_version__}.",
+    )
+
     parser.add_argument(
         "--timeout",
         type=float,
diff --git a/optimade/validator/validator.py b/optimade/validator/validator.py
index 00d2403d..411d5004 100644
--- a/optimade/validator/validator.py
+++ b/optimade/validator/validator.py
@@ -321,12 +321,6 @@ class ImplementationValidator:
                 entry_info_endpoint
             )
 
-        # Use the _entry_info_by_type to construct filters on the relevant endpoints
-        if not self.minimal:
-            for endp in self.available_json_endpoints:
-                self._log.debug("Testing queries on JSON entry endpoint of %s", endp)
-                self._recurse_through_endpoint(endp)
-
         # Test that the results from multi-entry-endpoints obey, e.g. page limits,
         # and that all entries can be deserialized with the patched models.
         # These methods also set the test_ids for each type of entry, which are validated
@@ -340,6 +334,12 @@ class ImplementationValidator:
             self._log.debug("Testing single entry request of type %s", endp)
             self._test_single_entry_endpoint(endp)
 
+        # Use the _entry_info_by_type to construct filters on the relevant endpoints
+        if not self.minimal:
+            for endp in self.available_json_endpoints:
+                self._log.debug("Testing queries on JSON entry endpoint of %s", endp)
+                self._recurse_through_endpoint(endp)
+
         # Test that the links endpoint can be serialized correctly
         self._log.debug("Testing %s endpoint", CONF.links_endpoint)
         self._test_info_or_links_endpoint(CONF.links_endpoint)
@@ -998,9 +998,6 @@ class ImplementationValidator:
         """Requests and deserializes a multi-entry endpoint with the
         appropriate model.
 
-        TODO: deserialization is currently classed as an optional
-        test until our models are robust to support levels.
-
         Parameters:
             request_str: The multi-entry request to make, e.g.,
                 "structures?filter=nsites<10"


Test patch:
diff --git a/tests/models/test_structures.py b/tests/models/test_structures.py
index 84979385..4fa91118 100644
--- a/tests/models/test_structures.py
+++ b/tests/models/test_structures.py
@@ -166,6 +166,22 @@ deformities = (
         {"chemical_formula_reduced": "Ag6 Cl2"},
         "chemical_formula_reduced\n  string does not match regex",
     ),
+    (
+        {"chemical_formula_reduced": "Ge2Si2"},
+        "chemical_formula_reduced 'Ge2Si2' is not properly reduced: greatest common divisor was 2, expected 1.",
+    ),
+    (
+        {"chemical_formula_reduced": "Ge144Si60V24"},
+        "chemical_formula_reduced 'Ge144Si60V24' is not properly reduced: greatest common divisor was 12, expected 1.",
+    ),
+    (
+        {"chemical_formula_anonymous": "A10B5C5"},
+        "chemical_formula_anonymous 'A10B5C5' is not properly reduced: greatest common divisor was 5, expected 1.",
+    ),
+    (
+        {"chemical_formula_anonymous": "A44B15C9D4E3F2GHI0J0K0L0"},
+        "chemical_formula_anonymous 'A44B15C9D4E3F2GHI0J0K0L0' cannot contain chemical proportion of 0.",
+    ),
 )