Repository: materialsproject/pymatgen

Issue:
Default `fmt` of `IStructure.to` doesn't agree with docstring
As per the `IStructure.to` docstring: https://github.com/materialsproject/pymatgen/blob/b5d03d7e313c4933ccad27ac0037cba4a1538351/src/pymatgen/core/structure.py#L2920-L2932


The default `fmt` should be `json` when `filename` is not provided.

However:
```python
from pymatgen.core import Structure, Lattice


structure = Structure(
    lattice=Lattice.cubic(3),
    species=("Fe", "Fe"),
    coords=((0, 0, 0), (0.5, 0.5, 0.5)),
)

structure.to()
```

Would throw an error as the `fmt` is empty string instead of JSON:
```
Traceback (most recent call last):
  File "/Users/yang/developer/pymatgen/debug/test_structure_to.py", line 10, in <module>
    structure.to()
    ~~~~~~~~~~~~^^
  File "/Users/yang/developer/pymatgen/src/pymatgen/core/structure.py", line 3028, in to
    raise ValueError(f"Format not specified and could not infer from {filename=}")
ValueError: Format not specified and could not infer from filename=''
```

https://github.com/materialsproject/pymatgen/blob/b5d03d7e313c4933ccad27ac0037cba4a1538351/src/pymatgen/core/structure.py#L3026-L3029

Hints from issue comments:

Solution patch:
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index fd952fe254..82d93c6eee 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -8,7 +8,7 @@ ci:
 
 repos:
   - repo: https://github.com/astral-sh/ruff-pre-commit
-    rev: v0.9.4
+    rev: v0.9.9
     hooks:
       - id: ruff
         args: [--fix, --unsafe-fixes]
@@ -22,7 +22,7 @@ repos:
       - id: trailing-whitespace
 
   - repo: https://github.com/pre-commit/mirrors-mypy
-    rev: v1.14.1
+    rev: v1.15.0
     hooks:
       - id: mypy
 
@@ -65,6 +65,6 @@ repos:
         args: [--drop-empty-cells, --keep-output]
 
   - repo: https://github.com/RobertCraigie/pyright-python
-    rev: v1.1.393
+    rev: v1.1.396
     hooks:
       - id: pyright
diff --git a/src/pymatgen/core/structure.py b/src/pymatgen/core/structure.py
index 7193952d03..c58e064462 100644
--- a/src/pymatgen/core/structure.py
+++ b/src/pymatgen/core/structure.py
@@ -2918,7 +2918,7 @@ class IStructure(SiteCollection, MSONable):
         return cls.from_sites(sites, charge=charge, properties=dct.get("properties"))
 
     def to(self, filename: PathLike = "", fmt: FileFormats = "", **kwargs) -> str:
-        """Output the structure to a file or string.
+        """Output the structure to a string (and to a file when filename is given).
 
         Args:
             filename (PathLike): If provided, output will be written to a file. If
@@ -2940,6 +2940,10 @@ class IStructure(SiteCollection, MSONable):
         """
         filename, fmt = str(filename), cast(FileFormats, fmt.lower())
 
+        # Default to JSON if filename not specified
+        if filename == "" and fmt == "":
+            fmt = "json"
+
         if fmt == "cif" or fnmatch(filename.lower(), "*.cif*"):
             from pymatgen.io.cif import CifWriter
 
diff --git a/src/pymatgen/io/ase.py b/src/pymatgen/io/ase.py
index 308128c798..bbd0e2bef0 100644
--- a/src/pymatgen/io/ase.py
+++ b/src/pymatgen/io/ase.py
@@ -6,6 +6,7 @@ Atoms object and pymatgen Structure objects.
 from __future__ import annotations
 
 import warnings
+from copy import deepcopy
 from importlib.metadata import PackageNotFoundError
 from typing import TYPE_CHECKING
 
@@ -204,7 +205,7 @@ class AseAtomsAdaptor:
 
         # 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 @@ class AseAtomsAdaptor:
             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


Test patch:
diff --git a/tests/core/test_structure.py b/tests/core/test_structure.py
index e5ececa44e..258202f046 100644
--- a/tests/core/test_structure.py
+++ b/tests/core/test_structure.py
@@ -930,6 +930,9 @@ Direct
         with pytest.raises(ValueError, match="Invalid fmt='badformat'"):
             self.struct.to(fmt="badformat")
 
+        # Default as JSON (no exception expected)
+        assert self.struct.to() == self.struct.to(fmt="json")
+
         self.struct.to(filename=(gz_json_path := "POSCAR.testing.gz"))
         struct = Structure.from_file(gz_json_path)
         assert struct == self.struct
diff --git a/tests/io/test_ase.py b/tests/io/test_ase.py
index 4328d0f695..87e101cef3 100644
--- a/tests/io/test_ase.py
+++ b/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)