Repository: materialsproject/pymatgen

Issue:
Incorrect Default Symmetry Operation in P1 Space Group Returns Three Projection Operations Instead of Identity
### Python version

Python 3.12

### Pymatgen version

2024.10.29

### Operating system version

_No response_

### Current behavior

When a structure does not contain any _symmetry_equiv_pos_as_xyz keys and pymatgen defaults to the P1 space group, the code currently generates three symmetry operations based on ("x", "y", "z"). This results in three separate projection operations along each axis rather than a single identity operation, which P1 space group should have.

in class `pymatgen.io.cif.CifParser`, method `get_symops`
`if not sym_ops:
            msg = "No _symmetry_equiv_pos_as_xyz type key found. Defaulting to P1."
            warnings.warn(msg)
            self.warnings.append(msg)
            sym_ops = [SymmOp.from_xyz_str(s) for s in ("x", "y", "z")]
`

### Expected Behavior

It should be: `sym_ops = [SymmOp.from_xyz_str(s) for s in ["x,y,z"]]` to get correct result.

### Minimal example

_No response_

### Relevant files to reproduce this bug

_No response_

Discussion:

Solution patch:
diff --git a/src/pymatgen/core/operations.py b/src/pymatgen/core/operations.py
index d4821d986a..c7f2a5b50d 100644
--- a/src/pymatgen/core/operations.py
+++ b/src/pymatgen/core/operations.py
@@ -19,7 +19,7 @@ from pymatgen.util.string import transformation_to_string
 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 @@ class SymmOp(MSONable):
         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 @@ class SymmOp(MSONable):
         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 @@ class SymmOp(MSONable):
         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 @@ class SymmOp(MSONable):
         """
         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 @@ class SymmOp(MSONable):
         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 @@ class SymmOp(MSONable):
     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 @@ class SymmOp(MSONable):
                     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
diff --git a/src/pymatgen/io/cif.py b/src/pymatgen/io/cif.py
index 0bb6413402..73d5ec85a5 100644
--- a/src/pymatgen/io/cif.py
+++ b/src/pymatgen/io/cif.py
@@ -820,7 +820,7 @@ class CifParser:
             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
 


Test patch:
diff --git a/tests/io/test_cif.py b/tests/io/test_cif.py
index 040ff45ee9..44fc38dc5d 100644
--- a/tests/io/test_cif.py
+++ b/tests/io/test_cif.py
@@ -2,10 +2,11 @@ from __future__ import annotations
 
 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 @@ class TestCifIO(PymatgenTest):
         ]
         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"