Bugfix: Structure/ase.Atoms interconversion side effects (#4297)

* deepcopy atoms.info/structure.properties during ase/pymatgen conversion

this fixes bug where atoms/structure objects were themselves modified when converted

* added tests to check spacegroup conversions

* pre-commit auto-fixes

* selective import for copy.deepcopy

in keeping with style of ase.py

* pre-commit auto-fixes

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Andrew S. Rosen <[email protected]>

Author: 3 people

src/pymatgen/io/ase.py

@@ -6,6 +6,7 @@
 from __future__ import annotations
 
 import warnings
+from copy import deepcopy
 from importlib.metadata import PackageNotFoundError
 from typing import TYPE_CHECKING
 
@@ -204,7 +205,7 @@ def get_atoms(structure: SiteCollection, msonable: bool = True, **kwargs) -> MSO
 
         # Atoms.info <---> Structure.properties
         if properties := structure.properties:
-            atoms.info = properties
+            atoms.info = deepcopy(properties)
 
         # Regenerate Spacegroup object from `.todict()` representation
         if isinstance(atoms.info.get("spacegroup"), dict):
@@ -298,10 +299,10 @@ def get_structure(atoms: Atoms, cls: type[Structure] = Structure, **cls_kwargs)
             sel_dyn = None
 
         # Atoms.info <---> Structure.properties
-        # But first make sure `spacegroup` is JSON serializable
-        if atoms.info.get("spacegroup") and isinstance(atoms.info["spacegroup"], Spacegroup):
-            atoms.info["spacegroup"] = atoms.info["spacegroup"].todict()
-        properties = getattr(atoms, "info", {})
+        properties = deepcopy(getattr(atoms, "info", {}))
+        # If present, convert Spacegroup object to JSON-serializable dict
+        if properties.get("spacegroup") and isinstance(properties["spacegroup"], Spacegroup):
+            properties["spacegroup"] = properties["spacegroup"].todict()
 
         # Return a Molecule object if that was specifically requested;
         # otherwise return a Structure object as expected

tests/io/test_ase.py

@@ -105,6 +105,21 @@ def test_get_atoms_from_structure_dyn():
         assert len(c.index) == len([mask for mask in sel_dyn if np.array_equal(mask, ~np.array(ase_mask))])
 
 
+def test_get_atoms_from_structure_spacegroup():
+    # Get structure with space group dictionary in properties
+    space_group_info = STRUCTURE.get_space_group_info()
+    STRUCTURE.properties["spacegroup"] = {"number": space_group_info[1], "setting": 1}
+
+    # Convert to atoms and check that structure was not modified
+    atoms = AseAtomsAdaptor.get_atoms(STRUCTURE)
+    assert isinstance(STRUCTURE.properties["spacegroup"], dict)
+    assert isinstance(atoms.info["spacegroup"], ase.spacegroup.Spacegroup)
+
+    # Check that space group matches
+    assert atoms.info["spacegroup"].no == STRUCTURE.properties["spacegroup"]["number"]
+    assert atoms.info["spacegroup"].setting == STRUCTURE.properties["spacegroup"]["setting"]
+
+
 def test_get_atoms_from_molecule():
     mol = Molecule.from_file(XYZ_STRUCTURE)
     atoms = AseAtomsAdaptor.get_atoms(mol)
@@ -217,6 +232,23 @@ def test_get_structure_dyn(select_dyn):
     assert len(ase_atoms) == len(structure)
 
 
+def test_get_structure_spacegroup():
+    # set up Atoms object with spacegroup information
+    a = 4.05
+    atoms = ase.spacegroup.crystal("Al", [(0, 0, 0)], spacegroup=225, cellpar=[a, a, a, 90, 90, 90])
+    assert "spacegroup" in atoms.info
+    assert isinstance(atoms.info["spacegroup"], ase.spacegroup.Spacegroup)
+
+    # Test that get_structure does not mutate atoms
+    structure = AseAtomsAdaptor.get_structure(atoms)
+    assert isinstance(atoms.info["spacegroup"], ase.spacegroup.Spacegroup)
+    assert isinstance(structure.properties["spacegroup"], dict)
+
+    # Test that spacegroup info matches
+    assert atoms.info["spacegroup"].no == structure.properties["spacegroup"]["number"]
+    assert atoms.info["spacegroup"].setting == structure.properties["spacegroup"]["setting"]
+
+
 def test_get_molecule():
     atoms = ase.io.read(XYZ_STRUCTURE)
     molecule = AseAtomsAdaptor.get_molecule(atoms)