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

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

wolearyc
Copy link
Contributor

Summary

This PR resolves #4289. There were bugs in pymatgen.io.ase.AseAtomsAdaptor in which Structure and ase.Atoms objects were modified when converted into one another. Specifically, Atoms.info and Structure.properties shared memory, leading to side effects.

Minor changes:

  • AseAtomsAdaptor now uses deepcopy when converting Atoms.info to Structure.properties and vice versa 3d47747
  • Added tests to check for side effects and check space group dictionary <-> ase.spacegroup.Spacegroup interconversion 1730f29

Checklist

  • Google format doc strings added. Check with ruff.
  • Type annotations included. Check with mypy.
  • Tests added for new features/fixes.
  • If applicable, new classes/functions/modules have duecredit @due.dcite decorators to reference relevant papers by DOI (example) (N/A)

this fixes bug where atoms/structure objects were themselves modified when converted
@wolearyc wolearyc changed the title [WIP] Bugfix: Eliminate Structure/ase.Atoms interconversion side effects [WIP] Bugfix: Structure/ase.Atoms interconversion side effects Feb 19, 2025
wolearyc and others added 2 commits February 20, 2025 07:55
@wolearyc wolearyc changed the title [WIP] Bugfix: Structure/ase.Atoms interconversion side effects Bugfix: Structure/ase.Atoms interconversion side effects Feb 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

structure.to_ase_atoms() modifies structure.properties in place
2 participants