From 0aecad4f2b3d388046bd9e9a254833b201d7f1d1 Mon Sep 17 00:00:00 2001 From: Michael Niklas Date: Wed, 21 Oct 2026 08:33:27 +0200 Subject: [PATCH 1/6] Import AGENTS.md from CLAUDE.md and use pixi in the agent instructions Claude Code before v2.1.277 doesn't read AGENTS.md, and newer versions skip it when a CLAUDE.md exists in a parent directory. A CLAUDE.md that imports AGENTS.md works in both cases. The test guidelines move to xarray/tests/AGENTS.md, so that other agents read them too. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 18 +++--- CLAUDE.md | 1 + xarray/tests/AGENTS.md | 136 ++++++++++++++++++++++++++++++++++++++++ xarray/tests/CLAUDE.md | 137 +---------------------------------------- 4 files changed, 149 insertions(+), 143 deletions(-) create mode 100644 CLAUDE.md create mode 100644 xarray/tests/AGENTS.md diff --git a/AGENTS.md b/AGENTS.md index 2b5ff3531c3..1b6a1bae649 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -2,22 +2,26 @@ ## Setup -```bash -uv sync -``` +Development environments are managed with [pixi](https://pixi.sh), the same ones +CI uses (see `[environments]` in `pixi.toml`). They are installed on first use. ## Run tests ```bash -uv run pytest xarray -n auto # All tests in parallel -uv run pytest xarray/tests/test_dataarray.py # Specific file +pixi run -e test-py313 -- python -m pytest xarray -n auto # All tests in parallel +pixi run -e test-py313 -- python -m pytest xarray/tests/test_dataarray.py # Specific file ``` +Environments like `test-py314-no-dask` or `test-py312-bare-minimum` run the tests +without some optional dependencies. + +See `xarray/tests/AGENTS.md` for guidelines on writing tests. + ## Linting & type checking ```bash -pre-commit run --all-files # Includes ruff and other checks -uv run dmypy run # Type checking with mypy +pixi exec pre-commit run --all-files # Includes ruff and other checks +pixi run -e test-py314-with-typing -- python -m mypy # Type checking with mypy ``` ## Code Style Guidelines diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 00000000000..43c994c2d36 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1 @@ +@AGENTS.md diff --git a/xarray/tests/AGENTS.md b/xarray/tests/AGENTS.md new file mode 100644 index 00000000000..c70d9a79975 --- /dev/null +++ b/xarray/tests/AGENTS.md @@ -0,0 +1,136 @@ +# Testing Guidelines for xarray + +## Handling Optional Dependencies + +xarray has many optional dependencies that may not be available in all testing environments. Always use the standard decorators and patterns when writing tests that require specific dependencies. + +### Standard Decorators + +**ALWAYS use decorators** like `@requires_dask`, `@requires_cftime`, etc. instead of conditional `if` statements. + +All available decorators are defined in `xarray/tests/__init__.py` (look for `requires_*` decorators). + +Use the dask helpers from `xarray.tests` instead of importing `dask.array` +directly. In dask-array expression test mode those helpers point at the +registered chunk manager, so tests can run against either implementation. + +### DO NOT use conditional imports or skipif + +❌ **WRONG - Do not do this:** + +```python +def test_mean_with_cftime(): + if has_dask: # WRONG! + ds = ds.chunk({}) + result = ds.mean() +``` + +❌ **ALSO WRONG - Avoid pytest.mark.skipif in parametrize:** + +```python +@pytest.mark.parametrize( + "chunk", + [ + pytest.param( + True, marks=pytest.mark.skipif(not has_dask, reason="requires dask") + ), + False, + ], +) +def test_something(chunk): ... +``` + +✅ **CORRECT - Do this instead:** + +```python +def test_mean_with_cftime(): + # Test without dask + result = ds.mean() + + +@requires_dask +def test_mean_with_cftime_dask(): + # Separate test for dask functionality + ds = ds.chunk({}) + result = ds.mean() +``` + +✅ **OR for parametrized tests, split them:** + +```python +def test_something_without_dask(): + # Test the False case + ... + + +@requires_dask +def test_something_with_dask(): + # Test the True case with dask + ... +``` + +### Multiple dependencies + +When a test requires multiple optional dependencies: + +```python +@requires_dask +@requires_scipy +def test_interpolation_with_dask(): ... +``` + +### Importing optional dependencies in tests + +For imports within test functions, use `pytest.importorskip`: + +```python +def test_cftime_functionality(): + cftime = pytest.importorskip("cftime") + # Now use cftime +``` + +### Common patterns + +1. **Split tests by dependency** - Don't mix optional dependency code with base functionality: + + ```python + def test_base_functionality(): + # Core test without optional deps + result = ds.mean() + assert result is not None + + + @requires_dask + def test_dask_functionality(): + # Dask-specific test + ds_chunked = ds.chunk({}) + result = ds_chunked.mean() + assert result is not None + ``` + +2. **Use fixtures for dependency-specific setup**: + + ```python + @pytest.fixture + def dask_array(): + pytest.importorskip("dask.array") + import dask.array as da + + return da.from_array([1, 2, 3], chunks=2) + ``` + +3. **Check available implementations**: + + ```python + from xarray.core.duck_array_ops import available_implementations + + + @pytest.mark.parametrize("implementation", available_implementations()) + def test_with_available_backends(implementation): ... + ``` + +### Key Points + +- CI environments intentionally exclude certain dependencies (e.g., `all-but-dask`, `bare-minimum`) +- A test failing in "all-but-dask" because it uses dask is a test bug, not a CI issue +- Look at similar existing tests for patterns to follow diff --git a/xarray/tests/CLAUDE.md b/xarray/tests/CLAUDE.md index c70d9a79975..43c994c2d36 100644 --- a/xarray/tests/CLAUDE.md +++ b/xarray/tests/CLAUDE.md @@ -1,136 +1 @@ -# Testing Guidelines for xarray - -## Handling Optional Dependencies - -xarray has many optional dependencies that may not be available in all testing environments. Always use the standard decorators and patterns when writing tests that require specific dependencies. - -### Standard Decorators - -**ALWAYS use decorators** like `@requires_dask`, `@requires_cftime`, etc. instead of conditional `if` statements. - -All available decorators are defined in `xarray/tests/__init__.py` (look for `requires_*` decorators). - -Use the dask helpers from `xarray.tests` instead of importing `dask.array` -directly. In dask-array expression test mode those helpers point at the -registered chunk manager, so tests can run against either implementation. - -### DO NOT use conditional imports or skipif - -❌ **WRONG - Do not do this:** - -```python -def test_mean_with_cftime(): - if has_dask: # WRONG! - ds = ds.chunk({}) - result = ds.mean() -``` - -❌ **ALSO WRONG - Avoid pytest.mark.skipif in parametrize:** - -```python -@pytest.mark.parametrize( - "chunk", - [ - pytest.param( - True, marks=pytest.mark.skipif(not has_dask, reason="requires dask") - ), - False, - ], -) -def test_something(chunk): ... -``` - -✅ **CORRECT - Do this instead:** - -```python -def test_mean_with_cftime(): - # Test without dask - result = ds.mean() - - -@requires_dask -def test_mean_with_cftime_dask(): - # Separate test for dask functionality - ds = ds.chunk({}) - result = ds.mean() -``` - -✅ **OR for parametrized tests, split them:** - -```python -def test_something_without_dask(): - # Test the False case - ... - - -@requires_dask -def test_something_with_dask(): - # Test the True case with dask - ... -``` - -### Multiple dependencies - -When a test requires multiple optional dependencies: - -```python -@requires_dask -@requires_scipy -def test_interpolation_with_dask(): ... -``` - -### Importing optional dependencies in tests - -For imports within test functions, use `pytest.importorskip`: - -```python -def test_cftime_functionality(): - cftime = pytest.importorskip("cftime") - # Now use cftime -``` - -### Common patterns - -1. **Split tests by dependency** - Don't mix optional dependency code with base functionality: - - ```python - def test_base_functionality(): - # Core test without optional deps - result = ds.mean() - assert result is not None - - - @requires_dask - def test_dask_functionality(): - # Dask-specific test - ds_chunked = ds.chunk({}) - result = ds_chunked.mean() - assert result is not None - ``` - -2. **Use fixtures for dependency-specific setup**: - - ```python - @pytest.fixture - def dask_array(): - pytest.importorskip("dask.array") - import dask.array as da - - return da.from_array([1, 2, 3], chunks=2) - ``` - -3. **Check available implementations**: - - ```python - from xarray.core.duck_array_ops import available_implementations - - - @pytest.mark.parametrize("implementation", available_implementations()) - def test_with_available_backends(implementation): ... - ``` - -### Key Points - -- CI environments intentionally exclude certain dependencies (e.g., `all-but-dask`, `bare-minimum`) -- A test failing in "all-but-dask" because it uses dask is a test bug, not a CI issue -- Look at similar existing tests for patterns to follow +@AGENTS.md From e266651465fbd30cbc3227bf5315989c1785ecad Mon Sep 17 00:00:00 2001 From: Michael Niklas Date: Wed, 21 Oct 2026 08:37:33 +0200 Subject: [PATCH 2/6] Mention the use_dask fixture and where to find fixtures Co-Authored-By: Claude Opus 5.5 --- xarray/tests/AGENTS.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/xarray/tests/AGENTS.md b/xarray/tests/AGENTS.md index c70d9a79975..aa6fb31dd7e 100644 --- a/xarray/tests/AGENTS.md +++ b/xarray/tests/AGENTS.md @@ -69,6 +69,27 @@ def test_something_with_dask(): ... ``` +### Testing with and without dask: the `use_dask` fixture + +To run the same test with numpy and with dask, use the `use_dask` fixture instead +of writing two tests. It runs the test once with `False` (id `numpy`) and once with +`True` (id `dask`), and the dask case is skipped automatically if dask is not +installed: + +```python +def test_mean(use_dask: bool) -> None: + ds = create_test_data() + if use_dask: + ds = ds.chunk() + result = ds.mean() +``` + +### Fixtures + +Shared fixtures are defined in `xarray/tests/conftest.py`, e.g. `use_dask`, +`compute_backend`, `use_cftime` and `time_unit`, and test data like `ds`, `da` and +`simple_datatree`. Check there before writing your own fixture or parametrization. + ### Multiple dependencies When a test requires multiple optional dependencies: From 9f5b36ea3cd83c678b727452fc110fd3b160f282 Mon Sep 17 00:00:00 2001 From: Michael Niklas Date: Wed, 21 Oct 2026 08:38:58 +0200 Subject: [PATCH 3/6] Skip tests at collection time instead of with importorskip Co-Authored-By: Claude Opus 5.5 --- xarray/tests/AGENTS.md | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/xarray/tests/AGENTS.md b/xarray/tests/AGENTS.md index aa6fb31dd7e..1ce8a22c712 100644 --- a/xarray/tests/AGENTS.md +++ b/xarray/tests/AGENTS.md @@ -102,12 +102,16 @@ def test_interpolation_with_dask(): ... ### Importing optional dependencies in tests -For imports within test functions, use `pytest.importorskip`: +Skip tests at collection time with the `requires_*` decorators (or +`pytest.param(..., marks=requires_*)` in a parametrization), not at runtime with +`pytest.importorskip`. Import the optional dependency inside the decorated test: ```python +@requires_cftime def test_cftime_functionality(): - cftime = pytest.importorskip("cftime") - # Now use cftime + import cftime + + ... ``` ### Common patterns @@ -129,18 +133,7 @@ def test_cftime_functionality(): assert result is not None ``` -2. **Use fixtures for dependency-specific setup**: - - ```python - @pytest.fixture - def dask_array(): - pytest.importorskip("dask.array") - import dask.array as da - - return da.from_array([1, 2, 3], chunks=2) - ``` - -3. **Check available implementations**: +2. **Check available implementations**: ```python from xarray.core.duck_array_ops import available_implementations From 1ac5cb742d3c2cee0191f11aaef7125e4b2ae700 Mon Sep 17 00:00:00 2001 From: Michael Niklas Date: Wed, 21 Oct 2026 08:39:45 +0200 Subject: [PATCH 4/6] Describe both ways to test with and without dask Co-Authored-By: Claude Opus 5.5 --- xarray/tests/AGENTS.md | 27 +++++++-------------------- 1 file changed, 7 insertions(+), 20 deletions(-) diff --git a/xarray/tests/AGENTS.md b/xarray/tests/AGENTS.md index 1ce8a22c712..e5cb4421862 100644 --- a/xarray/tests/AGENTS.md +++ b/xarray/tests/AGENTS.md @@ -55,26 +55,9 @@ def test_mean_with_cftime_dask(): result = ds.mean() ``` -✅ **OR for parametrized tests, split them:** - -```python -def test_something_without_dask(): - # Test the False case - ... - - -@requires_dask -def test_something_with_dask(): - # Test the True case with dask - ... -``` - -### Testing with and without dask: the `use_dask` fixture - -To run the same test with numpy and with dask, use the `use_dask` fixture instead -of writing two tests. It runs the test once with `False` (id `numpy`) and once with -`True` (id `dask`), and the dask case is skipped automatically if dask is not -installed: +✅ **OR use the `use_dask` fixture:** it runs the test once with `False` (id +`numpy`) and once with `True` (id `dask`), and skips the dask case automatically if +dask is not installed: ```python def test_mean(use_dask: bool) -> None: @@ -84,6 +67,10 @@ def test_mean(use_dask: bool) -> None: result = ds.mean() ``` +Use `use_dask` if the test is the same for numpy and dask apart from chunking the +data, and separate tests if the dask case checks something specific to dask, e.g. +that the result stays lazy. + ### Fixtures Shared fixtures are defined in `xarray/tests/conftest.py`, e.g. `use_dask`, From d70256d2f807dc6bdde0bc8c8cb50fc17b3e647d Mon Sep 17 00:00:00 2001 From: Michael Niklas Date: Wed, 21 Oct 2026 08:41:31 +0200 Subject: [PATCH 5/6] Ask agents to always add type hints Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 5 +++++ xarray/tests/AGENTS.md | 18 +++++++++--------- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 1b6a1bae649..eee8971d732 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -36,6 +36,11 @@ pixi run -e test-py314-with-typing -- python -m mypy # Type checking with mypy 2. Related third-party imports 3. Local application/library specific imports +### Type hints + +- Always add type hints to every function and method you create or edit, + including tests (`-> None`), fixtures and helpers + ## CI - The `test-py314 (flaky)` job runs tests that depend on external services diff --git a/xarray/tests/AGENTS.md b/xarray/tests/AGENTS.md index e5cb4421862..d5acc0f705a 100644 --- a/xarray/tests/AGENTS.md +++ b/xarray/tests/AGENTS.md @@ -19,7 +19,7 @@ registered chunk manager, so tests can run against either implementation. ❌ **WRONG - Do not do this:** ```python -def test_mean_with_cftime(): +def test_mean_with_cftime() -> None: if has_dask: # WRONG! ds = ds.chunk({}) result = ds.mean() @@ -37,19 +37,19 @@ def test_mean_with_cftime(): False, ], ) -def test_something(chunk): ... +def test_something(chunk: bool) -> None: ... ``` ✅ **CORRECT - Do this instead:** ```python -def test_mean_with_cftime(): +def test_mean_with_cftime() -> None: # Test without dask result = ds.mean() @requires_dask -def test_mean_with_cftime_dask(): +def test_mean_with_cftime_dask() -> None: # Separate test for dask functionality ds = ds.chunk({}) result = ds.mean() @@ -84,7 +84,7 @@ When a test requires multiple optional dependencies: ```python @requires_dask @requires_scipy -def test_interpolation_with_dask(): ... +def test_interpolation_with_dask() -> None: ... ``` ### Importing optional dependencies in tests @@ -95,7 +95,7 @@ Skip tests at collection time with the `requires_*` decorators (or ```python @requires_cftime -def test_cftime_functionality(): +def test_cftime_functionality() -> None: import cftime ... @@ -106,14 +106,14 @@ def test_cftime_functionality(): 1. **Split tests by dependency** - Don't mix optional dependency code with base functionality: ```python - def test_base_functionality(): + def test_base_functionality() -> None: # Core test without optional deps result = ds.mean() assert result is not None @requires_dask - def test_dask_functionality(): + def test_dask_functionality() -> None: # Dask-specific test ds_chunked = ds.chunk({}) result = ds_chunked.mean() @@ -127,7 +127,7 @@ def test_cftime_functionality(): @pytest.mark.parametrize("implementation", available_implementations()) - def test_with_available_backends(implementation): ... + def test_with_available_backends(implementation: str) -> None: ... ``` ### Key Points From b659e662a6c911b98ce4f78f4e93068358c7d4bd Mon Sep 17 00:00:00 2001 From: Michael Niklas Date: Wed, 21 Oct 2026 09:03:30 +0200 Subject: [PATCH 6/6] Prefer parametrizing over separate tests for optional dependencies Co-Authored-By: Claude Opus 5.5 --- xarray/tests/AGENTS.md | 79 ++++++++++++------------------------------ 1 file changed, 23 insertions(+), 56 deletions(-) diff --git a/xarray/tests/AGENTS.md b/xarray/tests/AGENTS.md index d5acc0f705a..41c53440dce 100644 --- a/xarray/tests/AGENTS.md +++ b/xarray/tests/AGENTS.md @@ -14,15 +14,15 @@ Use the dask helpers from `xarray.tests` instead of importing `dask.array` directly. In dask-array expression test mode those helpers point at the registered chunk manager, so tests can run against either implementation. -### DO NOT use conditional imports or skipif +### Skip at collection time, not at runtime -❌ **WRONG - Do not do this:** +❌ **WRONG - Do not check for a dependency inside the test:** ```python -def test_mean_with_cftime() -> None: +def test_mean() -> None: if has_dask: # WRONG! ds = ds.chunk({}) - result = ds.mean() + result = ds.mean() ``` ❌ **ALSO WRONG - Avoid pytest.mark.skipif in parametrize:** @@ -40,24 +40,10 @@ def test_mean_with_cftime() -> None: def test_something(chunk: bool) -> None: ... ``` -✅ **CORRECT - Do this instead:** - -```python -def test_mean_with_cftime() -> None: - # Test without dask - result = ds.mean() - - -@requires_dask -def test_mean_with_cftime_dask() -> None: - # Separate test for dask functionality - ds = ds.chunk({}) - result = ds.mean() -``` - -✅ **OR use the `use_dask` fixture:** it runs the test once with `False` (id -`numpy`) and once with `True` (id `dask`), and skips the dask case automatically if -dask is not installed: +✅ **CORRECT - Parametrize, e.g. with the `use_dask` fixture:** it runs the test +once with `False` (id `numpy`) and once with `True` (id `dask`), and the dask case +is skipped at collection time if dask is not installed. Sharing the test body +avoids duplicate code and checks that numpy and dask give the same result: ```python def test_mean(use_dask: bool) -> None: @@ -67,9 +53,17 @@ def test_mean(use_dask: bool) -> None: result = ds.mean() ``` -Use `use_dask` if the test is the same for numpy and dask apart from chunking the -data, and separate tests if the dask case checks something specific to dask, e.g. -that the result stays lazy. +For other optional dependencies, mark the parameters that need them: + +```python +@pytest.mark.parametrize( + "calendar", ["standard", pytest.param("noleap", marks=requires_cftime)] +) +def test_something(calendar: str) -> None: ... +``` + +Only write a separate test, decorated with e.g. `@requires_dask`, if it checks +something specific to that dependency, e.g. that the result stays lazy. ### Fixtures @@ -101,37 +95,10 @@ def test_cftime_functionality() -> None: ... ``` -### Common patterns - -1. **Split tests by dependency** - Don't mix optional dependency code with base functionality: - - ```python - def test_base_functionality() -> None: - # Core test without optional deps - result = ds.mean() - assert result is not None - - - @requires_dask - def test_dask_functionality() -> None: - # Dask-specific test - ds_chunked = ds.chunk({}) - result = ds_chunked.mean() - assert result is not None - ``` - -2. **Check available implementations**: - - ```python - from xarray.core.duck_array_ops import available_implementations - - - @pytest.mark.parametrize("implementation", available_implementations()) - def test_with_available_backends(implementation: str) -> None: ... - ``` - ### Key Points -- CI environments intentionally exclude certain dependencies (e.g., `all-but-dask`, `bare-minimum`) -- A test failing in "all-but-dask" because it uses dask is a test bug, not a CI issue +- CI environments intentionally exclude certain dependencies (e.g., + `test-py314-no-dask`, `test-py312-bare-minimum`) +- A test failing in `test-py314-no-dask` because it uses dask is a test bug, not a + CI issue - Look at similar existing tests for patterns to follow