CederGroupHub / smol

Statistical Mechanics on Lattices
https://cedergrouphub.github.io/smol/
Other
64 stars 14 forks source link

Make latest version of h5py as a dependency #445

Closed kamronald closed 10 months ago

kamronald commented 10 months ago

Summary

Major changes: Make the latest version of h5py as a required dependency. To prevent issue such as #441 from happening.

Todos

Checklist

Tip: Install pre-commit hooks to auto-check types and linting before every commit:

pip install -U pre-commit
pre-commit install
kamronald commented 10 months ago

I have no other comment but I'll suggest thinking twice before having to upgrade dependencies in pyproject.toml. It is typically not very canon to do this. Could there be other way that you can fix issue #441 ?

I actually added a new required dependency. Why would that be an issue?

lbluque commented 10 months ago

@qchempku2017 has a point, dependencies in the pyproject.toml should ideally be the oldest version we can support.

I had forgotten that h5py has never been a dependency - only an optional dependency to use the hdf5 files.

I think the case now is if we want to make it a dependency, which I am on board with. However, that needs the optional imports and @requires decorators to be removed from container.py.

kamronald commented 10 months ago

Thanks for the suggestion @lbluque, I just edited container.py to make those changes

lbluque commented 10 months ago

I think the best thing now is to find the oldest (or an older than most recent) version of h5py that runs without issue, and set that version in pyproject.toml