Skip to content

AseAtomsAdaptor: Retain tags property when interconverting Atoms and Structure/Molecule#3151

Merged
janosh merged 7 commits into
materialsproject:masterfrom
Andrew-S-Rosen:rase
Jul 22, 2023
Merged

AseAtomsAdaptor: Retain tags property when interconverting Atoms and Structure/Molecule#3151
janosh merged 7 commits into
materialsproject:masterfrom
Andrew-S-Rosen:rase

Conversation

@Andrew-S-Rosen
Copy link
Copy Markdown
Member

@Andrew-S-Rosen Andrew-S-Rosen commented Jul 12, 2023

Currently, the .tags property of the ASE Atoms object is lost when interconverting between ASE Atoms and Pymatgen Structure/Molecule. I adopted the solution by @brookwander in ulissigroup/catapalt#2 to ensure the tags do not get lost.

@Andrew-S-Rosen Andrew-S-Rosen changed the title Retain tags property when interconverting Atoms and Structure/Molecule in AseAtomsAdaptor AseAtomsAdaptor: Retain tags property when interconverting Atoms and Structure/Molecule Jul 12, 2023
@Andrew-S-Rosen Andrew-S-Rosen marked this pull request as draft July 12, 2023 22:13
@brookwander
Copy link
Copy Markdown

brookwander commented Jul 13, 2023

thanks @arosen93 !

@janosh janosh added enhancement A new feature or improvement to an existing one io Input/output functionality ase Atomic simulation environment labels Jul 13, 2023
@Andrew-S-Rosen Andrew-S-Rosen marked this pull request as ready for review July 21, 2023 23:44
@Andrew-S-Rosen
Copy link
Copy Markdown
Member Author

Andrew-S-Rosen commented Jul 22, 2023

@janosh --- why does ruff hate me? :(

@janosh
Copy link
Copy Markdown
Member

janosh commented Jul 22, 2023

Looks like ruff just had a new release that finds new issues on master. So nothing to do with your changes. Does this still need testing?

@Andrew-S-Rosen
Copy link
Copy Markdown
Member Author

Andrew-S-Rosen commented Jul 22, 2023

Ah I see. No, this should be done!

@janosh janosh merged commit 5d6f566 into materialsproject:master Jul 22, 2023
@Andrew-S-Rosen Andrew-S-Rosen deleted the rase branch July 24, 2023 16:07
@janosh janosh mentioned this pull request Feb 14, 2024
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

ase Atomic simulation environment enhancement A new feature or improvement to an existing one io Input/output functionality

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Andrew-S-Rosen @brookwander @janosh