Revert back TransformedStructure.__getattr__ (materialsproject#3617)

* Revert changes on __getattr__

* pre-commit auto-fixes

* object.__getattribute__(self) -> self.__getattribute__()

* add test for no infinite recursion (would have failed before this fix)

* refactor

---------

Co-authored-by: Janosh Riebesell <[email protected]>

Author: mjwen

.github/CODEOWNERS

@@ -1,4 +1,4 @@
 * @shyuep @janosh @mkhorton
 pymatgen/io/ase.py @Andrew-S-Rosen
 pymatgen/io/abinit/* @gmatteo
-pymatgen/io/lobster/* @JaGeo
+pymatgen/io/lobster/* @JaGeo

docs/usage.md

@@ -17,11 +17,13 @@
 from pymatgen.io.cif import CifParser
 from pymatgen.io.vasp.inputs import Poscar
 from pymatgen.io.vasp.sets import MPRelaxSet, VaspInputSet
+from pymatgen.transformations.transformation_abc import AbstractTransformation
 from pymatgen.util.provenance import StructureNL
 
 if TYPE_CHECKING:
+    from collections.abc import Sequence
+
     from pymatgen.alchemy.filters import AbstractStructureFilter
-    from pymatgen.transformations.transformation_abc import AbstractTransformation
 
 
 class TransformedStructure(MSONable):
@@ -35,16 +37,15 @@ class TransformedStructure(MSONable):
     def __init__(
         self,
         structure: Structure,
-        transformations: list[AbstractTransformation] | None = None,
+        transformations: AbstractTransformation | Sequence[AbstractTransformation] | None = None,
         history: list[AbstractTransformation | dict[str, Any]] | None = None,
         other_parameters: dict[str, Any] | None = None,
     ) -> None:
         """Initializes a transformed structure from a structure.
 
         Args:
             structure (Structure): Input structure
-            transformations (list[Transformation]): List of transformations to
-                apply.
+            transformations (list[Transformation]): List of transformations to apply.
             history (list[Transformation]): Previous history.
             other_parameters (dict): Additional parameters to be added.
         """
@@ -53,6 +54,8 @@ def __init__(
         self.other_parameters = other_parameters or {}
         self._undone: list[tuple[AbstractTransformation | dict[str, Any], Structure]] = []
 
+        if isinstance(transformations, AbstractTransformation):
+            transformations = [transformations]
         transformations = transformations or []
         for trafo in transformations:
             self.append_transformation(trafo)
@@ -86,8 +89,10 @@ def redo_next_change(self) -> None:
         self.history.append(hist)
         self.final_structure = struct
 
-    def __getattr__(self, name) -> Any:
-        return getattr(self.final_structure, name)
+    def __getattr__(self, name: str) -> Any:
+        # Don't use getattr(self.final_structure, name) here to avoid infinite recursion if name = "final_structure"
+        struct = self.__getattribute__("final_structure")
+        return getattr(struct, name)
 
     def __len__(self) -> int:
         return len(self.history)
@@ -161,7 +166,9 @@ def append_filter(self, structure_filter: AbstractStructureFilter) -> None:
         self.history.append(h_dict)
 
     def extend_transformations(
-        self, transformations: list[AbstractTransformation], return_alternatives: bool = False
+        self,
+        transformations: list[AbstractTransformation],
+        return_alternatives: bool = False,
     ) -> None:
         """Extends a sequence of transformations to the TransformedStructure.
 
@@ -209,7 +216,13 @@ def write_vasp_input(
             json.dump(self.as_dict(), file)
 
     def __str__(self) -> str:
-        output = ["Current structure", "------------", str(self.final_structure), "\nHistory", "------------"]
+        output = [
+            "Current structure",
+            "------------",
+            str(self.final_structure),
+            "\nHistory",
+            "------------",
+        ]
         for hist in self.history:
             hist.pop("input_structure", None)
             output.append(str(hist))
@@ -290,7 +303,9 @@ def from_cif_str(
 
     @classmethod
     def from_poscar_str(
-        cls, poscar_string: str, transformations: list[AbstractTransformation] | None = None
+        cls,
+        poscar_string: str,
+        transformations: list[AbstractTransformation] | None = None,
     ) -> TransformedStructure:
         """Generates TransformedStructure from a poscar string.
 

pymatgen/alchemy/materials.py

@@ -256,14 +256,14 @@ def _get_symmetry(self):
         # [1e-4, 2e-4, 1e-4]
         # (these are in fractional coordinates, so should be small denominator
         # fractions)
-        trans = []
+        translations = []
         for t in dct["translations"]:
-            trans.append([float(Fraction.from_float(c).limit_denominator(1000)) for c in t])
-        trans = np.array(trans)
+            translations.append([float(Fraction.from_float(c).limit_denominator(1000)) for c in t])
+        translations = np.array(translations)
 
         # fractional translations of 1 are more simply 0
-        trans[np.abs(trans) == 1] = 0
-        return dct["rotations"], trans
+        translations[np.abs(translations) == 1] = 0
+        return dct["rotations"], translations
 
     def get_symmetry_operations(self, cartesian=False):
         """Return symmetry operations as a list of SymmOp objects. By default returns

pymatgen/symmetry/analyzer.py

@@ -1,6 +1,7 @@
 from __future__ import annotations
 
 import json
+from copy import deepcopy
 
 import pytest
 
@@ -22,8 +23,8 @@ class TestTransformedStructure(PymatgenTest):
     def setUp(self):
         structure = PymatgenTest.get_structure("LiFePO4")
         self.structure = structure
-        trans = [SubstitutionTransformation({"Li": "Na"})]
-        self.trans = TransformedStructure(structure, trans)
+        trafos = [SubstitutionTransformation({"Li": "Na"})]
+        self.trans = TransformedStructure(structure, trafos)
 
     def test_append_transformation(self):
         trafo = SubstitutionTransformation({"Fe": "Mn"})
@@ -56,6 +57,8 @@ def test_get_vasp_input(self):
 
     def test_final_structure(self):
         assert self.trans.final_structure.reduced_formula == "NaFePO4"
+        # https://github.com/materialsproject/pymatgen/pull/3617
+        assert isinstance(deepcopy(self.trans), TransformedStructure)
 
     def test_from_dict(self):
         with open(f"{TEST_FILES_DIR}/transformations.json") as file:
@@ -68,11 +71,11 @@ def test_from_dict(self):
         assert ts.other_parameters == {"author": "Will", "tags": ["test"]}
 
     def test_undo_and_redo_last_change(self):
-        trans = [
+        trafos = [
             SubstitutionTransformation({"Li": "Na"}),
             SubstitutionTransformation({"Fe": "Mn"}),
         ]
-        ts = TransformedStructure(self.structure, trans)
+        ts = TransformedStructure(self.structure, trafos)
         assert ts.final_structure.reduced_formula == "NaMnPO4"
         ts.undo_last_change()
         assert ts.final_structure.reduced_formula == "NaFePO4"

tests/alchemy/test_materials.py

@@ -13,9 +13,8 @@
 
 class TestCifTransmuter(PymatgenTest):
     def test_init(self):
-        trans = []
-        trans.append(SubstitutionTransformation({"Fe": "Mn", "Fe2+": "Mn2+"}))
-        tsc = CifTransmuter.from_filenames([f"{TEST_FILES_DIR}/MultiStructure.cif"], trans)
+        trafos = [SubstitutionTransformation({"Fe": "Mn", "Fe2+": "Mn2+"})]
+        tsc = CifTransmuter.from_filenames([f"{TEST_FILES_DIR}/MultiStructure.cif"], trafos)
         assert len(tsc) == 2
         expected = {"Mn", "O", "Li", "P"}
         for s in tsc:
@@ -25,13 +24,12 @@ def test_init(self):
 
 class TestPoscarTransmuter(PymatgenTest):
     def test_init(self):
-        trans = []
-        trans.append(SubstitutionTransformation({"Fe": "Mn"}))
-        tsc = PoscarTransmuter.from_filenames([f"{TEST_FILES_DIR}/POSCAR", f"{TEST_FILES_DIR}/POSCAR"], trans)
+        trafos = [SubstitutionTransformation({"Fe": "Mn"})]
+        tsc = PoscarTransmuter.from_filenames([f"{TEST_FILES_DIR}/POSCAR", f"{TEST_FILES_DIR}/POSCAR"], trafos)
         assert len(tsc) == 2
         expected = {"Mn", "O", "P"}
-        for s in tsc:
-            els = {el.symbol for el in s.final_structure.elements}
+        for substitution in tsc:
+            els = {el.symbol for el in substitution.final_structure.elements}
             assert expected == els
 
     def test_transmuter(self):