[DRAFT] chore: move h5py from base deps to [benchmarks] extra#17
Draft
speckhard wants to merge 2 commits intoLeMaterial:mainfrom
Draft
[DRAFT] chore: move h5py from base deps to [benchmarks] extra#17speckhard wants to merge 2 commits intoLeMaterial:mainfrom
speckhard wants to merge 2 commits intoLeMaterial:mainfrom
Conversation
h5py is only imported by atompack-py/benchmarks/atom_hdf5_soa.py and
benchmarks/hdf5_tuned_experiment.py — comparison scripts, not the
package itself. Zero imports anywhere under python/atompack/ or
production code paths. Yet h5py was listed as a runtime dependency,
so every install of atompack-db pulled ~50 MB of HDF5 binaries that
were never used at import time.
Move h5py>=3.10 to a new [project.optional-dependencies] benchmarks
extra. Users who actually run the comparison benchmarks install with
'pip install atompack-db[benchmarks]'; everyone else gets a slimmer
wheel.
Tests under tests/benchmarks/ that exercise the comparison scripts
already use pytest.importorskip("h5py"), so they cleanly skip when
the extra isn't installed — no test failures from the move.
uv.lock regenerated to reflect the change. h5py is still locked, just
under the benchmarks extra now.
Independent reviewer caught two follow-ups for the h5py-to-extras PR: - atompack-py/benchmarks/README.md still claimed h5py was "part of the default benchmark environment" and shipped "through the base project dependencies." Updated both lines to point users at 'pip install atompack-db[benchmarks]'. - Makefile py-test-benchmarks target only passed --extra dev. After moving h5py to [benchmarks], that meant 'make py-test-benchmarks' silently skipped every hdf5-gated test (defeating the target's purpose). Added --extra benchmarks alongside --extra dev so the benchmarks suite actually exercises h5py paths again. Verified with a direct pytest run: 'uv run --extra dev --extra benchmarks --locked pytest tests/benchmarks' now reports 52 passed, 4 skipped (vs. the same command with only --extra dev which would have skipped all h5py tests).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
h5py>=3.10was a runtime dep but only used by 2 comparison benchmark scripts underatompack-py/benchmarks/. Move to a new[benchmarks]extra so the base wheel doesn't pull ~50 MB of HDF5 binaries unnecessarily.What changes
pyproject.toml—h5py>=3.10moves from[project] dependenciesto[project.optional-dependencies] benchmarks.uv.lock— regenerated; h5py is still locked, just under the benchmarks extra now.atompack-py/benchmarks/README.md— two stale claims about h5py being part of the base benchmark environment are updated to point users atpip install atompack-db[benchmarks].Makefilepy-test-benchmarkstarget — now passes--extra benchmarksalongside--extra devso the benchmark suite continues to actually exercise h5py paths instead of silently skipping them.Users running comparison benchmarks now install with:
Test plan
pytest.importorskip("h5py")calls:make py-test-benchmarks(now--extra dev --extra benchmarks) actually runs the h5py tests instead of skipping them — verified directly:Backward compat
python atompack-py/benchmarks/atom_hdf5_soa.pywithout re-installing will seeImportError: No module named 'h5py'. They shouldpip install atompack-db[benchmarks].atompackPython package never imported h5py, so they're unaffected.Considered alternatives
[dev]: rejected — most contributors don't run the HDF5 benchmark and shouldn't pay the install cost.devincludebenchmarks(nested extras): no precedent for nested extras in this repo. Just updated the Makefile target to pass both.