Skip to content

Fix hatch matrix setup for minimal and optional dependencies #2872

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.

Sign up for GitHub

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

Merged
merged 10 commits into from
Apr 16, 2025
Merged

Fix hatch matrix setup for minimal and optional dependencies #2872

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
7976bd8
Run minimal tests without fsspec, requests, aiohttp
maxrjones Feb 27, 2025
11b2c78
Retain existing test env names
maxrjones Feb 28, 2025
b2442ba
Use importorskip
maxrjones Feb 28, 2025
5320edc
Merge branch 'main' into minimal-tests
maxrjones Feb 28, 2025
9dc1845
Specify which matrix config to upload codecov on
maxrjones Feb 28, 2025
39fff00
Merge branch 'main' into minimal-tests
maxrjones Apr 11, 2025
ec880dd
Remove redundant gpu env
maxrjones Apr 12, 2025
f5c3990
Add obstore to min_deps definition
maxrjones Apr 12, 2025
1364fb9
Fix optional dependency set
maxrjones Apr 12, 2025
0a266d8
Add remote_tests set to doctest
maxrjones Apr 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ jobs:
run: |
hatch env run --env test.py${{ matrix.python-version }}-${{ matrix.numpy-version }}-${{ matrix.dependency-set }} run-coverage
- name: Upload coverage
if: ${{ matrix.dependency-set == 'optional' && matrix.os == 'ubuntu-latest' }}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The minimal environment will have less coverage than optional, so there's no value in uploading it to codecov

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a downside to uploading it? I think it might be worth just keeping all the uploads to keep the complexity of this file down.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seemed to me a tradeoff between complexity in the workflow and complexity in understanding the codecov logs - https://app.codecov.io/gh/zarr-developers/zarr-python/commit/ebadec01e74acdd5bc3273be771c20a0f6a67cce. But not many people will interact with those logs so I can remove this line

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fair - I tend not to look at the logs, but agree that it makes the logs a bit easier. Lets leave the line in and I'll merge 🚢

uses: codecov/codecov-action@v5
with:
token: ${{ secrets.CODECOV_TOKEN }}
Expand Down
33 changes: 14 additions & 19 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,18 +73,19 @@ gpu = [
test = [
"coverage",
"pytest",
"pytest-asyncio",
"pytest-cov",
"pytest-accept",
"rich",
"mypy",
"hypothesis",
]
remote_tests = [
'zarr[remote]',
"botocore",
"s3fs",
"moto[s3,server]",
"pytest-asyncio",
"pytest-accept",
"requests",
"rich",
"mypy",
"hypothesis",
"universal-pathlib",
]
optional = ["rich", "universal-pathlib"]
docs = [
Expand Down Expand Up @@ -143,28 +144,21 @@ hooks.vcs.version-file = "src/zarr/_version.py"
[tool.hatch.envs.test]
dependencies = [
"numpy~={matrix:numpy}",
"universal_pathlib",
]
features = ["test"]

[[tool.hatch.envs.test.matrix]]
python = ["3.11", "3.12", "3.13"]
numpy = ["1.25", "2.1"]
version = ["minimal"]

[[tool.hatch.envs.test.matrix]]
python = ["3.11", "3.12", "3.13"]
numpy = ["1.25", "2.1"]
features = ["optional"]
deps = ["minimal", "optional"]

[[tool.hatch.envs.test.matrix]]
python = ["3.11", "3.12", "3.13"]
numpy = ["1.25", "2.1"]
features = ["gpu"]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We separately have the testgpu environment, so there's no need to include another gpu feature set in the regular test matrix. I removed it because it only makes the setup more confusing.

[tool.hatch.envs.test.overrides]
matrix.deps.dependencies = [
{value = "zarr[remote, remote_tests, test, optional]", if = ["optional"]}
]
Comment on lines 150 to +158
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I determined by looking at the old logs that there were no actual differences between the minimal and optional groups of the matrix. This override setup properly includes optional dependencies for the optional group and excludes them from the minimal group.


[tool.hatch.envs.test.scripts]
run-coverage = "pytest --cov-config=pyproject.toml --cov=pkg --cov-report xml --cov=src --junitxml=junit.xml -o junit_family=legacy"
run-coverage-gpu = "pip install cupy-cuda12x && pytest -m gpu --cov-config=pyproject.toml --cov=pkg --cov-report xml --cov=src --junitxml=junit.xml -o junit_family=legacy"
run-coverage-html = "pytest --cov-config=pyproject.toml --cov=pkg --cov-report html --cov=src"
run = "run-coverage --no-cov"
run-pytest = "run"
Expand All @@ -174,7 +168,7 @@ run-hypothesis = "run-coverage --hypothesis-profile ci --run-slow-hypothesis tes
list-env = "pip list"

[tool.hatch.envs.doctest]
features = ["test", "optional", "remote"]
features = ["test", "optional", "remote", "remote_tests"]
description = "Test environment for doctests"

[tool.hatch.envs.doctest.scripts]
Expand Down Expand Up @@ -255,6 +249,7 @@ dependencies = [
'universal_pathlib==0.0.22',
'typing_extensions==4.9.*',
'donfig==0.8.*',
'obstore==0.5.*',
# test deps
'zarr[test]',
]
Expand Down
2 changes: 2 additions & 0 deletions tests/test_store/test_core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,8 @@ async def test_make_store_path_invalid() -> None:

async def test_make_store_path_fsspec(monkeypatch) -> None:
pytest.importorskip("fsspec")
pytest.importorskip("requests")
pytest.importorskip("aiohttp")
store_path = await make_store_path("http://foo.com/bar")
assert isinstance(store_path.store, FsspecStore)

Expand Down