Expand dev optional dependencies to include everything required to run the entire test suite#2262
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the project’s “dev” optional dependencies so contributors (and CI) can install a single extra that includes all dependencies needed to run the full test suite, and aligns contributor docs + CI workflows to use it.
Changes:
- Added IO/test-related dependencies (
h5py,netCDF4,zarr,pandas) to thedevoptional dependency group. - Updated contributor quick-start instructions to install editable Heat with
.[dev]. - Updated CI workflows to install
.[dev]instead of IO-specific extras.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
quick_start.md |
Simplifies contributor install command to pip install -e '.[dev]'. |
pyproject.toml |
Expands dev extra to include IO dependencies required by tests. |
.github/workflows/ci_full.yaml |
Switches CI install matrix option from IO extras to .[dev]. |
.github/workflows/ci.yaml |
Switches CI install matrix option from IO extras to .[dev]. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Additional dependencies required to run the entire test suite | ||
| "h5py>=3.11", | ||
| "netCDF4>=1.7", | ||
| "zarr", | ||
| "pandas", |
There was a problem hiding this comment.
dev now repeats IO deps that already exist as standalone extras (hdf5, netcdf, zarr, pandas). Duplicating version specs across multiple extras increases the chance of drift. Consider introducing a dedicated tests extra (pytest/coverage + IO deps) and keeping dev as a superset, so CI and contributors can install the minimal test set without pulling in QA/benchmarking tools.
| - '3.14' # Latest stable | ||
| mpi: [ 'openmpi' ] | ||
| install-options: [ '.', '.[hdf5,netcdf,pandas,zarr]' ] | ||
| install-options: [ '.', '.[dev]' ] |
There was a problem hiding this comment.
Switching CI installs to .[dev] pulls in developer tooling (pre-commit/ruff/mypy/perun) that isn’t needed to run tests and can break on newer Python versions due to missing wheels (especially for Rust/binary packages like ruff). Consider introducing a dedicated optional-dependency group (e.g. tests) containing only runtime test requirements (pytest + IO deps), and have CI install that instead of dev (keeping dev as a superset for contributors).
| install-options: [ '.', '.[dev]' ] | |
| install-options: [ '.' ] |
| - '3.14' # Latest stable | ||
| mpi: [ 'openmpi' ] | ||
| install-options: [ '.', '.[hdf5,netcdf,pandas,zarr]' ] | ||
| install-options: [ '.', '.[dev]' ] |
There was a problem hiding this comment.
PR description says CI updates are still needed / should be handled separately, but this PR already modifies the CI workflow matrix to install .[dev]. Please either update the PR description to reflect that CI was changed here, or move these workflow changes into the intended follow-up PR to keep scope clear.
| - '3.14' | ||
| mpi: [ 'openmpi' ] | ||
| install-options: [ '.', '.[hdf5,netcdf,pandas,zarr]' ] | ||
| install-options: [ '.', '.[dev]' ] |
There was a problem hiding this comment.
Same concern as in ci.yaml: installing .[dev] in the full CI matrix adds QA/benchmarking tools (ruff/mypy/pre-commit/perun) that are not required for tests and can introduce avoidable install time and Python-version compatibility failures. Prefer a dedicated tests extra (pytest + test-only runtime deps) for CI.
| install-options: [ '.', '.[dev]' ] | |
| install-options: [ '.' ] |
|
I updated the optional dependencies at codebase. |
In this PR, all additional dependencies are added to the
devoptional dependencies that are needed to run any tests.For now this is just IO stuff, but in #2199, we will then add scikit-learn and use it as reference in the tests. This is actually the reason why we do this PR. Heat doesn't need scikit-learn because it should be used as a kind of replacement for it. So we need a streamlined set of optional dependencies for the tests. Since developers are supposed to run the tests, @JuanPedroGHM suggested to just put this in the
devoptional dependencies.This PR is only half of it. We still need to adapt the codebase CI. I am not sure on which branch exactly I should edit what exactly. Can you take care of it, @mtar?