Fix hatch matrix setup for minimal and optional dependencies #2872
Conversation
github-actions
bot
added
the
needs release notes
|
sounds good to me |
|
Tests are failing because the environment name changed, I'll fix this in a bit |
maxrjones
changed the title
| [[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"] | ||
| [tool.hatch.envs.test.overrides] | ||
| matrix.deps.dependencies = [ | ||
| {value = "zarr[remote, remote_tests, test, optional]", if = ["optional"]} | ||
| ] |
There was a problem hiding this comment.
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.matrix]] | ||
| python = ["3.11", "3.12", "3.13"] | ||
| numpy = ["1.25", "2.1"] | ||
| features = ["gpu"] |
There was a problem hiding this comment.
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.
| @@ -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' }} | |||
There was a problem hiding this comment.
The minimal environment will have less coverage than optional, so there's no value in uploading it to codecov
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 🚢
| @@ -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' }} | |||
There was a problem hiding this comment.
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.
…velopers#2872) * Run minimal tests without fsspec, requests, aiohttp * Retain existing test env names * Use importorskip * Specify which matrix config to upload codecov on * Remove redundant gpu env * Add obstore to min_deps definition * Fix optional dependency set * Add remote_tests set to doctest
I noticed after Martin's comment in #2774 (comment) that even the 'minimal' tests include optional dependencies including fsspec and universal_pathlib. I think it's worth only including the required dependencies and those necessary for running tests / type checking in the "minimal" test environment.