From d4cd5c0b6b3379ef15ad0f12f332a451639f1582 Mon Sep 17 00:00:00 2001 From: Martin Yeo Date: Mon, 26 Oct 2026 08:44:50 +0000 Subject: [PATCH 01/18] Load and save the NOAA GFS sample store Eleven tests over the published subset now in iris-test-data. Four of them pin behaviour no synthetic fixture in this package reaches: the store names its CRS variable in coordinates rather than grid_mapping, so no coordinate system is built; it carries an attribute called dtype, one of the names that used to be shadowed on load; mode='w' replaces an existing store, which was the one of the three modes nothing covered; and a round trip drops the source shard layout, which nothing promises to keep and nothing warns about. Corrects the claim in fixtures.py that the real store replaces the synthetic one. It does not: the synthetic store needs no external data, and it is the more CF-conforming of the two. Co-Authored-By: Claude Opus 5 Session-Id: 511ac21a-e935-4d77-baa6-1b8cb42dac17 --- lib/iris/tests/integration/zarr/fixtures.py | 8 +- .../tests/integration/zarr/test_gfs_sample.py | 200 ++++++++++++++++++ 2 files changed, 207 insertions(+), 1 deletion(-) create mode 100644 lib/iris/tests/integration/zarr/test_gfs_sample.py diff --git a/lib/iris/tests/integration/zarr/fixtures.py b/lib/iris/tests/integration/zarr/fixtures.py index ee300200b0..7445856da9 100644 --- a/lib/iris/tests/integration/zarr/fixtures.py +++ b/lib/iris/tests/integration/zarr/fixtures.py @@ -8,7 +8,13 @@ I/O design names as the ones worth exercising: version 3 `dimension_names`, a sharded codec, a scalar grid-mapping variable, a two-dimensional auxiliary coordinate, and the base64 `_FillValue` that released xarray writes for every -floating dtype. A cut-down copy of the real store replaces this. +floating dtype. + +A cut-down copy of that real store sits beside this one, and `test_gfs_sample.py` +is where it is loaded. The two are not redundant. This one needs no external +data, so the tests here run wherever `iris-test-data` does not; and it is the +more CF-conforming of the pair, because it gives its data variable a +`grid_mapping` attribute, which the published store does not have. Bounds on `latitude` are not part of that real store, but earn their place anyway: `CFBoundaryVariable.spans` is one of the overrides this pull request diff --git a/lib/iris/tests/integration/zarr/test_gfs_sample.py b/lib/iris/tests/integration/zarr/test_gfs_sample.py new file mode 100644 index 0000000000..e6dfe1ec64 --- /dev/null +++ b/lib/iris/tests/integration/zarr/test_gfs_sample.py @@ -0,0 +1,200 @@ +# Copyright Iris contributors +# +# This file is part of Iris and is released under the BSD license. +# See LICENSE in the root of the repository for full licensing details. +"""A published Zarr store, loaded and saved. + +A subset of the NOAA GFS forecast archive published by dynamical.org, held in +``iris-test-data`` under ``Zarr/``. Every synthetic fixture in this package +was written by someone who already knew the rules; this store was not, and +the things it does differently from the synthetic one are the point of it. + +The design document is ``docs/superpowers/specs/2026-09-21-zarr-io-design.md``; +section 7 says what the subset keeps and why. +""" + +import warnings + +import numpy as np +import pytest + +zarr = pytest.importorskip("zarr") +xr = pytest.importorskip("xarray") + +import iris +from iris.loading import LOAD_PROBLEMS +from iris.tests import _shared_utils +from iris.warnings import IrisLoadWarning + +#: The shards and inner chunks the published store is written with. +SOURCE_SHARDS = (1, 4, 120, 120) +SOURCE_CHUNKS = (1, 2, 40, 40) + + +@pytest.fixture +def sample(): + return _shared_utils.get_data_path(("Zarr", "gfs_forecast_sample.zarr")) + + +@pytest.fixture(autouse=True) +def _assert_clean_load(): + """Every load in this module must be free of recorded problems. + + The published store loads silently, so anything recorded here is a + change in Iris rather than a quirk of the data. Same shape as the + fixture of this name in ``test_load.py``. + """ + LOAD_PROBLEMS.reset() + with warnings.catch_warnings(): + warnings.simplefilter("error", IrisLoadWarning) + yield + assert LOAD_PROBLEMS.problems == [] + + +@_shared_utils.skip_data +def test_the_sample_yields_four_cubes(sample): + cubes = iris.load(sample) + assert sorted(cube.name() for cube in cubes) == [ + "air_temperature", + "eastward_wind", + "northward_wind", + "precipitation_flux", + ] + + +@_shared_utils.skip_data +def test_the_sharded_variable_loads_lazily(sample): + cube = iris.load_cube(sample, "air_temperature") + assert cube.has_lazy_data() + assert cube.shape == (2, 8, 120, 120) + + +@_shared_utils.skip_data +def test_the_two_dimensional_time_coordinate_spans_both_its_dimensions(sample): + """``valid_time`` is a function of initialisation and lead time together. + + It carries ``standard_name: time``, so it shares its Iris name with the + ``init_time`` dimension coordinate and ``var_name`` is what tells them + apart. + """ + cube = iris.load_cube(sample, "air_temperature") + assert cube.coord_dims(cube.coord(var_name="valid_time")) == (0, 1) + + +@_shared_utils.skip_data +def test_the_base64_fill_value_is_read_and_masks_nothing(sample): + """A NaN sentinel in xarray's base64 encoding, over data with no NaNs. + + Read wrongly the attribute raises rather than quietly doing nothing, + which is why this asserts the unmasked result and not just that the + load succeeded. + """ + cube = iris.load_cube(sample, "air_temperature") + assert not np.ma.is_masked(cube.data) + + +@_shared_utils.skip_data +def test_the_crs_variable_arrives_as_a_coordinate_not_a_coordinate_system(sample): + """The store names ``spatial_ref`` in ``coordinates``, not ``grid_mapping``. + + So there is nothing for Iris to build a coordinate system from, and the + CRS arrives as a scalar coordinate carrying ``crs_wkt``. CF section 5.6 + asks for a ``grid_mapping`` attribute; this publisher does not write one. + Whether Iris should fall back to reading ``crs_wkt`` off a coordinate is + :issue:`7352`. + """ + cube = iris.load_cube(sample, "air_temperature") + assert cube.coord("latitude").coord_system is None + assert "crs_wkt" in cube.coord("spatial_ref").attributes + + +@_shared_utils.skip_data +def test_an_attribute_named_dtype_does_not_shadow_the_dtype(sample): + """The publisher records the source NumPy dtype in an attribute of that name. + + ``dtype`` is one of the names that used to be shadowed on load, because + it collides with a property of the backend variable object. Here the + attribute says ``timedelta64[us]`` while the coordinate's own dtype is + float64, and both survive. + """ + cube = iris.load_cube(sample, "air_temperature") + coord = cube.coord("forecast_period") + assert coord.attributes["dtype"] == "timedelta64[us]" + assert coord.dtype == np.dtype("f8") + + +@_shared_utils.skip_data +def test_a_structured_attribute_survives(sample): + """CF cannot express a nested attribute; Zarr allows one. Carry it.""" + cube = iris.load_cube(sample, "air_temperature") + statistics = cube.coord(var_name="valid_time").attributes["statistics_approximate"] + assert set(statistics) == {"min", "max"} + + +@_shared_utils.skip_data +def test_the_sample_round_trips(tmp_path, sample): + """Saving adds a ``Conventions`` global, and changes nothing else.""" + cube = iris.load_cube(sample, "air_temperature") + with iris.FUTURE.context(save_split_attrs=True): + iris.save(cube, tmp_path / "rt.zarr") + + back = iris.load_cube(str(tmp_path / "rt.zarr")) + assert back.attributes.globals["Conventions"] == "CF-1.7" + del back.attributes.globals["Conventions"] + assert back == cube + + +@_shared_utils.skip_data +def test_an_existing_store_is_replaced_under_mode_w(tmp_path, sample): + """``mode="w"`` is the third of the three modes, and the least pinned. + + ``test_save.py`` covers what ``mode="w-"`` refuses and what ``mode="a"`` + adds. This covers the one that replaces, which is what a script rerun + over the same path depends on. + """ + path = tmp_path / "replaced.zarr" + first = iris.load_cube(sample, "air_temperature") + second = iris.load_cube(sample, "eastward_wind") + with iris.FUTURE.context(save_split_attrs=True): + iris.save(first, path) + iris.save(second, path, mode="w") + + names = [cube.name() for cube in iris.load(str(path))] + assert names == ["eastward_wind"] + + +@_shared_utils.skip_data +def test_the_round_trip_does_not_keep_the_source_chunk_layout(tmp_path, sample): + """Iris sizes chunks itself, so a store's own layout does not survive. + + The published arrays are sharded; the saved ones are not. Nothing in the + design promises otherwise, and nothing warns either, so this is the test + that would have to change before the behaviour could. + """ + cube = iris.load_cube(sample, "air_temperature") + with iris.FUTURE.context(save_split_attrs=True): + iris.save(cube, tmp_path / "rt.zarr") + + source = zarr.open_group(sample, mode="r")["temperature_2m"] + assert source.shards == SOURCE_SHARDS + assert source.chunks == SOURCE_CHUNKS + + saved = zarr.open_group(str(tmp_path / "rt.zarr"), mode="r")["temperature_2m"] + assert saved.shards is None + assert saved.chunks != SOURCE_CHUNKS + + +@_shared_utils.skip_data +def test_xarray_opens_the_round_tripped_store(tmp_path, sample): + """Asserted on the open, because a bad attribute fails the whole open.""" + cubes = iris.load(sample) + with iris.FUTURE.context(save_split_attrs=True): + iris.save(cubes, tmp_path / "rt.zarr") + + dataset = xr.open_zarr(str(tmp_path / "rt.zarr")) + assert sorted(dataset.data_vars) == [ + "precipitation_surface", + "temperature_2m", + "wind_u_10m", + "wind_v_10m", + ] From ea013bb2588e25ed306fab0b13314fb7b35998cd Mon Sep 17 00:00:00 2001 From: Martin Yeo Date: Mon, 26 Oct 2026 08:47:07 +0000 Subject: [PATCH 02/18] Load a fixed-length unicode variable from a Zarr store Every Session-Id: 511ac21a-e935-4d77-baa6-1b8cb42dac17 --- .../zarr/loader/test_load_cubes.py | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/lib/iris/tests/unit/fileformats/zarr/loader/test_load_cubes.py b/lib/iris/tests/unit/fileformats/zarr/loader/test_load_cubes.py index 09a56862cf..977b28096b 100644 --- a/lib/iris/tests/unit/fileformats/zarr/loader/test_load_cubes.py +++ b/lib/iris/tests/unit/fileformats/zarr/loader/test_load_cubes.py @@ -136,6 +136,38 @@ def test_a_group_with_no_arrays_names_the_groups_that_have_them(tmp_path): list(load_cubes(str(tmp_path / "e.zarr"))) +@pytest.mark.parametrize("zarr_format", [2, 3]) +def test_a_fixed_length_unicode_variable_loads(tmp_path, zarr_format): + """` Date: Mon, 26 Oct 2026 08:57:02 +0000 Subject: [PATCH 03/18] Benchmark saving a cube to Zarr ZarrSave mirrors NetcdfSave over the same cubesphere cubes, so the two numbers are comparable. Two differences: it passes mode='w', because the Zarr saver defaults to mode='w-' and raises FileExistsError on an existing target while ASV calls a timing function many times between setup() calls; and it has no mesh-only benchmark, because iris.mesh.save_mesh writes netCDF only. Co-Authored-By: Claude Opus 5 Session-Id: 511ac21a-e935-4d77-baa6-1b8cb42dac17 --- benchmarks/benchmarks/save.py | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/benchmarks/benchmarks/save.py b/benchmarks/benchmarks/save.py index 4bac1b1450..ce9cd68a31 100644 --- a/benchmarks/benchmarks/save.py +++ b/benchmarks/benchmarks/save.py @@ -41,3 +41,36 @@ def tracemalloc_netcdf_save(self, n_cubesphere, is_unstructured): # Don't need to copy the cube here since track_ benchmarks don't # do repeats between self.setup() calls. self._save_data(self.cube, do_copy=False) + + +class ZarrSave: + params = [[50, 600], [False, True]] + param_names = ["cubesphere-N", "is_unstructured"] + + def setup(self, n_cubesphere, is_unstructured): + self.cube = make_cube_like_2d_cubesphere( + n_cube=n_cubesphere, with_mesh=is_unstructured + ) + + def _save_data(self, cube, do_copy=True): + if do_copy: + # Copy the cube, to avoid distorting the results by changing it + # Because we known that older Iris code realises lazy coords + cube = cube.copy() + # mode="w" because the Zarr saver defaults to mode="w-" and raises + # FileExistsError on a target that exists - and ASV calls this many + # times between setup() calls. + save(cube, "tmp.zarr", mode="w") + + def time_zarr_save_cube(self, n_cubesphere, is_unstructured): + self._save_data(self.cube) + + def tracemalloc_zarr_save(self, n_cubesphere, is_unstructured): + # Don't need to copy the cube here since track_ benchmarks don't + # do repeats between self.setup() calls. + self._save_data(self.cube, do_copy=False) + + # No mesh-only equivalent of NetcdfSave.time_netcdf_save_mesh: + # iris.mesh.save_mesh is defined in iris.fileformats.netcdf.saver and + # writes netCDF only. A cube that has a mesh does save its mesh here, + # through is_unstructured=True. From 680d6c7b009057ca811d0f40be6169781422c8b6 Mon Sep 17 00:00:00 2001 From: Martin Yeo Date: Mon, 26 Oct 2026 09:04:13 +0000 Subject: [PATCH 04/18] Benchmark loading and realising a Zarr store LoadAndRealise gains a Zarr file format beside its NetCDF one, so the two are measured over the same synthetic UM data at the same shapes. create_um_files needed two changes for it. Its reuse check tested is_file(), which is always False for a store directory, so REUSE_DATA would never have held and the data would have been rebuilt on every run. And compression is spelled compressors= rather than zlib=: the Zarr saver rejects netCDF keywords rather than translating them onto codecs. Co-Authored-By: Claude Opus 5 Session-Id: 511ac21a-e935-4d77-baa6-1b8cb42dac17 --- .../benchmarks/generate_data/um_files.py | 19 ++++++++++++++----- benchmarks/benchmarks/load/__init__.py | 2 +- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/benchmarks/benchmarks/generate_data/um_files.py b/benchmarks/benchmarks/generate_data/um_files.py index 40bf83e79c..4801956d98 100644 --- a/benchmarks/benchmarks/generate_data/um_files.py +++ b/benchmarks/benchmarks/generate_data/um_files.py @@ -2,7 +2,7 @@ # # This file is part of Iris and is released under the BSD license. # See LICENSE in the root of the repository for full licensing details. -"""Generate FF, PP and NetCDF files based on a minimal synthetic FF file. +"""Generate FF, PP, NetCDF and Zarr files based on a minimal synthetic FF file. NOTE: uses the Mule package, so depends on an environment with Mule installed. """ @@ -122,12 +122,13 @@ def add_field(level_: int, time_step_: int) -> None: ff_path = save_paths.get("FF", None) pp_path = save_paths.get("PP", None) nc_path = save_paths.get("NetCDF", None) + zarr_path = save_paths.get("Zarr", None) if ff_path: new_ff.to_file(ff_path) if pp_path: fields_to_pp_file(str(pp_path), new_ff.fields) - if nc_path: + if nc_path or zarr_path: temp_ff_path = None # Need an Iris Cube from the FF content. if ff_path: @@ -139,12 +140,18 @@ def add_field(level_: int, time_step_: int) -> None: new_ff.to_file(temp_ff_path.name) ff_cube = load_cube(temp_ff_path.name) - save_cube(ff_cube, nc_path, zlib=compress) + if nc_path: + save_cube(ff_cube, nc_path, zlib=compress) + if zarr_path: + # Zarr's own spelling of compression. The Zarr saver rejects + # netCDF keywords such as zlib= rather than translating them. + compressors = "auto" if compress else None + save_cube(ff_cube, zarr_path, mode="w", compressors=compressors) if temp_ff_path: temp_ff_path.close() -FILE_EXTENSIONS = {"FF": "", "PP": ".pp", "NetCDF": ".nc"} +FILE_EXTENSIONS = {"FF": "", "PP": ".pp", "NetCDF": ".nc", "Zarr": ".zarr"} def create_um_files( @@ -177,7 +184,9 @@ def create_um_files( for file_type in file_types: file_ext = FILE_EXTENSIONS[file_type] save_path = (save_dir / f"{compress}").with_suffix(file_ext) - files_exist = files_exist and save_path.is_file() + # exists() not is_file(): a Zarr store is a directory, so is_file() + # is always False for one and REUSE_DATA would never hold. + files_exist = files_exist and save_path.exists() save_paths[file_type] = str(save_path) if not REUSE_DATA or not files_exist: diff --git a/benchmarks/benchmarks/load/__init__.py b/benchmarks/benchmarks/load/__init__.py index 5c5a62a515..b68819c729 100644 --- a/benchmarks/benchmarks/load/__init__.py +++ b/benchmarks/benchmarks/load/__init__.py @@ -18,7 +18,7 @@ class LoadAndRealise: params = ( [(50, 50, 2), (1280, 960, 5), (2, 2, 1000)], [False, True], - ["FF", "PP", "NetCDF"], + ["FF", "PP", "NetCDF", "Zarr"], ) param_names = ["xyz", "compressed", "file_format"] From 24525659edbf35d40e6db77e5765e025d2f9ab9c Mon Sep 17 00:00:00 2001 From: Martin Yeo Date: Mon, 26 Oct 2026 09:11:56 +0000 Subject: [PATCH 05/18] Record PR 7 in the design document Six findings, four of them things the published NOAA GFS store does that no synthetic fixture did. The EOPF fixture is dropped on licence grounds and sections 5 and 7 now say so; section 7 keeps the store as cited evidence, because the rules it motivated do not depend on Iris shipping a copy. Also adds the plan this pull request was built from. Co-Authored-By: Claude Opus 5 Session-Id: 511ac21a-e935-4d77-baa6-1b8cb42dac17 --- .../plans/2026-10-26-zarr-io-pr7.md | 1008 +++++++++++++++++ .../specs/2026-09-21-zarr-io-design.md | 89 +- .../tests/integration/zarr/test_gfs_sample.py | 4 +- 3 files changed, 1087 insertions(+), 14 deletions(-) create mode 100644 docs/superpowers/plans/2026-10-26-zarr-io-pr7.md diff --git a/docs/superpowers/plans/2026-10-26-zarr-io-pr7.md b/docs/superpowers/plans/2026-10-26-zarr-io-pr7.md new file mode 100644 index 0000000000..8f1925fd2e --- /dev/null +++ b/docs/superpowers/plans/2026-10-26-zarr-io-pr7.md @@ -0,0 +1,1008 @@ +# Zarr I/O PR 7 — Real-world test data and benchmarks: Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +| | | +|---|---| +| **Spec** | [`docs/superpowers/specs/2026-09-21-zarr-io-design.md`](../specs/2026-09-21-zarr-io-design.md) §5 (PR 7), §6, §7, §12.1, §12.5 | +| **Implements** | Pull request 7 of 9. `Part of #6961` — never a closing keyword. | +| **Branch** | `zarr-io-pr-7`, branched from `zarr-io-pr-6`. Opened as a preview pull request in `trexfeathers/iris` targeting `zarr-io-pr-6`; retargeted at `SciTools/iris:brownfield` once [#7346](https://github.com/SciTools/iris/pull/7346) merges | +| **Baseline** | `f594416da` on `zarr-io-pr-6`; Iris `3.17.0.dev16`; zarr `3.4.1`; xarray `2026.9.0`; numpy `2.5.2`; asv `0.6.x`; Python 3.12 | +| **Delivers** | The published NOAA GFS version 3 store in `SciTools/iris-test-data`, integration tests that load and save it, and ASV coverage of Zarr saving and loading | +| **Behaviour change** | None. Every library file this plan touches is a test module, a benchmark, or a design document. The one exception is `benchmarks/benchmarks/generate_data/um_files.py`, which is benchmark support code and reaches no shipped module. | +| **Public API change** | None. | + +**Goal:** Prove the native Zarr reader and writer against a store NOAA actually publishes, and give ASV a Zarr save and a Zarr load to watch for regressions. + +**Architecture:** One published store — the NOAA GFS forecast archive, version 3 and sharded — is contributed to `SciTools/iris-test-data` from the subset already cut. A new module under `lib/iris/tests/integration/zarr/` loads and saves it and asserts what it shows that no synthetic fixture does. A second, small module fills the one read-side gap that dropping the ESA EOPF fixture leaves. The benchmarks follow the shapes already in the repository: `ZarrSave` mirrors `NetcdfSave` over the same cubesphere cubes, and `LoadAndRealise` grows a `"Zarr"` file format beside its `"NetCDF"` one. + +**Tech Stack:** Python 3.12–3.14, zarr-python ≥3.0.8, xarray (test-only), pytest, ASV, pre-commit (ruff, numpydoc, and the repository's local hooks). The companion repository is `SciTools/iris-test-data`, via the fork `trexfeathers/iris-test-data`. + +--- + +## Global Constraints + +Every task's requirements implicitly include this section. + +| Constraint | Value, verbatim from its source | +|---|---| +| Target branch | `brownfield`, not `main` (spec §5). Until #7346 merges, the preview pull request targets `zarr-io-pr-6` | +| PR labels | `Agentic` and `Type: Feature Branch` (spec §5) | +| Issue reference | `Part of #6961`. **Never** a closing keyword — the issues close at merge-back (spec §5) | +| Changelog | No `changelog/` fragment in this pull request. Fragments are written once at merge-back; the pull request body must say so (spec §5) | +| GitHub posts | Every pull request description, comment and review reply opens with `(posted by Claude Opus)`, before anything else. This **replaces** any trailing harness attribution (root `AGENTS.md`) | +| Commit messages | Out of scope for the above; they keep `Co-Authored-By:` trailers | +| No network in tests | "**Remote stores** are covered by an in-memory store and a local directory store only. No test touches the network" (spec §6). Every new test reads a local path under `TEST_DATA_DIR`, or builds a store in `tmp_path` | +| Test skipping | A test needing `iris-test-data` carries `@_shared_utils.skip_data`. `lib/iris/tests/integration/zarr/__init__.py` already carries `pytest.importorskip("zarr", minversion="3.0.8")` and covers everything under it (spec §6) | +| Test style | pytest style, plain assertions, no unittest classes in new files (`lib/iris/tests/AGENTS.md`) | +| Test prose | A test docstring is notes, under about six lines. Never name a task, plan or review finding in a test name, docstring or comment; cite an issue, a pull request or a design-document section (`lib/iris/tests/AGENTS.md`) | +| Benchmark data generation | "Be sure not to use the benchmarking environment to generate any test objects/files" — anything a benchmark needs built is built through `benchmarks/benchmarks/generate_data` and `run_function_elsewhere` (`benchmarks/README.md`) | +| Benchmark re-runs | ASV calls a timing function many times between `setup()` calls (`benchmarks/README.md`). A benchmark that writes must survive being called again (F7) | +| Third-party data | Data contributed to `iris-test-data` from an outside publisher keeps its upstream attribution and names its upstream licence. The repository's own blanket licence statement is not this pull request's to change (F16) | +| Copyright header | Every new Python file opens with the four-line BSD header (root `AGENTS.md`) | +| Line length | 88 characters (Ruff default) | +| Published prose | `.github/WRITING.md` governs every line written for a person to read, including this plan, the two pull request bodies and the test-data README | +| Before finishing | Run the repository's pre-commit hooks and fix everything they report (root `AGENTS.md`) | + +### Running anything + +The conda environment is `iris-dev`. Either activate it, or prefix commands with +`conda run -n iris-dev`. It now holds `asv`, `nox` and an editable install of +Iris, added for this work with the user's agreement; `zarr 3.4.1` and +`xarray 2026.9.0` were already there. Do not install or remove anything else +without asking. + +Set `NPY_DISABLE_CPU_FEATURES="AVX512F,AVX512CD,AVX512_SKX"` before running the +suite, per the root `AGENTS.md`. + +`iris.config.TEST_DATA_DIR` resolves to +`/data/home/martin.yeo/repos/iris-test-data/test_data`, a checkout of +`trexfeathers/iris-test-data`. Task 1 puts the new store there, which is what +lets Task 3 run green before the companion pull request merges. + +### Running the benchmarks + +Do not use `nox`, and do not let ASV build an environment. `conda.anaconda.org` +answers 403 on this network, the `ssstack` lockfile support lives on the +`bm-agents` branch and not on this one, and `ssstack` is not on `PATH`. Use the +route that bypasses environment building altogether (F11): + +```bash +export OVERRIDE_TEST_DATA_REPOSITORY=/data/home/martin.yeo/repos/iris-test-data/test_data +python benchmarks/bm_runner.py trialrun "" "$(which python)" +``` + +Anything built by `create_um_files` needs Mule in that interpreter, which +`bm_runner` installs only when it builds the environment itself. Task 6's first +step installs it. + +## Review Focus + +Five things that no test in the suite exercises and that a person loading +published Zarr would meet in ordinary work. Each is pinned by a test in the +task named. All five were run against the GFS subset or against a store built +in `tmp_path` on 2026-10-26; none is speculative. + +1. **A CRS variable a store references through `coordinates` rather than + `grid_mapping`.** The NOAA GFS archive does this. Iris builds no coordinate + system from it, so `cube.coord("latitude").coord_system` is `None` and + `spatial_ref` arrives as a scalar auxiliary coordinate carrying `crs_wkt`. + A user who sees a `crs_wkt` in the file and no coordinate system on the cube + will read it as an Iris bug. → **Task 3, Step 1**: + `test_the_crs_variable_arrives_as_a_coordinate_not_a_coordinate_system`, + and **Task 7** raises the issue. +2. **An attribute whose name collides with an array property.** The published + store gives `lead_time`, `expected_forecast_length` and + `ingested_forecast_length` an attribute literally called `dtype`, holding + `"timedelta64[us]"`. That is one of the six names the merge-back changelog + records as having been silently shadowed on load, and the programme's fix + for it has never been run against a store that actually contains one. → + **Task 3, Step 1**: `test_an_attribute_named_dtype_does_not_shadow_the_dtype`. +3. **Replacing a store that already exists.** `test_save.py` pins that the + default `mode="w-"` refuses an existing target, and that `mode="a"` adds to + one. Nothing pins that `mode="w"` — the keyword §4.5 offers as the way back + to replacement — actually replaces, so a script rerun over the same path, + or an ASV timing function called a second time, rests on an untested + promise. → **Task 3, Step 1**: + `test_an_existing_store_is_replaced_under_mode_w`, and **Task 5, Step 1**, + where `ZarrSave` depends on it. +4. **Reading a fixed-length unicode variable.** ` List[Path]`. + +- [ ] **Step 1: Show the defect** + +```bash +cd /data/home/martin.yeo/repos/iris-test-data/indexing +conda run -n iris-dev python -c " +import generate_index as g +from pathlib import Path +print(len(g.getDataFilepaths(Path('../test_data/Zarr')))) +" +``` + +Expected: a count in the tens — one path per `zarr.json` and per chunk object +(F13). + +- [ ] **Step 2: Make a store one path** + +Change `getDataFilepaths` so that a directory whose name ends in `.zarr` is +yielded as a single path, and nothing beneath it is walked. Leave +`IGNORE_SUFFIXES` and every other file alone. + +- [ ] **Step 3: Re-run the count** + +Run Step 1's command again. Expected: `1`. + +- [ ] **Step 4: Check the rest of the pipeline takes a directory** + +```bash +conda run -n iris-dev python -c " +import generate_index as g +from pathlib import Path +infos = g.getFileInfos(g.getDataFilepaths(Path('../test_data/Zarr'))) +for key, items in infos.items(): + for info in items: + print(info.path, '|', len(info.items or []), 'items |', info.exceptions) +" +``` + +Expected: one entry, with a cube summary and no exception. `Image.open` on a +directory raises `IsADirectoryError`, which is an `OSError` and so is caught by +the existing `except IOError`. + +- [ ] **Step 5: Commit** + +```bash +git add indexing/generate_index.py +git commit -m "Index a Zarr store as one item, not as its chunks" +``` + +- [ ] **Step 6: Push and open the companion pull request** + +```bash +git push -u origin zarr-samples +gh pr create --repo SciTools/iris-test-data --base master \ + --head trexfeathers:zarr-samples --title "A NOAA GFS Zarr sample store" +``` + +The body opens with `(posted by Claude Opus)`, says what the store is and where +it came from, says that the indexing change is what keeps a store from becoming +thirty-five INDEX entries, and names the licence. It is not hard wrapped. + +--- + +## Task 3: The NOAA GFS sample, loaded and saved + +Characterisation, not development: the behaviour already exists, and these +tests say what it is. A test that fails here is a defect to record and raise in +Task 7, not to fix in this pull request. + +**Files:** +- Create: `lib/iris/tests/integration/zarr/test_gfs_sample.py` +- Modify: `lib/iris/tests/integration/zarr/fixtures.py` + +**Interfaces:** +- Consumes: `iris.tests._shared_utils.skip_data` and `get_data_path`, and + Task 1's `test_data/Zarr/gfs_forecast_sample.zarr`. +- Produces: nothing other code calls. + +- [ ] **Step 1: Write the module** + +Every test carries `@_shared_utils.skip_data`. A fixture returns +`_shared_utils.get_data_path(("Zarr", "gfs_forecast_sample.zarr"))`. A second +fixture, modelled on `_assert_clean_load` in `test_load.py`, resets +`LOAD_PROBLEMS`, escalates `IrisLoadWarning` to an error for the duration, and +asserts `LOAD_PROBLEMS.problems == []` afterwards — the sample loads silently +(F1), so the strict fixture is correct here. + +```python +def test_the_sample_yields_four_cubes(sample): + cubes = iris.load(sample) + assert sorted(cube.name() for cube in cubes) == [ + "air_temperature", + "eastward_wind", + "northward_wind", + "precipitation_flux", + ] + + +def test_the_sharded_variable_loads_lazily(sample): + cube = iris.load_cube(sample, "air_temperature") + assert cube.has_lazy_data() + assert cube.shape == (2, 8, 120, 120) + + +def test_the_two_dimensional_time_coordinate_spans_both_its_dimensions(sample): + cube = iris.load_cube(sample, "air_temperature") + assert cube.coord_dims(cube.coord(var_name="valid_time")) == (0, 1) + + +def test_the_base64_fill_value_is_read_and_masks_nothing(sample): + cube = iris.load_cube(sample, "air_temperature") + assert not np.ma.is_masked(cube.data) + + +def test_the_crs_variable_arrives_as_a_coordinate_not_a_coordinate_system(sample): + cube = iris.load_cube(sample, "air_temperature") + assert cube.coord("latitude").coord_system is None + assert "crs_wkt" in cube.coord("spatial_ref").attributes + + +def test_an_attribute_named_dtype_does_not_shadow_the_dtype(sample): + cube = iris.load_cube(sample, "air_temperature") + coord = cube.coord("forecast_period") + assert coord.attributes["dtype"] == "timedelta64[us]" + assert coord.dtype == np.dtype("f8") + + +def test_a_structured_attribute_survives(sample): + cube = iris.load_cube(sample, "air_temperature") + assert cube.coord(var_name="valid_time").attributes["statistics_approximate"] + + +def test_the_sample_round_trips(tmp_path, sample): + cube = iris.load_cube(sample, "air_temperature") + with iris.FUTURE.context(save_split_attrs=True): + iris.save(cube, tmp_path / "rt.zarr") + back = iris.load_cube(str(tmp_path / "rt.zarr")) + del back.attributes.globals["Conventions"] + assert back == cube + + +def test_an_existing_store_is_replaced_under_mode_w(tmp_path, sample): + ... + + +def test_the_round_trip_does_not_keep_the_source_chunk_layout(tmp_path, sample): + ... + + +def test_xarray_opens_the_round_tripped_store(tmp_path, sample): + ... +``` + +`test_the_crs_variable_arrives_as_a_coordinate_not_a_coordinate_system` carries +the Review Focus 1 docstring: the sample's data variables name `spatial_ref` in +`coordinates` and carry no `grid_mapping` attribute, so there is nothing for +Iris to build a coordinate system from, and the issue Task 7 raises is cited by +number. + +`test_an_attribute_named_dtype_does_not_shadow_the_dtype` carries the Review +Focus 2 docstring: the publisher records the source NumPy dtype in an attribute +of that name, and `dtype` is one of the names that used to be shadowed on load. + +`test_an_existing_store_is_replaced_under_mode_w` saves one cube to a path, +then saves a differently named cube to the same path with `mode="w"`, and +asserts the reloaded store holds only the second. `test_save.py` pins what +`mode="w-"` refuses and what `mode="a"` adds; `mode="w"` is the third of +§4.5's three modes. + +`test_the_round_trip_does_not_keep_the_source_chunk_layout` opens both stores +with `zarr.open_group` and asserts the source array is sharded — `shards == +(1, 4, 120, 120)` — while the saved one is not. Its docstring says Iris sizes +chunks itself and neither §4.4 nor §4.5 promises to preserve a store's layout +(F2). + +`test_xarray_opens_the_round_tripped_store` saves all four cubes, opens the +result with `xarray.open_zarr`, and asserts the four data variable names. The +assertion is on the open, because the failure mode this guards is a `TypeError` +during open (spec §6). + +- [ ] **Step 2: Run the module** + +```bash +conda run -n iris-dev pytest lib/iris/tests/integration/zarr/test_gfs_sample.py -v +``` + +Expected: every test PASSES, and none is skipped. A skip means +`TEST_DATA_DIR` is not pointing at Task 1's output — fix that rather than +accepting it, per `lib/iris/tests/AGENTS.md`. + +- [ ] **Step 3: Correct the synthetic fixture's docstring** + +`fixtures.py` says "A cut-down copy of the real store replaces this." It does +not: the synthetic store keeps `test_load.py` running where `iris-test-data` is +absent, and it is the more CF-conforming of the two, because it gives its data +variable a `grid_mapping` attribute that the published store does not have. +Replace that sentence with what is true, and say where the real store's tests +live. + +- [ ] **Step 4: Run the whole Zarr integration package** + +```bash +conda run -n iris-dev pytest lib/iris/tests/integration/zarr/ -v +``` + +Expected: everything that passed before still passes, and the new module's +tests pass. + +- [ ] **Step 5: Commit** + +```bash +git add lib/iris/tests/integration/zarr/test_gfs_sample.py \ + lib/iris/tests/integration/zarr/fixtures.py +git commit -m "Load and save the NOAA GFS sample store" +``` + +--- + +## Task 4: A fixed-length unicode variable loads + +The one thing the EOPF fixture was going to cover that nothing else does (F6). +It needs no external data, which is the whole reason it survives the cut. + +**Files:** +- Modify: `lib/iris/tests/unit/fileformats/zarr/loader/test_load_cubes.py` + +**Interfaces:** +- Consumes: `iris.fileformats.zarr.load_cubes`, already imported in that module. +- Produces: nothing other code calls. + +- [ ] **Step 1: Write the test** + +Add one parametrised test beside the existing synthetic load tests. + +```python +@pytest.mark.parametrize("zarr_format", [2, 3]) +def test_a_fixed_length_unicode_variable_loads(tmp_path, zarr_format): + ... + assert coord.points.tolist() == ["b02", "b03", "b04"] +``` + +It builds a store in `tmp_path` holding a ` Date: Mon, 26 Oct 2026 09:13:08 +0000 Subject: [PATCH 06/18] Record the preview pull request number Co-Authored-By: Claude Opus 5 Session-Id: 511ac21a-e935-4d77-baa6-1b8cb42dac17 --- docs/superpowers/specs/2026-09-21-zarr-io-design.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/superpowers/specs/2026-09-21-zarr-io-design.md b/docs/superpowers/specs/2026-09-21-zarr-io-design.md index 4b7cf7e7cf..9d4ddf5fac 100644 --- a/docs/superpowers/specs/2026-09-21-zarr-io-design.md +++ b/docs/superpowers/specs/2026-09-21-zarr-io-design.md @@ -9,7 +9,7 @@ | | | |---|---| | **Phase** | In delivery | -| **Progress** | PRs 1, 2, 2.5, 3, 4 and 5 merged into `brownfield`; PR 6 in review ([#7346](https://github.com/SciTools/iris/pull/7346)); PR 7 built on PR 6's branch, awaiting it; merge-back not started — see §12.1 | +| **Progress** | PRs 1, 2, 2.5, 3, 4 and 5 merged into `brownfield`; PR 6 in review ([#7346](https://github.com/SciTools/iris/pull/7346)); PR 7 built on PR 6's branch and previewed at [trexfeathers/iris#101](https://github.com/trexfeathers/iris/pull/101); merge-back not started — see §12.1 | | **Next action** | PR 6's review, then PR 7's rebase onto `brownfield`, then PR 8 — see §12.1 | | **Blocked on** | Nothing | | **Branch** | `zarr-io-design` on `bjlittle/iris`, targeting `SciTools/iris:brownfield` | @@ -2295,7 +2295,7 @@ nothing else, and can land beside any of the others. | 4 | Zarr loading | Merged | [#7336](https://github.com/SciTools/iris/pull/7336) | | 5 | Relocate the CF saver | Merged | [#7338](https://github.com/SciTools/iris/pull/7338) | | 6 | Zarr saving | In review | [#7346](https://github.com/SciTools/iris/pull/7346) | -| 7 | Real-world test data and benchmarks | Built, awaiting PR 6 | — | +| 7 | Real-world test data and benchmarks | Built, awaiting PR 6. Previewed at [trexfeathers/iris#101](https://github.com/trexfeathers/iris/pull/101), targeting PR 6's branch so the diff can be read before PR 6 merges | — | | 8 | The terms-of-art sweep | Not started | — | | — | **Merge back** `brownfield` → `main`: closing keywords, changelog fragments, docs | Not started | — | From d2d04bf851d312e47dd2d7e37f1663dba4ed7b3d Mon Sep 17 00:00:00 2001 From: Martin Yeo Date: Mon, 26 Oct 2026 09:38:32 +0000 Subject: [PATCH 07/18] Skip the GFS sample tests when the sample is absent They failed instead. _shared_utils.skip_data asks only whether iris-test-data is present at all, and get_data_path does not check that the path it builds exists, so a checkout predating the store gave eleven failures rather than eleven skips. Iris and iris-test-data are versioned separately, so that window reopens whenever one moves before the other. Also moves the xarray import into the one test that needs it, so an install with zarr but without xarray loses that test rather than the whole module. Corrects three things in the design document that this branch's own section 12.4 entry contradicts: section 7 called spatial_ref a grid mapping when the store declares no grid_mapping attribute, said both sources are contributed when only one is, and named the wrong fork. The header table said nothing was blocked. Co-Authored-By: Claude Opus 5 Session-Id: 511ac21a-e935-4d77-baa6-1b8cb42dac17 --- .../plans/2026-10-26-zarr-io-pr7.md | 13 +++++++++++- .../specs/2026-09-21-zarr-io-design.md | 21 ++++++++++++------- .../tests/integration/zarr/test_gfs_sample.py | 20 ++++++++++++++++-- 3 files changed, 44 insertions(+), 10 deletions(-) diff --git a/docs/superpowers/plans/2026-10-26-zarr-io-pr7.md b/docs/superpowers/plans/2026-10-26-zarr-io-pr7.md index 8f1925fd2e..18fa721678 100644 --- a/docs/superpowers/plans/2026-10-26-zarr-io-pr7.md +++ b/docs/superpowers/plans/2026-10-26-zarr-io-pr7.md @@ -953,7 +953,18 @@ Apply the `Agentic` and `Type: Feature Branch` labels. Blocked until the companion pull request merges and `SciTools/iris-test-data` publishes a new version tag. Everything else in this plan can be done without it; nothing in this plan can finish without it, because until it is done CI -tests the new module by skipping it. +runs against a version of the test data that does not hold the store. + +**This task's original premise was wrong, and the whole-branch review caught +it.** It said CI would report the branch green by skipping the new module. +`_shared_utils.skip_data` asks only whether `iris-test-data` is present at +all, and `get_data_path` does not check that the path it builds exists, so +the eleven tests *failed* rather than skipped — eleven failures on any +checkout without the companion merged, CI included. Task 3's module therefore +gained a `pytestmark` guard on the store's own existence, which is what makes +the skip real. The guard stays after this task lands: Iris and +`iris-test-data` are versioned separately, so the window it covers reopens +every time one moves before the other. **Files:** - Modify: `.github/workflows/ci-tests.yml` diff --git a/docs/superpowers/specs/2026-09-21-zarr-io-design.md b/docs/superpowers/specs/2026-09-21-zarr-io-design.md index 9d4ddf5fac..2f1b768358 100644 --- a/docs/superpowers/specs/2026-09-21-zarr-io-design.md +++ b/docs/superpowers/specs/2026-09-21-zarr-io-design.md @@ -10,8 +10,8 @@ |---|---| | **Phase** | In delivery | | **Progress** | PRs 1, 2, 2.5, 3, 4 and 5 merged into `brownfield`; PR 6 in review ([#7346](https://github.com/SciTools/iris/pull/7346)); PR 7 built on PR 6's branch and previewed at [trexfeathers/iris#101](https://github.com/trexfeathers/iris/pull/101); merge-back not started — see §12.1 | -| **Next action** | PR 6's review, then PR 7's rebase onto `brownfield`, then PR 8 — see §12.1 | -| **Blocked on** | Nothing | +| **Next action** | Raise PR 7's companion pull request on `SciTools/iris-test-data`, then PR 6's review, then PR 7's rebase onto `brownfield`, then PR 8 — see §12.1 | +| **Blocked on** | PR 7's tests, which skip until the companion `iris-test-data` pull request merges and `IRIS_TEST_DATA_VERSION` is bumped — see §12.1 | | **Branch** | `zarr-io-design` on `bjlittle/iris`, targeting `SciTools/iris:brownfield` | | **Delivery** | Nine pull requests into `brownfield`, then one merge-back into `main` — §5 | | **Resolves** | SciTools/iris#6977, #6979, #6980 — at merge-back, not before | @@ -2096,8 +2096,9 @@ installed must show no new failures and no quietly skipped netCDF test. ## 7. Real-world test data -Two sources, both cut down before contribution to `SciTools/iris-test-data` -(fork `bjlittle/iris-test-data`, default branch `master`). +Two sources. Only the first is contributed to `SciTools/iris-test-data`, via +the fork `trexfeathers/iris-test-data`, default branch `master`; §5 says why +the second is cited here but not shipped. **NOAA GFS, dynamical.org** — the version 3 fixture. Confirmed by fetching `https://data.dynamical.org/noaa/gfs/forecast/latest.zarr/zarr.json`: @@ -2112,11 +2113,17 @@ therefore already been cut, ahead of the rest of the programme; §12.5 holds its current location. It is 1.4 MB, 35 files, 12 members, 2 forecast initialisations x 8 lead times x 120 x 120 points, for `temperature_2m`, `wind_u_10m`, `wind_v_10m` and `precipitation_surface`. It -keeps the sharding codec, the scalar `spatial_ref` grid mapping, the +keeps the sharding codec, the scalar `spatial_ref` variable, the two-dimensional `valid_time` auxiliary coordinate, all source attributes verbatim including the base64 `_FillValue`, and the `CC-BY-4.0` licence and attribution in the root group. +`spatial_ref` is a CRS variable but **not** a grid mapping as CF defines one: +the data variables name it in `coordinates` and declare no `grid_mapping` +attribute, so Iris builds no coordinate system from it **[verified]**. §12.4 +records what follows for the fixture, and it is why the synthetic store in +`integration/zarr/fixtures.py` is kept rather than replaced. + Two of those are load-bearing rather than incidental. The **sharding codec** makes this the fixture that exercises the read-unit rule in §4.4 and the write-alignment invariant in §4.5, the two rules in this design most easily got @@ -2295,7 +2302,7 @@ nothing else, and can land beside any of the others. | 4 | Zarr loading | Merged | [#7336](https://github.com/SciTools/iris/pull/7336) | | 5 | Relocate the CF saver | Merged | [#7338](https://github.com/SciTools/iris/pull/7338) | | 6 | Zarr saving | In review | [#7346](https://github.com/SciTools/iris/pull/7346) | -| 7 | Real-world test data and benchmarks | Built, awaiting PR 6. Previewed at [trexfeathers/iris#101](https://github.com/trexfeathers/iris/pull/101), targeting PR 6's branch so the diff can be read before PR 6 merges | — | +| 7 | Real-world test data and benchmarks | Built, awaiting PR 6 | [trexfeathers/iris#101](https://github.com/trexfeathers/iris/pull/101), a preview against PR 6's branch | | 8 | The terms-of-art sweep | Not started | — | | — | **Merge back** `brownfield` → `main`: closing keywords, changelog fragments, docs | Not started | — | @@ -3286,5 +3293,5 @@ raised as its own issue, not taken in a pull request that adds test data. | 2026-10-23 | PR 6 rebased onto `brownfield` now that #7336 and #7338 have merged, and raised as #7346. The rebase met three things that landed while it was being built: the generic saver renamed `_CFSaver`, cubelist savers registered by identity rather than by module name, and UGRID mesh loading, which turned the mesh round trip from a strict `xfail` into a passing test. | | 2026-10-23 | Actioned the Codex review of #7346. Five findings in PR 6's own work, all reproduced and fixed, and one unguarded Zarr import in a netCDF test module. §12.3 and §12.4 record them. The same review found this document contradicting itself in four places, each one a change made in §4.2 or in the decision log and not carried through to §5, §6 or §12.3: the `storage_settings` rename, the removal of read-side alignment, the skipping rule, and Q10's closure. Reconciled, and Q4 closed, its premise having been the alignment guard that was removed. No design change. | | 2026-10-23 | `supports_dtype` restated in §4.2 as a table of the dtypes every accepted zarr-python version holds, rather than a list of the dtypes to exclude: the zarr-python 3.0.8 data-type enum is much smaller than 3.4's registry, so any exclusion list is wrong on the version floor Iris declares. The header table, which had gone stale against §12.1, now carries the rule that it is a summary of §12.1 and moves when §12.1 moves. | -| 2026-10-26 | PR 7 built: the NOAA GFS subset contributed to `iris-test-data`, eleven integration tests over it, a read-side test for a fixed-length unicode variable, `ZarrSave` and a `"Zarr"` file format in `LoadAndRealise`. The ESA EOPF fixture dropped on licence grounds and §5 and §7 amended to say so; §7 keeps the store as cited evidence. Six findings logged, four of them things the published store does that no synthetic fixture did. | | 2026-10-23 | Byte order removed from the capability question rather than documented as an exception to it: `create_variable` creates in the platform's own byte order, so `supports_dtype` can answer `True` for a big-endian dtype on every accepted version. §4.2 says why neither obvious answer was tenable. The version claims are run rather than read — zarr-python 3.0.8 installed in a throwaway environment, every dtype in the table created in turn, on 3.0.8 and 3.4.1 both. | +| 2026-10-26 | PR 7 built: the NOAA GFS subset contributed to `iris-test-data`, eleven integration tests over it, a read-side test for a fixed-length unicode variable, `ZarrSave` and a `"Zarr"` file format in `LoadAndRealise`. The ESA EOPF fixture dropped on licence grounds and §5 and §7 amended to say so; §7 keeps the store as cited evidence. Six findings logged, four of them things the published store does that no synthetic fixture did. | diff --git a/lib/iris/tests/integration/zarr/test_gfs_sample.py b/lib/iris/tests/integration/zarr/test_gfs_sample.py index b06d7df26a..0e160578c8 100644 --- a/lib/iris/tests/integration/zarr/test_gfs_sample.py +++ b/lib/iris/tests/integration/zarr/test_gfs_sample.py @@ -13,13 +13,13 @@ section 7 says what the subset keeps and why. """ +from pathlib import Path import warnings import numpy as np import pytest zarr = pytest.importorskip("zarr") -xr = pytest.importorskip("xarray") import iris from iris.loading import LOAD_PROBLEMS @@ -30,10 +30,21 @@ SOURCE_SHARDS = (1, 4, 120, 120) SOURCE_CHUNKS = (1, 2, 40, 40) +SAMPLE = Path(_shared_utils.get_data_path(("Zarr", "gfs_forecast_sample.zarr"))) + +# `skip_data` only asks whether iris-test-data is present at all, so without +# this every test here FAILS rather than skips on a checkout that predates +# the store. iris and iris-test-data are versioned separately, so there is +# always a window where one has the test and the other lacks the data. +pytestmark = pytest.mark.skipif( + not SAMPLE.is_dir(), + reason=f"No {SAMPLE.name} in iris-test-data; see IRIS_TEST_DATA_VERSION.", +) + @pytest.fixture def sample(): - return _shared_utils.get_data_path(("Zarr", "gfs_forecast_sample.zarr")) + return str(SAMPLE) @pytest.fixture(autouse=True) @@ -187,6 +198,11 @@ def test_the_round_trip_does_not_keep_the_source_chunk_layout(tmp_path, sample): @_shared_utils.skip_data def test_xarray_opens_the_round_tripped_store(tmp_path, sample): """Asserted on the open, because a bad attribute fails the whole open.""" + # Imported here, not at module scope: xarray is a test-only dependency + # and only this test needs it, so an install without it should lose + # this test rather than every test in the module. + xr = pytest.importorskip("xarray") + cubes = iris.load(sample) with iris.FUTURE.context(save_split_attrs=True): iris.save(cubes, tmp_path / "rt.zarr") From 2decf75928f914c1131719ad3911dd031875c910 Mon Sep 17 00:00:00 2001 From: Martin Yeo Date: Mon, 26 Oct 2026 11:10:15 +0000 Subject: [PATCH 08/18] Settle the grid-mapping question, and record the companion issues The published GFS store names its CRS variable in coordinates and declares no grid_mapping attribute, so Iris builds no coordinate system from it. A fallback reading crs_wkt off an unreferenced scalar coordinate was proposed and rejected: section 1 already says a reader is never bent to fit one publisher's files, and the specification is what decides. Section 12.4 now records the decision rather than leaving the question open. Records SciTools/iris-test-data#98 and #99 in section 12.1, and names #98 as what the header table's Blocked on entry waits for. Co-Authored-By: Claude Opus 5 Session-Id: 511ac21a-e935-4d77-baa6-1b8cb42dac17 --- .../specs/2026-09-21-zarr-io-design.md | 31 ++++++++++++------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/docs/superpowers/specs/2026-09-21-zarr-io-design.md b/docs/superpowers/specs/2026-09-21-zarr-io-design.md index 2f1b768358..cd15054e0f 100644 --- a/docs/superpowers/specs/2026-09-21-zarr-io-design.md +++ b/docs/superpowers/specs/2026-09-21-zarr-io-design.md @@ -10,8 +10,8 @@ |---|---| | **Phase** | In delivery | | **Progress** | PRs 1, 2, 2.5, 3, 4 and 5 merged into `brownfield`; PR 6 in review ([#7346](https://github.com/SciTools/iris/pull/7346)); PR 7 built on PR 6's branch and previewed at [trexfeathers/iris#101](https://github.com/trexfeathers/iris/pull/101); merge-back not started — see §12.1 | -| **Next action** | Raise PR 7's companion pull request on `SciTools/iris-test-data`, then PR 6's review, then PR 7's rebase onto `brownfield`, then PR 8 — see §12.1 | -| **Blocked on** | PR 7's tests, which skip until the companion `iris-test-data` pull request merges and `IRIS_TEST_DATA_VERSION` is bumped — see §12.1 | +| **Next action** | PR 6's review, then PR 7's rebase onto `brownfield`, then PR 8 — see §12.1 | +| **Blocked on** | PR 7's tests, which skip until [SciTools/iris-test-data#98](https://github.com/SciTools/iris-test-data/pull/98) merges and `IRIS_TEST_DATA_VERSION` is bumped — see §12.1 | | **Branch** | `zarr-io-design` on `bjlittle/iris`, targeting `SciTools/iris:brownfield` | | **Delivery** | Nine pull requests into `brownfield`, then one merge-back into `main` — §5 | | **Resolves** | SciTools/iris#6977, #6979, #6980 — at merge-back, not before | @@ -2310,7 +2310,8 @@ Companion work outside `SciTools/iris`: | Repository | Purpose | State | Link | |---|---|---|---| -| `SciTools/iris-test-data` | Cut-down NOAA GFS (v3) fixture for PR 7, and a `.zarr` directory indexed as one item | Built, awaiting PR | — | +| `SciTools/iris-test-data` | Cut-down NOAA GFS (v3) fixture for PR 7, and a `.zarr` directory indexed as one item | In review | [#98](https://github.com/SciTools/iris-test-data/pull/98) | +| `SciTools/iris-test-data` | The repository-wide licence statement, which #98's data does not fit | Raised, not for this programme to settle | [#99](https://github.com/SciTools/iris-test-data/issues/99) | States are `Not started`, `Drafted`, `Built, awaiting PR N` (work complete on a branch based on another pull request not yet merged, with no number of its @@ -3201,14 +3202,22 @@ does not have. Its data variables name `spatial_ref` in `coordinates` and carry no `grid_mapping` attribute at all, so `spatial_ref` arrives as a scalar auxiliary coordinate carrying `crs_wkt` and the cube's `latitude` has no - coordinate system **[verified]**. §7 says the subset keeps "the scalar - `spatial_ref` grid mapping", which is true of the variable and false of the - reference. This is CF behaving as specified — §5.6 asks for a - `grid_mapping` attribute and this publisher does not write one — so Iris is - unchanged and the behaviour is pinned by test instead. It is also why the - synthetic fixture in `integration/zarr/fixtures.py` is kept rather than - replaced: it gives its data variable a `grid_mapping` attribute, so it is - the more CF-conforming of the two and reaches code the real store does not. + coordinate system **[verified]**. §7 said the subset keeps "the scalar + `spatial_ref` grid mapping", which was true of the variable and false of + the reference; §7 now says so. + + **Settled, and not an open question.** CF §5.6 makes `grid_mapping` the way + a data variable points at its grid mapping, so Iris is already right and + nothing changes. A fallback — reading `crs_wkt` off an unreferenced scalar + coordinate — was proposed and rejected by @trexfeathers: a library that + second-guesses every file it might meet has given up the specification as + the thing that decides, which is the rule §1 states and the one the + `_FillValue` episode in §12.4 was the single deliberate exception to. The + behaviour is pinned by test so that the absent coordinate system reads as + the data's doing rather than as a regression. It is also why the synthetic + fixture in `integration/zarr/fixtures.py` is kept rather than replaced: it + gives its data variable a `grid_mapping` attribute, so it is the more + CF-conforming of the two and reaches code the published store does not. - **The published store carries an attribute named `dtype`.** Three of its coordinates hold `dtype: "timedelta64[us]"` as producer bookkeeping. That is one of the six names this log records as having been silently shadowed From 13e191d56e50af5326d7c61a441afab7ceca2a7f Mon Sep 17 00:00:00 2001 From: Martin Yeo Date: Tue, 27 Oct 2026 08:02:42 +0000 Subject: [PATCH 09/18] Pin the iris-test-data version carrying the Zarr sample v2.31 is the first release holding test_data/Zarr/gfs_forecast_sample.zarr, so until now the eleven integration tests over it skipped. Both workflow files pin the version and the comment in ci-tests.yml points at the other. Co-Authored-By: Claude Opus 5 Session-Id: 0022a6ec-642a-4844-ae3b-6b8f4421303a --- .github/workflows/benchmarks_run.yml | 2 +- .github/workflows/ci-tests.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/benchmarks_run.yml b/.github/workflows/benchmarks_run.yml index bada87bd85..949f3dcaef 100644 --- a/.github/workflows/benchmarks_run.yml +++ b/.github/workflows/benchmarks_run.yml @@ -73,7 +73,7 @@ jobs: env: IRIS_TEST_DATA_LOC_PATH: benchmarks IRIS_TEST_DATA_PATH: benchmarks/iris-test-data - IRIS_TEST_DATA_VERSION: "2.30" + IRIS_TEST_DATA_VERSION: "2.31" # Lets us manually bump the cache to rebuild ENV_CACHE_BUILD: "1" TEST_DATA_CACHE_BUILD: "2" diff --git a/.github/workflows/ci-tests.yml b/.github/workflows/ci-tests.yml index 45ec8cb151..97a3c88cb4 100644 --- a/.github/workflows/ci-tests.yml +++ b/.github/workflows/ci-tests.yml @@ -54,7 +54,7 @@ jobs: env: # NOTE: IRIS_TEST_DATA_VERSION is also set in benchmarks_run.yml - IRIS_TEST_DATA_VERSION: "2.30" + IRIS_TEST_DATA_VERSION: "2.31" ENV_NAME: "ci-tests" steps: From dadeac31ef8134d30e7c3bcf63c01de048c7bf47 Mon Sep 17 00:00:00 2001 From: Martin Yeo Date: Tue, 27 Oct 2026 08:03:27 +0000 Subject: [PATCH 10/18] Record the test-data release that unblocks PR 7's tests Co-Authored-By: Claude Opus 5 Session-Id: 0022a6ec-642a-4844-ae3b-6b8f4421303a --- docs/superpowers/specs/2026-09-21-zarr-io-design.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/docs/superpowers/specs/2026-09-21-zarr-io-design.md b/docs/superpowers/specs/2026-09-21-zarr-io-design.md index cd15054e0f..17ea7e32f8 100644 --- a/docs/superpowers/specs/2026-09-21-zarr-io-design.md +++ b/docs/superpowers/specs/2026-09-21-zarr-io-design.md @@ -11,12 +11,12 @@ | **Phase** | In delivery | | **Progress** | PRs 1, 2, 2.5, 3, 4 and 5 merged into `brownfield`; PR 6 in review ([#7346](https://github.com/SciTools/iris/pull/7346)); PR 7 built on PR 6's branch and previewed at [trexfeathers/iris#101](https://github.com/trexfeathers/iris/pull/101); merge-back not started — see §12.1 | | **Next action** | PR 6's review, then PR 7's rebase onto `brownfield`, then PR 8 — see §12.1 | -| **Blocked on** | PR 7's tests, which skip until [SciTools/iris-test-data#98](https://github.com/SciTools/iris-test-data/pull/98) merges and `IRIS_TEST_DATA_VERSION` is bumped — see §12.1 | +| **Blocked on** | Nothing | | **Branch** | `zarr-io-design` on `bjlittle/iris`, targeting `SciTools/iris:brownfield` | | **Delivery** | Nine pull requests into `brownfield`, then one merge-back into `main` — §5 | | **Resolves** | SciTools/iris#6977, #6979, #6980 — at merge-back, not before | -**Created:** 2026-09-21 · **Last updated:** 2026-10-26 (see §12.6) +**Created:** 2026-09-21 · **Last updated:** 2026-10-27 (see §12.6) **Baseline:** `brownfield` at `b0ab90b98`, Iris `3.17.0.dev4` **Verified against:** zarr-python 3.4.0, numcodecs 0.16.5, Python 3.13, dask 2026.7.1, xarray 2026.7.0 @@ -2310,7 +2310,7 @@ Companion work outside `SciTools/iris`: | Repository | Purpose | State | Link | |---|---|---|---| -| `SciTools/iris-test-data` | Cut-down NOAA GFS (v3) fixture for PR 7, and a `.zarr` directory indexed as one item | In review | [#98](https://github.com/SciTools/iris-test-data/pull/98) | +| `SciTools/iris-test-data` | Cut-down NOAA GFS (v3) fixture for PR 7, and a `.zarr` directory indexed as one item | Merged, released as v2.31 | [#98](https://github.com/SciTools/iris-test-data/pull/98) | | `SciTools/iris-test-data` | The repository-wide licence statement, which #98's data does not fit | Raised, not for this programme to settle | [#99](https://github.com/SciTools/iris-test-data/issues/99) | States are `Not started`, `Drafted`, `Built, awaiting PR N` (work complete on @@ -3247,7 +3247,7 @@ does not have. | Artefact | Location | State | |---|---|---| -| Cut-down NOAA GFS fixture, 1.4 MB, 12 members, Zarr v3 | `~/iris-zarr-testdata/gfs_forecast_sample.zarr` | Contributed to `iris-test-data` by PR 7's companion, as `test_data/Zarr/gfs_forecast_sample.zarr` | +| Cut-down NOAA GFS fixture, 1.4 MB, 12 members, Zarr v3 | `iris-test-data` v2.31, as `test_data/Zarr/gfs_forecast_sample.zarr` | Released. The cut it was made from is at `~/iris-zarr-testdata/gfs_forecast_sample.zarr` | | Cut-down ESA EOPF fixture, Zarr v2 | — | Not pursued. §5 and §7 say why | The GFS subset was cut ahead of the rest of the programme because its source @@ -3304,3 +3304,4 @@ raised as its own issue, not taken in a pull request that adds test data. | 2026-10-23 | `supports_dtype` restated in §4.2 as a table of the dtypes every accepted zarr-python version holds, rather than a list of the dtypes to exclude: the zarr-python 3.0.8 data-type enum is much smaller than 3.4's registry, so any exclusion list is wrong on the version floor Iris declares. The header table, which had gone stale against §12.1, now carries the rule that it is a summary of §12.1 and moves when §12.1 moves. | | 2026-10-23 | Byte order removed from the capability question rather than documented as an exception to it: `create_variable` creates in the platform's own byte order, so `supports_dtype` can answer `True` for a big-endian dtype on every accepted version. §4.2 says why neither obvious answer was tenable. The version claims are run rather than read — zarr-python 3.0.8 installed in a throwaway environment, every dtype in the table created in turn, on 3.0.8 and 3.4.1 both. | | 2026-10-26 | PR 7 built: the NOAA GFS subset contributed to `iris-test-data`, eleven integration tests over it, a read-side test for a fixed-length unicode variable, `ZarrSave` and a `"Zarr"` file format in `LoadAndRealise`. The ESA EOPF fixture dropped on licence grounds and §5 and §7 amended to say so; §7 keeps the store as cited evidence. Six findings logged, four of them things the published store does that no synthetic fixture did. | +| 2026-10-27 | [SciTools/iris-test-data#98](https://github.com/SciTools/iris-test-data/pull/98) merged and released as v2.31, so `IRIS_TEST_DATA_VERSION` is bumped in both workflow files and PR 7's integration tests run rather than skip. The indexer's store handling was rewritten on review by @pp-mo, from filtering a full walk to recursive descent; that also removed a bug where a checkout under any directory named `*.zarr` suppressed the whole index. | From cbb30a897503f2eaf69f35dea9c4f008c3dda5af Mon Sep 17 00:00:00 2001 From: Martin Yeo Date: Tue, 27 Oct 2026 08:28:12 +0000 Subject: [PATCH 11/18] Action review: drop the custom skipper, parametrise the save benchmark The GFS tests carried both a module skipif on the store's own existence and the standard skip_data decorators. Reviewers objected from opposite sides, and skip_data is the one to keep: it also honours IRIS_TEST_NO_DATA, which the custom guard ignored, so keeping only the custom guard would have run these tests for someone who had asked the suite not to touch external data. The guard existed because the data was unreleased; iris-test-data v2.31 is pinned now, and there is no backwards-compatibility contract with older versions. NetcdfSave and ZarrSave were near-identical, so they become one Save class parametrised over file_format, the way LoadAndRealise and TimeConstraint already are. time_save_mesh is guarded on the format rather than skipped, because ASV's skip facility raises out of setup() and so cannot skip one method for one parameter value. Co-Authored-By: Claude Opus 5 Session-Id: 0022a6ec-642a-4844-ae3b-6b8f4421303a --- benchmarks/benchmarks/save.py | 64 +++++++------------ .../tests/integration/zarr/test_gfs_sample.py | 14 +--- .../zarr/loader/test_load_cubes.py | 2 +- 3 files changed, 24 insertions(+), 56 deletions(-) diff --git a/benchmarks/benchmarks/save.py b/benchmarks/benchmarks/save.py index ce9cd68a31..efdc5d7940 100644 --- a/benchmarks/benchmarks/save.py +++ b/benchmarks/benchmarks/save.py @@ -9,68 +9,48 @@ from .generate_data.ugrid import make_cube_like_2d_cubesphere +#: Target name and save keywords per format. Zarr needs mode="w" because its +#: saver defaults to mode="w-" and raises FileExistsError on a target that +#: exists - and ASV calls a timing function many times between setup() calls. +SAVE_TARGETS = { + "NetCDF": ("tmp.nc", {}), + "Zarr": ("tmp.zarr", {"mode": "w"}), +} -class NetcdfSave: - params = [[50, 600], [False, True]] - param_names = ["cubesphere-N", "is_unstructured"] - def setup(self, n_cubesphere, is_unstructured): +class Save: + params = [[50, 600], [False, True], list(SAVE_TARGETS)] + param_names = ["cubesphere-N", "is_unstructured", "file_format"] + + def setup(self, n_cubesphere, is_unstructured, file_format): self.cube = make_cube_like_2d_cubesphere( n_cube=n_cubesphere, with_mesh=is_unstructured ) + self.save_target, self.save_kwargs = SAVE_TARGETS[file_format] def _save_data(self, cube, do_copy=True): if do_copy: # Copy the cube, to avoid distorting the results by changing it # Because we known that older Iris code realises lazy coords cube = cube.copy() - save(cube, "tmp.nc") + save(cube, self.save_target, **self.save_kwargs) def _save_mesh(self, cube): # In this case, we are happy that the mesh is *not* modified save_mesh(cube.mesh, "mesh.nc") - def time_netcdf_save_cube(self, n_cubesphere, is_unstructured): + def time_save_cube(self, n_cubesphere, is_unstructured, file_format): self._save_data(self.cube) - def time_netcdf_save_mesh(self, n_cubesphere, is_unstructured): - if is_unstructured: + def time_save_mesh(self, n_cubesphere, is_unstructured, file_format): + # iris.mesh.save_mesh is defined in iris.fileformats.netcdf.saver and + # writes netCDF only, so there is nothing to time for Zarr. A cube + # that *has* a mesh still saves it through time_save_cube, with + # is_unstructured=True. + if is_unstructured and file_format == "NetCDF": self._save_mesh(self.cube) - def tracemalloc_netcdf_save(self, n_cubesphere, is_unstructured): - # Don't need to copy the cube here since track_ benchmarks don't - # do repeats between self.setup() calls. - self._save_data(self.cube, do_copy=False) - - -class ZarrSave: - params = [[50, 600], [False, True]] - param_names = ["cubesphere-N", "is_unstructured"] - - def setup(self, n_cubesphere, is_unstructured): - self.cube = make_cube_like_2d_cubesphere( - n_cube=n_cubesphere, with_mesh=is_unstructured - ) - - def _save_data(self, cube, do_copy=True): - if do_copy: - # Copy the cube, to avoid distorting the results by changing it - # Because we known that older Iris code realises lazy coords - cube = cube.copy() - # mode="w" because the Zarr saver defaults to mode="w-" and raises - # FileExistsError on a target that exists - and ASV calls this many - # times between setup() calls. - save(cube, "tmp.zarr", mode="w") - - def time_zarr_save_cube(self, n_cubesphere, is_unstructured): - self._save_data(self.cube) - - def tracemalloc_zarr_save(self, n_cubesphere, is_unstructured): + def tracemalloc_save(self, n_cubesphere, is_unstructured, file_format): # Don't need to copy the cube here since track_ benchmarks don't # do repeats between self.setup() calls. self._save_data(self.cube, do_copy=False) - - # No mesh-only equivalent of NetcdfSave.time_netcdf_save_mesh: - # iris.mesh.save_mesh is defined in iris.fileformats.netcdf.saver and - # writes netCDF only. A cube that has a mesh does save its mesh here, - # through is_unstructured=True. diff --git a/lib/iris/tests/integration/zarr/test_gfs_sample.py b/lib/iris/tests/integration/zarr/test_gfs_sample.py index 0e160578c8..478fdca0c0 100644 --- a/lib/iris/tests/integration/zarr/test_gfs_sample.py +++ b/lib/iris/tests/integration/zarr/test_gfs_sample.py @@ -13,7 +13,6 @@ section 7 says what the subset keeps and why. """ -from pathlib import Path import warnings import numpy as np @@ -30,21 +29,10 @@ SOURCE_SHARDS = (1, 4, 120, 120) SOURCE_CHUNKS = (1, 2, 40, 40) -SAMPLE = Path(_shared_utils.get_data_path(("Zarr", "gfs_forecast_sample.zarr"))) - -# `skip_data` only asks whether iris-test-data is present at all, so without -# this every test here FAILS rather than skips on a checkout that predates -# the store. iris and iris-test-data are versioned separately, so there is -# always a window where one has the test and the other lacks the data. -pytestmark = pytest.mark.skipif( - not SAMPLE.is_dir(), - reason=f"No {SAMPLE.name} in iris-test-data; see IRIS_TEST_DATA_VERSION.", -) - @pytest.fixture def sample(): - return str(SAMPLE) + return _shared_utils.get_data_path(("Zarr", "gfs_forecast_sample.zarr")) @pytest.fixture(autouse=True) diff --git a/lib/iris/tests/unit/fileformats/zarr/loader/test_load_cubes.py b/lib/iris/tests/unit/fileformats/zarr/loader/test_load_cubes.py index 977b28096b..85f661155a 100644 --- a/lib/iris/tests/unit/fileformats/zarr/loader/test_load_cubes.py +++ b/lib/iris/tests/unit/fileformats/zarr/loader/test_load_cubes.py @@ -138,7 +138,7 @@ def test_a_group_with_no_arrays_names_the_groups_that_have_them(tmp_path): @pytest.mark.parametrize("zarr_format", [2, 3]) def test_a_fixed_length_unicode_variable_loads(tmp_path, zarr_format): - """` Date: Tue, 27 Oct 2026 08:28:36 +0000 Subject: [PATCH 12/18] Benchmark a time constraint on Zarr too No technical reason it was absent - the design document named LoadAndRealise and the save benchmarks, so that is what was built. setup_cache already passes the format list straight to create_um_files, so this is one word. Co-Authored-By: Claude Opus 5 Session-Id: 0022a6ec-642a-4844-ae3b-6b8f4421303a --- benchmarks/benchmarks/load/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/benchmarks/benchmarks/load/__init__.py b/benchmarks/benchmarks/load/__init__.py index b68819c729..abba7ea577 100644 --- a/benchmarks/benchmarks/load/__init__.py +++ b/benchmarks/benchmarks/load/__init__.py @@ -78,7 +78,7 @@ def time_stash_constraint(self, _, __, ___) -> None: class TimeConstraint: - params = ([3, 20], ["FF", "PP", "NetCDF"]) + params = ([3, 20], ["FF", "PP", "NetCDF", "Zarr"]) param_names = ["time_dim_len", "file_format"] def setup_cache(self) -> dict: From a4948c68613c2ec3b2718a6316e84311611dc9a6 Mon Sep 17 00:00:00 2001 From: Martin Yeo Date: Tue, 27 Oct 2026 08:44:32 +0000 Subject: [PATCH 13/18] Record the stored unicode fixtures, and why a generated test cannot pin them Section 6 gains the rule the review turned up: a test that writes an array and reads it back moves when the writing library moves, so it answers the reader-against-writer question and not the interoperability one. That needs bytes which do not move, which is SciTools/iris-test-data#100. Section 12.4 records the three other review actions, and 12.1 and 12.5 carry the new companion pull request. Co-Authored-By: Claude Opus 5 Session-Id: 0022a6ec-642a-4844-ae3b-6b8f4421303a --- .../specs/2026-09-21-zarr-io-design.md | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/docs/superpowers/specs/2026-09-21-zarr-io-design.md b/docs/superpowers/specs/2026-09-21-zarr-io-design.md index 17ea7e32f8..96f846878d 100644 --- a/docs/superpowers/specs/2026-09-21-zarr-io-design.md +++ b/docs/superpowers/specs/2026-09-21-zarr-io-design.md @@ -1956,6 +1956,18 @@ question needs a table of the storage library's data types that nothing consults and nothing checks. What is pinned for those instead is that a dtype zarr-python refuses leaves the group as it was. +**A store a test writes cannot pin how a store was written.** A test that +writes an array with zarr-python and reads it back with Iris moves when +zarr-python moves: change the encoding and both halves change together, so +the test still passes while a file written years earlier stops loading. That +is fine for the reader-against-writer question, which is most of them, and +useless for the interoperability question — *this* encoding, which a +published file already contains, must still load. The second needs bytes +that do not move, which means a committed fixture in `iris-test-data` and a +test under `integration/`. Both are worth having, and neither replaces the +other. Fixed-length unicode is the case where it bites, because ` Date: Tue, 27 Oct 2026 08:49:29 +0000 Subject: [PATCH 14/18] Read fixed-length unicode from stored Zarr stores The unit test for Session-Id: 0022a6ec-642a-4844-ae3b-6b8f4421303a --- .../zarr/test_unicode_coords_sample.py | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 lib/iris/tests/integration/zarr/test_unicode_coords_sample.py diff --git a/lib/iris/tests/integration/zarr/test_unicode_coords_sample.py b/lib/iris/tests/integration/zarr/test_unicode_coords_sample.py new file mode 100644 index 0000000000..1f32ce6fa1 --- /dev/null +++ b/lib/iris/tests/integration/zarr/test_unicode_coords_sample.py @@ -0,0 +1,83 @@ +# Copyright Iris contributors +# +# This file is part of Iris and is released under the BSD license. +# See LICENSE in the root of the repository for full licensing details. +"""Stored Zarr stores, read back with their fixed-length unicode intact. + +The unit tests in `lib/iris/tests/unit/fileformats/zarr/` write a ` Date: Tue, 27 Oct 2026 08:50:17 +0000 Subject: [PATCH 15/18] Say that PR 7 is red, and why that was chosen Co-Authored-By: Claude Opus 5 Session-Id: 0022a6ec-642a-4844-ae3b-6b8f4421303a --- docs/superpowers/specs/2026-09-21-zarr-io-design.md | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/docs/superpowers/specs/2026-09-21-zarr-io-design.md b/docs/superpowers/specs/2026-09-21-zarr-io-design.md index 96f846878d..34bab1b5f9 100644 --- a/docs/superpowers/specs/2026-09-21-zarr-io-design.md +++ b/docs/superpowers/specs/2026-09-21-zarr-io-design.md @@ -11,7 +11,7 @@ | **Phase** | In delivery | | **Progress** | PRs 1, 2, 2.5, 3, 4 and 5 merged into `brownfield`; PR 6 in review ([#7346](https://github.com/SciTools/iris/pull/7346)); PR 7 built on PR 6's branch and previewed at [trexfeathers/iris#101](https://github.com/trexfeathers/iris/pull/101); merge-back not started — see §12.1 | | **Next action** | PR 6's review, then PR 7's rebase onto `brownfield`, then PR 8 — see §12.1 | -| **Blocked on** | Nothing | +| **Blocked on** | PR 7, whose unicode tests fail until [SciTools/iris-test-data#100](https://github.com/SciTools/iris-test-data/pull/100) merges and releases — deliberately committed red so the new test can be reviewed beside its fixtures | | **Branch** | `zarr-io-design` on `bjlittle/iris`, targeting `SciTools/iris:brownfield` | | **Delivery** | Nine pull requests into `brownfield`, then one merge-back into `main` — §5 | | **Resolves** | SciTools/iris#6977, #6979, #6980 — at merge-back, not before | @@ -3287,6 +3287,14 @@ does not have. `fixed_length_utf32` with `length_bytes: 28` on version 3 **[verified]**, and a test asserts those strings so that regenerating the fixtures — which would destroy their only value — fails rather than passing quietly. +- **That test is committed red**, against the usual rule that a branch stays + green. A reviewer cannot review a file that is not there, and the preview + pull request exists to be reviewed; the alternative was holding the test + back until the fixtures released, which would have put the new file in + front of nobody. Decided by @trexfeathers. The four failures are + `FileNotFoundError` on `Zarr/unicode_coords_v2.zarr` and go away with the + `IRIS_TEST_DATA_VERSION` bump — against a checkout that has the fixtures + the Zarr integration package is 86 passed **[verified]**. ### 12.5 Artefacts From 9d2a396c8a8c151ba3ddcbeff182f6db5e745260 Mon Sep 17 00:00:00 2001 From: Martin Yeo Date: Tue, 27 Oct 2026 12:26:14 +0000 Subject: [PATCH 16/18] Record the pull request PR 7 was raised as Co-Authored-By: Claude Opus 5 Session-Id: c18b0b04-01bf-4ba9-96b0-4d8c8b5cfa02 --- docs/superpowers/specs/2026-09-21-zarr-io-design.md | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/docs/superpowers/specs/2026-09-21-zarr-io-design.md b/docs/superpowers/specs/2026-09-21-zarr-io-design.md index 34bab1b5f9..882a27d406 100644 --- a/docs/superpowers/specs/2026-09-21-zarr-io-design.md +++ b/docs/superpowers/specs/2026-09-21-zarr-io-design.md @@ -9,8 +9,8 @@ | | | |---|---| | **Phase** | In delivery | -| **Progress** | PRs 1, 2, 2.5, 3, 4 and 5 merged into `brownfield`; PR 6 in review ([#7346](https://github.com/SciTools/iris/pull/7346)); PR 7 built on PR 6's branch and previewed at [trexfeathers/iris#101](https://github.com/trexfeathers/iris/pull/101); merge-back not started — see §12.1 | -| **Next action** | PR 6's review, then PR 7's rebase onto `brownfield`, then PR 8 — see §12.1 | +| **Progress** | PRs 1, 2, 2.5, 3, 4, 5 and 6 merged into `brownfield`; PR 7 rebased onto `brownfield` and in review ([#7349](https://github.com/SciTools/iris/pull/7349)); merge-back not started — see §12.1 | +| **Next action** | PR 7's review, then PR 8 — see §12.1 | | **Blocked on** | PR 7, whose unicode tests fail until [SciTools/iris-test-data#100](https://github.com/SciTools/iris-test-data/pull/100) merges and releases — deliberately committed red so the new test can be reviewed beside its fixtures | | **Branch** | `zarr-io-design` on `bjlittle/iris`, targeting `SciTools/iris:brownfield` | | **Delivery** | Nine pull requests into `brownfield`, then one merge-back into `main` — §5 | @@ -2313,8 +2313,8 @@ nothing else, and can land beside any of the others. | 3 | Relocate the CF loader | Merged | [#7316](https://github.com/SciTools/iris/pull/7316) | | 4 | Zarr loading | Merged | [#7336](https://github.com/SciTools/iris/pull/7336) | | 5 | Relocate the CF saver | Merged | [#7338](https://github.com/SciTools/iris/pull/7338) | -| 6 | Zarr saving | In review | [#7346](https://github.com/SciTools/iris/pull/7346) | -| 7 | Real-world test data and benchmarks | Built, awaiting PR 6 | [trexfeathers/iris#101](https://github.com/trexfeathers/iris/pull/101), a preview against PR 6's branch | +| 6 | Zarr saving | Merged | [#7346](https://github.com/SciTools/iris/pull/7346) | +| 7 | Real-world test data and benchmarks | In review | [#7349](https://github.com/SciTools/iris/pull/7349); the preview at [trexfeathers/iris#101](https://github.com/trexfeathers/iris/pull/101) is closed, and its review threads stay there | | 8 | The terms-of-art sweep | Not started | — | | — | **Merge back** `brownfield` → `main`: closing keywords, changelog fragments, docs | Not started | — | @@ -3288,8 +3288,8 @@ does not have. and a test asserts those strings so that regenerating the fixtures — which would destroy their only value — fails rather than passing quietly. - **That test is committed red**, against the usual rule that a branch stays - green. A reviewer cannot review a file that is not there, and the preview - pull request exists to be reviewed; the alternative was holding the test + green. A reviewer cannot review a file that is not there, and a pull + request exists to be reviewed; the alternative was holding the test back until the fixtures released, which would have put the new file in front of nobody. Decided by @trexfeathers. The four failures are `FileNotFoundError` on `Zarr/unicode_coords_v2.zarr` and go away with the @@ -3360,3 +3360,4 @@ raised as its own issue, not taken in a pull request that adds test data. | 2026-10-26 | PR 7 built: the NOAA GFS subset contributed to `iris-test-data`, eleven integration tests over it, a read-side test for a fixed-length unicode variable, `ZarrSave` and a `"Zarr"` file format in `LoadAndRealise`. The ESA EOPF fixture dropped on licence grounds and §5 and §7 amended to say so; §7 keeps the store as cited evidence. Six findings logged, four of them things the published store does that no synthetic fixture did. | | 2026-10-27 | [SciTools/iris-test-data#98](https://github.com/SciTools/iris-test-data/pull/98) merged and released as v2.31, so `IRIS_TEST_DATA_VERSION` is bumped in both workflow files and PR 7's integration tests run rather than skip. The indexer's store handling was rewritten on review by @pp-mo, from filtering a full walk to recursive descent; that also removed a bug where a checkout under any directory named `*.zarr` suppressed the whole index. | | 2026-10-27 | Actioned @pp-mo's review of PR 7. The custom skip guard removed in favour of the standard `skip_data`, which also honours `IRIS_TEST_NO_DATA`; the save benchmarks collapsed into one class parametrised over `file_format`; `TimeConstraint` given Zarr. His sharpest point was that a test writing a ` Date: Tue, 27 Oct 2026 12:35:20 +0000 Subject: [PATCH 17/18] Pin the test-data version carrying the unicode fixtures Co-Authored-By: Claude Opus 5 Session-Id: c18b0b04-01bf-4ba9-96b0-4d8c8b5cfa02 --- .github/workflows/benchmarks_run.yml | 2 +- .github/workflows/ci-tests.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/benchmarks_run.yml b/.github/workflows/benchmarks_run.yml index 949f3dcaef..6cf29848d5 100644 --- a/.github/workflows/benchmarks_run.yml +++ b/.github/workflows/benchmarks_run.yml @@ -73,7 +73,7 @@ jobs: env: IRIS_TEST_DATA_LOC_PATH: benchmarks IRIS_TEST_DATA_PATH: benchmarks/iris-test-data - IRIS_TEST_DATA_VERSION: "2.31" + IRIS_TEST_DATA_VERSION: "2.32" # Lets us manually bump the cache to rebuild ENV_CACHE_BUILD: "1" TEST_DATA_CACHE_BUILD: "2" diff --git a/.github/workflows/ci-tests.yml b/.github/workflows/ci-tests.yml index 97a3c88cb4..2e915912f9 100644 --- a/.github/workflows/ci-tests.yml +++ b/.github/workflows/ci-tests.yml @@ -54,7 +54,7 @@ jobs: env: # NOTE: IRIS_TEST_DATA_VERSION is also set in benchmarks_run.yml - IRIS_TEST_DATA_VERSION: "2.31" + IRIS_TEST_DATA_VERSION: "2.32" ENV_NAME: "ci-tests" steps: From 93eaede29b774640768482253dc9529c7c754628 Mon Sep 17 00:00:00 2001 From: Martin Yeo Date: Tue, 27 Oct 2026 12:35:26 +0000 Subject: [PATCH 18/18] Record the release that takes PR 7 green Co-Authored-By: Claude Opus 5 Session-Id: c18b0b04-01bf-4ba9-96b0-4d8c8b5cfa02 --- docs/superpowers/specs/2026-09-21-zarr-io-design.md | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/docs/superpowers/specs/2026-09-21-zarr-io-design.md b/docs/superpowers/specs/2026-09-21-zarr-io-design.md index 882a27d406..41198165d8 100644 --- a/docs/superpowers/specs/2026-09-21-zarr-io-design.md +++ b/docs/superpowers/specs/2026-09-21-zarr-io-design.md @@ -11,7 +11,7 @@ | **Phase** | In delivery | | **Progress** | PRs 1, 2, 2.5, 3, 4, 5 and 6 merged into `brownfield`; PR 7 rebased onto `brownfield` and in review ([#7349](https://github.com/SciTools/iris/pull/7349)); merge-back not started — see §12.1 | | **Next action** | PR 7's review, then PR 8 — see §12.1 | -| **Blocked on** | PR 7, whose unicode tests fail until [SciTools/iris-test-data#100](https://github.com/SciTools/iris-test-data/pull/100) merges and releases — deliberately committed red so the new test can be reviewed beside its fixtures | +| **Blocked on** | Nothing | | **Branch** | `zarr-io-design` on `bjlittle/iris`, targeting `SciTools/iris:brownfield` | | **Delivery** | Nine pull requests into `brownfield`, then one merge-back into `main` — §5 | | **Resolves** | SciTools/iris#6977, #6979, #6980 — at merge-back, not before | @@ -2324,7 +2324,7 @@ Companion work outside `SciTools/iris`: |---|---|---|---| | `SciTools/iris-test-data` | Cut-down NOAA GFS (v3) fixture for PR 7, and a `.zarr` directory indexed as one item | Merged, released as v2.31 | [#98](https://github.com/SciTools/iris-test-data/pull/98) | | `SciTools/iris-test-data` | The repository-wide licence statement, which #98's data does not fit | Raised, not for this programme to settle | [#99](https://github.com/SciTools/iris-test-data/issues/99) | -| `SciTools/iris-test-data` | Stored `