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

Add ASE plugin support #150

Merged
merged 4 commits into from
Nov 26, 2024
Merged

Add ASE plugin support #150

merged 4 commits into from
Nov 26, 2024

Conversation

lkkmpn
Copy link
Contributor

@lkkmpn lkkmpn commented Nov 25, 2024

This PR adds ASE plugin support, so that ase.io.read and ase.io.write can also operate on ZnH5MD files. Probably most useful in a visualization context, as ase gui atoms.h5 also works.

Note that there are some performance issues in ASE regarding reading a single index from a large file which currently affect for instance ase.io.read('atoms.h5', index=0) but not znh5md.IO('atoms.h5')[0]. See the related issue on ASE's repository for more details.

@PythonFZ
Copy link
Member

Hey Luuk,

Thanks for the addition! Do you think it would be possible to add a test or give me an example on how this would be used so I could add a test?

@lkkmpn
Copy link
Contributor Author

lkkmpn commented Nov 26, 2024

I have added a few tests. Let me know if you'd like any more to be added!

(Usage, by the way, is as simple as installing the package—might need to reinstall existing editable installations for the entry point to get picked up—and then using read() or write() with .h5 files.)

@PythonFZ
Copy link
Member

@lkkmpn That is a great addition! Thanks for including it. LGTM.

@PythonFZ PythonFZ merged commit 0f10cc1 into zincware:main Nov 26, 2024
4 of 5 checks passed
Copy link

Benchmark

Write: Varying number of images

Write: Varying number of atoms

Read: Varying number of images

Read: Varying number of atoms

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.

2 participants