Fix P1 SymmOp string for CifParser.get_symops (#4279)

* use P1 symmop str

* minor type improve

* use NDArray over np.ndarray

* patch a cif with no symmop info available

* assert warning msg

Author: DanielYang59

src/pymatgen/core/operations.py

@@ -19,7 +19,7 @@
 if TYPE_CHECKING:
     from typing import Any
 
-    from numpy.typing import ArrayLike
+    from numpy.typing import ArrayLike, NDArray
     from typing_extensions import Self
 
 __author__ = "Shyue Ping Ong, Shyam Dwaraknath, Matthew Horton"
@@ -31,7 +31,7 @@ class SymmOp(MSONable):
     for efficiency. Read: https://wikipedia.org/wiki/Affine_transformation.
 
     Attributes:
-        affine_matrix (np.ndarray): A 4x4 array representing the symmetry operation.
+        affine_matrix (NDArray): A 4x4 array representing the symmetry operation.
     """
 
     def __init__(
@@ -116,7 +116,7 @@ def from_rotation_and_translation(
         affine_matrix[:3][:, 3] = translation_vec
         return cls(affine_matrix, tol)
 
-    def operate(self, point: ArrayLike) -> np.ndarray:
+    def operate(self, point: ArrayLike) -> NDArray:
         """Apply the operation on a point.
 
         Args:
@@ -128,7 +128,7 @@ def operate(self, point: ArrayLike) -> np.ndarray:
         affine_point = np.asarray([*point, 1])
         return np.dot(self.affine_matrix, affine_point)[:3]
 
-    def operate_multi(self, points: ArrayLike) -> np.ndarray:
+    def operate_multi(self, points: ArrayLike) -> NDArray:
         """Apply the operation on a list of points.
 
         Args:
@@ -141,7 +141,7 @@ def operate_multi(self, points: ArrayLike) -> np.ndarray:
         affine_points = np.concatenate([points, np.ones(points.shape[:-1] + (1,))], axis=-1)
         return np.inner(affine_points, self.affine_matrix)[..., :-1]
 
-    def apply_rotation_only(self, vector: ArrayLike) -> np.ndarray:
+    def apply_rotation_only(self, vector: ArrayLike) -> NDArray:
         """Vectors should only be operated by the rotation matrix and not the
         translation vector.
 
@@ -150,7 +150,7 @@ def apply_rotation_only(self, vector: ArrayLike) -> np.ndarray:
         """
         return np.dot(self.rotation_matrix, vector)
 
-    def transform_tensor(self, tensor: np.ndarray) -> np.ndarray:
+    def transform_tensor(self, tensor: NDArray) -> NDArray:
         """Apply rotation portion to a tensor. Note that tensor has to be in
         full form, not the Voigt form.
 
@@ -239,12 +239,12 @@ def are_symmetrically_related_vectors(
         return False, False
 
     @property
-    def rotation_matrix(self) -> np.ndarray:
+    def rotation_matrix(self) -> NDArray:
         """A 3x3 numpy.array representing the rotation matrix."""
         return self.affine_matrix[:3][:, :3]
 
     @property
-    def translation_vector(self) -> np.ndarray:
+    def translation_vector(self) -> NDArray:
         """A rank 1 numpy.array of dim 3 representing the translation vector."""
         return self.affine_matrix[:3][:, 3]
 
@@ -462,16 +462,17 @@ def as_xyz_str(self) -> str:
     def from_xyz_str(cls, xyz_str: str) -> Self:
         """
         Args:
-            xyz_str: string of the form 'x, y, z', '-x, -y, z', '-2y+1/2, 3x+1/2, z-y+1/2', etc.
+            xyz_str (str): "x, y, z", "-x, -y, z", "-2y+1/2, 3x+1/2, z-y+1/2", etc.
 
         Returns:
             SymmOp
         """
-        rot_matrix = np.zeros((3, 3))
-        trans = np.zeros(3)
-        tokens = xyz_str.strip().replace(" ", "").lower().split(",")
+        rot_matrix: NDArray = np.zeros((3, 3))
+        trans: NDArray = np.zeros(3)
+        tokens: list[str] = xyz_str.strip().replace(" ", "").lower().split(",")
         re_rot = re.compile(r"([+-]?)([\d\.]*)/?([\d\.]*)([x-z])")
         re_trans = re.compile(r"([+-]?)([\d\.]+)/?([\d\.]*)(?![x-z])")
+
         for idx, tok in enumerate(tokens):
             # Build the rotation matrix
             for match in re_rot.finditer(tok):
@@ -480,11 +481,13 @@ def from_xyz_str(cls, xyz_str: str) -> Self:
                     factor *= float(match[2]) / float(match[3]) if match[3] != "" else float(match[2])
                 j = ord(match[4]) - 120
                 rot_matrix[idx, j] = factor
+
             # Build the translation vector
             for match in re_trans.finditer(tok):
                 factor = -1 if match[1] == "-" else 1
                 num = float(match[2]) / float(match[3]) if match[3] != "" else float(match[2])
                 trans[idx] = num * factor
+
         return cls.from_rotation_and_translation(rot_matrix, trans)
 
     @classmethod

src/pymatgen/io/cif.py

@@ -820,7 +820,7 @@ def get_symops(self, data: CifBlock) -> list[SymmOp]:
             msg = "No _symmetry_equiv_pos_as_xyz type key found. Defaulting to P1."
             warnings.warn(msg, stacklevel=2)
             self.warnings.append(msg)
-            sym_ops = [SymmOp.from_xyz_str(s) for s in ("x", "y", "z")]
+            sym_ops = [SymmOp.from_xyz_str("x, y, z")]
 
         return sym_ops
 

tests/io/test_cif.py

@@ -2,10 +2,11 @@
 
 import numpy as np
 import pytest
+from monty.tempfile import ScratchDir
 from pytest import approx
 
 from pymatgen.analysis.structure_matcher import StructureMatcher
-from pymatgen.core import Composition, DummySpecies, Element, Lattice, Species, Structure
+from pymatgen.core import Composition, DummySpecies, Element, Lattice, Species, Structure, SymmOp
 from pymatgen.electronic_structure.core import Magmom
 from pymatgen.io.cif import CifBlock, CifParser, CifWriter
 from pymatgen.symmetry.structure import SymmetrizedStructure
@@ -243,6 +244,23 @@ def test_get_symmetrized_structure(self):
         ]
         assert set(sym_structure.labels) == {"O1", "Li1"}
 
+    def test_no_sym_ops(self):
+        with ScratchDir("."):
+            with open(f"{TEST_FILES_DIR}/cif/Li2O.cif") as fin, open("test.cif", "w") as fout:
+                for line_num, line in enumerate(fin, start=1):
+                    # Skip "_symmetry_equiv_pos_as_xyz", "_symmetry_space_group_name_H-M"
+                    # and "_symmetry_Int_Tables_number" sections such that
+                    # no symmop info would be available
+                    if line_num not in range(44, 236) and line_num not in {40, 41}:
+                        fout.write(line)
+
+            parser = CifParser("test.cif")
+
+        # Need `parse_structures` call to update `symmetry_operations`
+        _structure = parser.parse_structures(primitive=False, symmetrized=False)[0]
+        assert parser.symmetry_operations[0] == SymmOp.from_xyz_str("x, y, z")
+        assert any("No _symmetry_equiv_pos_as_xyz type key found" in msg for msg in parser.warnings)
+
     def test_site_symbol_preference(self):
         parser = CifParser(f"{TEST_FILES_DIR}/cif/site_type_symbol_test.cif")
         assert parser.parse_structures()[0].formula == "Ge1.6 Sb1.6 Te4"