diff --git a/AGENTS.md b/AGENTS.md index 2b5ff3531c3..eee8971d732 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 @@ -32,6 +36,11 @@ uv run dmypy run # 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/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..41c53440dce --- /dev/null +++ b/xarray/tests/AGENTS.md @@ -0,0 +1,104 @@ +# 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. + +### Skip at collection time, not at runtime + +❌ **WRONG - Do not check for a dependency inside the test:** + +```python +def test_mean() -> None: + 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: bool) -> None: ... +``` + +✅ **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: + ds = create_test_data() + if use_dask: + ds = ds.chunk() + result = ds.mean() +``` + +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 + +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: + +```python +@requires_dask +@requires_scipy +def test_interpolation_with_dask() -> None: ... +``` + +### Importing optional dependencies in tests + +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() -> None: + import cftime + + ... +``` + +### Key Points + +- 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 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