Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MSONAtoms may not serialize appropriately if the .info flag is populated #3668

Closed
Andrew-S-Rosen opened this issue Mar 3, 2024 · 1 comment · Fixed by #3670
Closed

MSONAtoms may not serialize appropriately if the .info flag is populated #3668

Andrew-S-Rosen opened this issue Mar 3, 2024 · 1 comment · Fixed by #3670
Labels

Comments

@Andrew-S-Rosen
Copy link
Member

Andrew-S-Rosen commented Mar 3, 2024

Python version

3.9+

Pymatgen version

2024.3.1

Operating system version

No response

Current behavior

The recently introduced MSONAtoms class in #3619 gives Pymatgen additional flexibility in making ASE Atoms objects MSONable, helping with lossless (de)serialization. This method generally works well, but JSON serialization can fail if the .info attribute of the Atoms object contains an MSONable entry.

Expected Behavior

The MSONAtoms should be JSON serializable via the various monty utilities (for instance).

Minimal example

from pymatgen.core import Structure
from monty.serialization import dumpfn

structure = Structure(
    lattice=[[0, 2.13, 2.13], [2.13, 0, 2.13], [2.13, 2.13, 0]],
    species=["Mg", "O"],
    coords=[[0, 0, 0], [0.5, 0.5, 0.5]],
)
mson_atoms = structure.to_ase_atoms()
mson_atoms.info["test"] = structure
dumpfn(mson_atoms, "test.json")

or...

from ase.build import bulk
from pymatgen.io.ase import AseAtomsAdaptor
from monty.serialization import dumpfn

atoms = bulk("Cu")
atoms.info["test"] = AseAtomsAdaptor().get_structure(atoms)
structure = AseAtomsAdaptor.get_structure(atoms)
mson_atoms = structure.to_ase_atoms()
dumpfn(mson_atoms, "test.json")

In both cases, you get

TypeError: Cannot convert object of type <class 'pymatgen.core.structure.Structure'> to dictionary for JSON
@Andrew-S-Rosen
Copy link
Member Author

Andrew-S-Rosen commented Mar 3, 2024

I don't actually know that this one is readily solvable since people can dump whatever they want into .info, and the logic for the (de)serialization is in monty.

I'll close this as a result but wanted to report it anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant