PREVIEW - Zarr I/O PR 7: real-world test data and benchmarks by trexfeathers · Pull Request #101 · trexfeathers/iris

trexfeathers · 2026-10-26T09:12:14Z

(posted by Claude Opus)

Adds the real-world test data and the ASV coverage that PR 7 of the Zarr I/O programme owes: a published NOAA GFS store loaded and saved by eleven integration tests, a Zarr save benchmark beside the netCDF one, and a Zarr file format in LoadAndRealise. If the integration tests are wrong, Iris ships believing it reads published Zarr when it does not. What I most want from a reviewer is a second opinion on dropping the EOPF fixture.

This is a preview. It targets zarr-io-pr-6 rather than brownfield, so that GitHub shows this branch's own diff while #7346 is still in review. It will be rebased onto brownfield and re-raised once SciTools#7346 merges.

Warning

CI is red on this branch on purpose, and will stay red until SciTools/iris-test-data#100 merges and releases. test_unicode_coords_sample.py reads two stores that only exist on that pull request's branch, so its four tests fail with FileNotFoundError on Zarr/unicode_coords_v2.zarr. Committed that way deliberately: the file cannot be reviewed if it is not here. Against a checkout that has the fixtures, the Zarr integration package is 86 passed. When #100 releases, IRIS_TEST_DATA_VERSION moves off 2.31 and the red goes away — nothing else changes.

The GFS data landed in SciTools/iris-test-data#98, released as v2.31, and both workflow files pin that version here, so its eleven tests run rather than skip. Locally against a v2.31 checkout: 82 passed in the Zarr integration package, none skipped; 12898 passed and 61 skipped across the whole suite, the same counts as before this branch.

Important

The published GFS store names its CRS variable in coordinates and carries no grid_mapping attribute, so Iris builds no coordinate system from it and cube.coord("latitude").coord_system is None. CF section 5.6 makes grid_mapping the way a data variable points at its grid mapping, so Iris is already right and this branch changes nothing. A fallback that read 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. The behaviour is pinned by test so that the absent coordinate system reads as the data's doing rather than as a regression.

What is here

The data. A 1.4 MB subset of the dynamical.org NOAA GFS forecast archive, Zarr version 3, contributed to iris-test-data by SciTools/iris-test-data#98. It keeps the sharding codec, the scalar spatial_ref variable, the two-dimensional valid_time coordinate and the base64 _FillValue encoding, because those are what it exists to exercise. That pull request also teaches the index generator to treat a store as one item rather than descending into its chunks.

The integration tests. lib/iris/tests/integration/zarr/test_gfs_sample.py. Loading, the coordinate structure, the attribute handling, a round trip, and xarray opening what Iris wrote. Four of them pin behaviour no synthetic fixture in the package reaches: the grid-mapping finding above; an attribute literally named dtype, which is one of the names that used to be shadowed on load; mode="w" replacing an existing store, which was the one of the three save modes nothing covered; and a round trip dropping the source shard layout, which nothing promises to keep and nothing warns about.

One unit test. Reading a fixed-length unicode variable, on both Zarr versions. Every other <U test in the package was on the write side.

The benchmarks. ZarrSave mirrors NetcdfSave over the same cubesphere cubes so the numbers are comparable, and LoadAndRealise grows a "Zarr" file format beside its "NetCDF" one.

Where I would look first

The _assert_clean_load fixture in test_gfs_sample.py escalates IrisLoadWarning to an error and asserts LOAD_PROBLEMS is empty. That is only correct because the published store happens to load silently; if a future Iris starts recording a problem for it, this module fails rather than quietly covering it, which is the intent, but it does make the module strict.

ZarrSave._save_data passes mode="w". Without it the benchmark raises FileExistsError on its second call, because ASV re-runs a timing function many times between setup() calls and the Zarr saver defaults to mode="w-".

test_gfs_sample.py carries a module pytestmark guarding on the store's own existence. _shared_utils.skip_data asks whether iris-test-data is present at all and never whether the file is, so without the guard these eleven tests failed rather than skipped on any checkout older than v2.31 — I had that backwards until the whole-branch review demonstrated it. The guard stays now the version is pinned, because Iris and iris-test-data are versioned separately and that window reopens whenever one moves first.

What I rejected

The ESA EOPF version 2 fixture, which the design document promised. Its licence is a bespoke Copernicus instrument rather than a standard one, and having it read by someone who handles Copernicus terms did not fit the schedule. Before dropping it I checked what it would have covered: the raise when a root group holds no data variables, the version 2 fill-value split, and nested JSON attributes are all already pinned by stores built in tmp_path. The one gap was reading a fixed-length unicode variable, which is the unit test above. The store stays in the design document as cited evidence for the rules it motivated; it goes only as a shipped fixture.

Replacing the synthetic GFS-like fixture with the real one, which fixtures.py said would happen. The synthetic store needs no external data, so it 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 the grid_mapping attribute the published store lacks. The docstring is corrected rather than the fixture deleted.

Amending README.rst in iris-test-data. Its blanket Crown copyright claim has been inaccurate since a CC BY-SA 4.0 CMIP6 file landed there in 2019. That is a SciTools decision, raised as SciTools/iris-test-data#99; this work records provenance per directory instead.

No changelog fragment: per the design document, fragments are written once at merge-back.

pp-mo

A few points to consider, but otherwise basically OK I think.
I've checked that the skipped test works with the latest iris-test-data, but I haven't exercised the benchmarks : If you update the IRIS_TEST_DATA_VERSION, we can then check the benchmarks on the PR too.

* Plan PR 6, stage 1: framing, constraints and file structure The parts of the plan that every task reads: what PR 6 delivers, the constraints copied verbatim from the design document, the scope boundary, how a branch over two unmerged pull requests gets rebased, nine findings that refine the spec, the file structure and the interface the tasks leave behind. Four decisions taken with the user and recorded as findings: grid-mapping writing gets a fifth hook rather than the regularisation Q8 defers (F2); _attribute_handlers moves here, in its own commit (F3); the write-side value conversions get their own module (F4); xarray goes in the requirement files but the lock files are regenerated by hand later (F5). Every zarr-python and xarray claim the write path leans on was re-checked against zarr 3.4.1 and xarray 2026.9.0 and reproduces (F6). Tasks follow in later stages. Review Focus is written last, when there are tasks to pin each line to. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: b8474d78-fdfa-43e5-b907-4bc3fe833fb3 * Plan PR 6, stage 2: the two changes to generic code Task 1 moves the CF attribute handlers out of the netCDF package, which is what §12.4 assigns to this pull request and what closes cf/saver.py's last two-way import. Task 2 adds the fifth saver hook, so that the 63 grid-mapping parameters reach a store with no netCDF4 object behind it. Both are prerequisites for any Zarr saving at all, and neither mentions Zarr, so they lead and they commit separately. The grid-mapping hook keeps netCDF writing to the same object it writes to today, by returning it. That is what lets this pull request leave the encoding question where Q8 put it, in its own pull request, rather than changing what lands in a netCDF file as a side effect of adding Zarr. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: b8474d78-fdfa-43e5-b907-4bc3fe833fb3 * Plan PR 6, stage 3: four more findings, and the encoding module Task 3 adds zarr/_encode.py: JSON-native attribute values, the _FillValue and missing_value encodings, the default fill table and masked-array filling. Pure functions, so the task tests them on plain values with no store in sight. Four findings behind it, each checked against zarr 3.4.1 and xarray 2026.9.0 rather than assumed: - missing_value must be a plain number. Written as base64, like _FillValue, xarray opens the store and masks nothing (F10). - zarr's default fill value is 0 for every dtype, floats included, so a masked cube saved with no fill_value would write zeros and declare nothing. Iris writes netCDF's default fills instead, and always declares them as _FillValue (F11). Decided with the user. - create_variable never sees the data, so the spec's "derive the write grid from the source's chunking" cannot happen there. The invariant is enforced by rechunking at write time instead, with the data variable's chunks defaulted from the cube (F12). - A masked array written straight to a zarr array raises inside numpy.ma, so filling is not an optimisation (F13). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: b8474d78-fdfa-43e5-b907-4bc3fe833fb3 * Plan PR 6, stage 4: opening for writing, and creating arrays Task 4 opens the four write modes, defends the w- default that makes a clobber deliberate, and puts the version 3 guard in one place. Task 5 turns the two creation calls CFSaver makes into Zarr arrays: a declared dimension is a dict entry, an array carries its dimension names, and a name that already exists is refused rather than overwritten. The keyword table is here too. A netCDF storage keyword raises and names the Zarr replacement where there is one, and says there is none where there is not - which is what stops the translation layer the spec rules out from growing back by accident. The fill value a create gets when the caller names none is netCDFs default for the dtype, and no _FillValue attribute goes with it. Who declares the attribute, and when, is Task 11: it has to happen before the metadata is consolidated, and only the saver knows the data is masked. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: b8474d78-fdfa-43e5-b907-4bc3fe833fb3 * Plan PR 6, stage 5: writing values, and the saver itself Task 6 puts each kind of write at the one place it passes through: attributes are converted before zarr sees them, because a value its encoder rejects stays in the mapping; masked values are filled on the way into an array; and finalise consolidates the whole store from the root, with zarr's "not part of the version 3 specification" warning suppressed and explained. Task 7 is the saver. It is thin, which is the point of PR 5: opening a store, one hook saying Zarr holds text natively and needs no trailing character dimension, and the save() users call. Storage keywords are validated before the store is created, so a mistyped one does not leave an empty store behind. Deferred writes, the alignment invariant and the masked _FillValue declaration are Task 8, on the same class. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: b8474d78-fdfa-43e5-b907-4bc3fe833fb3 * Plan PR 6, stage 6: deferred writes, and reaching iris.save Task 8 is the one where a mistake loses data rather than raising. The _FillValue a masked source needs is declared in _backend_write, before the metadata is consolidated, because under compute=False the data is written long after. The lazy source is rechunked to the array's write grid - shards when sharded, chunks otherwise - which is what licenses lock=False, and the task carries the §4.5 reproduction as a test. delayed_completion returns a real Delayed that owns the consolidation, and binds the dataset locally rather than through self, because the closure is serialised to every distributed worker. Task 9 registers the saver. The entry is a shim in the zarr package rather than the saver function itself: registration happens the first time any format is saved, and reaching the real save would import zarr-python with it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: b8474d78-fdfa-43e5-b907-4bc3fe833fb3 * Plan PR 6, stage 7: round trips, cross-reader conformance and docs Task 10 saves realistic_4d_w_everything and loads it back, and pins the two format behaviours Iris cannot fix: a deleted chunk reads as fill with no warning, and consolidated metadata goes stale silently. Both fail loudly here if a zarr-python release changes them. Task 11 is the one that justifies two design decisions rather than merely implementing them - xarray opens every store Iris writes, and Iris loads what xarray wrote. It adds xarray to the three requirement files and deliberately leaves the lock files alone. Task 12 replaces the Saving section PR 4 wrote, whose whole content is that Iris cannot save to Zarr. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: b8474d78-fdfa-43e5-b907-4bc3fe833fb3 * Plan PR 6, stage 8: review focus, the record, and self-review The Review Focus section is written last, with the spec in front of me and the tasks already down: five input classes a user meets in ordinary work that no task's own tests would have reached - a mesh cube, packed data, int64, a non-ASCII attribute and a dataless cube. Each got a test added to the task that owns the code, rather than being noted and left. The self-review found one real error: Task 10's fixture called realistic_4d_w_everything(lazy=True), and the function's only parameter is w_mesh. Fixed, and the mesh case now uses it deliberately. Task 13 records the seven decisions in the design document, says what PR 6 does and does not close of Q5 and Q8, and sets out what the pull request body has to say - above all that it is stacked on two unmerged branches and that CI has no xarray until the lock files are regenerated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: b8474d78-fdfa-43e5-b907-4bc3fe833fb3 * Plan PR 6: create the cf/saver test package in Task 2 There is no tests/unit/fileformats/cf/saver/ yet - PR 5 left the savers tests under netcdf/saver/, driving the generic machinery through the only subclass there was. Task 2 adds the first test that belongs to CFSaver itself, so it has to create the package marker too. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: b8474d78-fdfa-43e5-b907-4bc3fe833fb3 * Move the CF attribute handlers into the CF package cf/saver.py imported ATTRIBUTE_HANDLERS from the netCDF package, which in turn imported cf/saver.py, surviving only because _attribute_handlers itself imported nothing from iris.fileformats.cf. The handlers serialise the Iris-only attribute types that every CF store needs - STASH, GRIB_PARAM, ukmo__process_flags - so the CF package is their natural home, and moving them there closes that two-way import. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 6582234f-93d1-42d0-94ce-b54ee4a252af * Let grid-mapping parameters reach a store that is not netCDF _add_grid_mapping_to_dataset bound grid_variable to cf_var_grid.variable and made sixty-three plain attribute assignments to it. That object is netCDF4's own, so any store with no netCDF4 variable behind it raised a bare AttributeError for every cube with a coordinate system. Routing those assignments through .attributes instead would change how some of them are typed in a netCDF file (crs_wkt as NC_STRING against grid_mapping_name as NC_CHAR), which Q8 of the design document rules out of this programme. So the fix is a fifth hook, _grid_mapping_writer, that returns the assignment target: the base implementation returns a small writer onto cf_var.attributes, and netCDF's override returns cf_var.variable unchanged, so the netCDF path writes to the identical object it always has. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 6582234f-93d1-42d0-94ce-b54ee4a252af * Add the Zarr write-side value conversions The native Zarr saver needs a few value conversions before anything can reach a store: attributes into JSON-native types, `_FillValue` and `missing_value` into the encodings xarray expects, a default fill value per dtype, and masked arrays into filled ones. Keeping these as pure functions, mirroring `_decode.py`, lets every rule be tested without a store, ahead of the dataset and saver code that will call them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 6582234f-93d1-42d0-94ce-b54ee4a252af * Open a Zarr store for writing ZarrDataset.__init__ accepted mode="r" only. This adds the four write modes zarr-python itself supports, with Iris's own default: "w-" creates and refuses to clobber an existing store, because a Zarr clobber is many non-atomic deletes rather than one atomic rename, so replacing one is opted into via mode="w" instead of assumed. Writing is guarded to version 3 in one place, in _WRITABLE_ZARR_FORMAT, so that version 2 writing can be added later by implementing the three version-specific points of the design document and deleting that guard. Reading keeps working unchanged for either version. _declared_dimensions is added as empty state for the next task to fill, and the old _NO_SAVING constant is retired along with _READ_MODES in favour of _WRITE_MODES and _MODES; the three still unimplemented write members now raise their message inline. An existing test asserted that mode="w" was refused outright; that is superseded by this change and is removed, with its replacement now covered in the new write test module. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 6582234f-93d1-42d0-94ce-b54ee4a252af * Close the code fence that swallowed half the plan A missing closing fence after the _dimension_size helper left every heading from Task 6 onward inside a code block, so the three paragraphs that introduce check_encoding_keywords rendered as Python and the tool that extracts one task's text for an implementer could not find where Task 5 ends. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 6582234f-93d1-42d0-94ce-b54ee4a252af * Declare Zarr dimensions, and create Zarr arrays Zarr has no dimension object, so create_dimension records a name and size and catches a conflicting redeclaration itself, rather than letting two arrays silently disagree. create_variable then writes those names onto a new array, refuses a Zarr storage keyword it does not recognise rather than guessing a netCDF translation, and applies the dtype-dependent _FillValue encoding to both the array's own fill and its attribute. The pre-existing write-surface test covered three members that always refused; two of them now work on a writable dataset, so it is narrowed to the one still unimplemented, finalise, with the read-only refusal of the other two covered by the new tests instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 6582234f-93d1-42d0-94ce-b54ee4a252af * Let a string variable default its own fill value create_variable always looked a default fill up in the netCDF table when none was given, but dtype=str has no entry there - it asks Zarr for a variable-length string array, which is not a netCDF concept to have a default for. Skip the lookup for that case and omit the fill_value keyword entirely, so zarr-python's own default for the dtype applies, matching what the docstring already promised. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 6582234f-93d1-42d0-94ce-b54ee4a252af * Write Zarr values, attributes and consolidated metadata Attributes now convert to a JSON-native form before zarr sees them, on every assignment rather than only at variable creation, so a rejected value never lands in the attributes mapping. _FillValue and missing_value get their own CF encodings, decided by the array's dtype. A write now fills any masked array with its own fill value first, since a Zarr array has no mask of its own and numpy.ma raises deep inside the bytes codec otherwise. finalise() consolidates the whole store's metadata, the step that makes a remote store usable to a reader that fetches one document instead of one per node. The _FillValue pre-encoding in create_variable was a stopgap for when the attributes mapping did not yet encode anything; it is removed now that __setitem__ does the encoding itself, so a fill value is no longer at risk of being encoded twice. test_finalise_refuses pinned the old NotImplementedError; it is superseded by TestFinalise::test_a_read_only_dataset_refuses, which pins the real refusal once finalise does its job. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 6582234f-93d1-42d0-94ce-b54ee4a252af * Add the Zarr saver, and iris.fileformats.zarr.save CFSaver already does the CF work; the Zarr-specific saver only opens a store, says how a string is stored, provides the save() entry point that validates Zarr's own encoding keywords before anything is created, and completes the store with a consolidated metadata document. delayed_completion() wraps the inherited lazy-write delayed with a real Delayed that also runs the consolidation, so both the eager (compute=True) and deferred (compute=False) paths finish through the same mechanism, and a save that never completes never claims to have consolidated. Also refuse, rather than silently rename, a cube whose data-variable name already exists in the group when adding to an existing store: the generic CFSaver renaming is right for two cubes of one call sharing a name, and wrong for a store this call did not just create. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 6582234f-93d1-42d0-94ce-b54ee4a252af * Fix four review findings in the Zarr saver mode="a" onto a store this saver had already consolidated raised a bare KeyError: ZarrDataset.__init__ opened it with zarr-python's default consolidated-metadata fast path, so the reopened group listed only the arrays present at the last consolidation, never the one the new write was about to add. Pass use_consolidated=False for every write mode; a read mode keeps the fast path, which is what it is for. Saver.write()'s own repeated-name refusal read self._dataset.variables live, which create_variable invalidates on every call, so it also caught a name created moments earlier by another cube of the same save() call - exactly the case its own docstring says must still be renamed. Compare against a snapshot taken once, when the Saver opens the dataset, instead. The refusal's message was duplicated verbatim between saver.py and _dataset.py. Shared it as existing_name_message() in _dataset.py. A scalar cube failed to save: CFSaver writes a whole variable as cf_var[:] = data, and zarr rejects slice(None) on a 0-d array. Zarr array indexing wants Ellipsis or () instead; ZarrDatasetVariable .__setitem__ now makes that substitution for a 0-d array. Corrected delayed_completion's docstring, which claimed the inherited method returns a tuple for a sequence of sources: for one delayed write it returns a plain dask.array.core.Array, never a tuple and never actually a Delayed despite the annotation. The #7291 reference and the reason for wrapping it both still hold. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 6582234f-93d1-42d0-94ce-b54ee4a252af * Defer Zarr writes, aligned to the write grid A lazy source handed straight to da.store(..., lock=False) can have its dask chunking subdivide the array's write grid - its shards when sharded, its chunks otherwise - so several concurrent tasks read- modify-write one stored object and lose each other's updates. Rechunk the source to the write grid first, so each task owns whole cells and no cell is touched twice. Declare a _FillValue when the saved data is masked, even when the cube's own attributes name none, so a reader sees the mask rather than plain sentinel values with nothing marking them missing. Reshape delayed_completion's inner function into a module-level one so the Delayed it returns survives a plain pickle round trip, not only cloudpickle, which a distributed scheduler needs regardless. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 6582234f-93d1-42d0-94ce-b54ee4a252af * Fix review findings in the Zarr saver's write alignment and masking Drop the two finding labels from shipped test comments, and correct one that described the netCDF compute=False path wrongly - it returns a dask Array for one delayed write, not always a tuple. Make _tiles_the_grid raise on a chunks/grid rank mismatch rather than silently answering True for dimensions it never examined: the one function whose whole job is to prevent a silent, mismatched write must not default to "safe" for input it did not check. Use np.ma.is_masked instead of np.ma.isMaskedArray to decide whether real data needs a declared fill value: the latter is true of an np.ma array with an all-False mask, which Iris produces routinely, and would invent a missing-value declaration for data whose source never carried one. Lazy data keeps is_lazy_masked_data, since it cannot be inspected without computing it. Rework the "aligned source is left alone" test: chunking it at exactly the write-grid size made the old chunks-equality assertion pass whether or not the early return existed, because dask's own rechunk() is already a no-op in that case. A single oversized chunk forces dask's rechunk to subdivide if asked, so identity is now a check that can fail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 6582234f-93d1-42d0-94ce-b54ee4a252af * Reach the Zarr saver from iris.save An earlier pull request taught find_saver to match the .zarr suffix, but iris.save still had no saver registered for it, so the lookup returned None. Register the zarr package's _save_cubes shim for the extension, keeping the import of zarr-python deferred until a Zarr target is actually saved. The shim's __module__ is "iris.fileformats.zarr", the package, not "iris.fileformats.zarr.saver": iris.save reads __module__ to decide whether a saver takes a whole CubeList at once, so that is the string the frozenset needs, not the module the shim forwards to. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 6582234f-93d1-42d0-94ce-b54ee4a252af * Check the zarr version floor in the save path too _save_cubes reaches iris.fileformats.zarr.saver with a plain from .saver import save, which bypasses __getattr__ and the _check_zarr_version call it makes on every load. On an installation with zarr missing or too old, iris.load already explained the problem clearly, but iris.save silently wrote a store (when zarr was merely too old) or leaked a bare "No module named 'zarr'" (when it was entirely absent). Repeat the same checks __getattr__ runs, so all three routes into this package give the same clear error. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 6582234f-93d1-42d0-94ce-b54ee4a252af * Test the Zarr save round trip, and what the format does not promise Everything so far has tested one piece of the Zarr saver in isolation. This is the first test that puts a realistic cube with meshes, packed data, int64 data, non-ASCII attributes and a dataless form through iris.save and iris.load together, and checks what comes back - and pins the two Zarr format behaviours Iris depends on but does not control: a deleted chunk reads back as fill with no warning, and consolidated metadata goes stale until Iris re-consolidates. Two of the five ordinary-input cases fail and are left failing on purpose: packed data round-trips to the wrong values, because nothing in the Zarr saver applies the scale_factor/add_offset transform before writing (netCDF relies on netCDF4-python doing this automatically, and Zarr has no equivalent); and a mesh does not reattach on load, because the Zarr loader (an earlier, already-merged piece of work) does not build MeshXY objects, and iris.mesh.load_meshes explicitly ignores non-netCDF sources. Both are real defects the tests found rather than authoring mistakes, so they are reported rather than papered over. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 6582234f-93d1-42d0-94ce-b54ee4a252af * Apply CF packing before writing to a Zarr array The generic CF saver only ever sets scale_factor/add_offset as attributes and streams the unpacked values straight through, because netCDF4-python applies the pack transform itself on assignment once those attributes are set on the variable. Zarr has no such mechanism, so a packed Zarr variable was silently storing the original values rounded and cast into the narrow dtype, unrelated to what the scale_factor/add_offset attributes claimed - correct metadata over wrong data, with no warning on either the write or the read side. ZarrDatasetVariable.__setitem__ is the one place every write passes through, eager and lazy alike, so that is where the pack transform now runs: round((value - add_offset) / scale_factor), narrowed to the array's own dtype, with either attribute absent behaving as its identity. It runs before the existing mask fill rather than after, because the fill value lives in packed (stored) space and the data arrives unpacked - filling first would push an already-filled sentinel through the same transform as real data and lose it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 6582234f-93d1-42d0-94ce-b54ee4a252af * Split the mesh round trip into what saving proves and what loading cannot yet The Zarr saver writes a complete, CF-UGRID-compliant mesh - a topology variable, a connectivity with its start_index, and data-variable mesh/location attributes - confirmed by reading the store directly. Loading one back does not reattach it: the Zarr loader (already-merged, out of this pull request's scope) never builds a MeshXY, and iris.mesh.load_meshes filters to the netCDF handler only, so neither public entry point recovers a mesh from a store this saver wrote. Replaces the single round-trip test with two: one against the written store, asserting the structure the saver actually delivers, and one strict xfail for the full iris.save/iris.load round trip, so that loader support landing later is caught rather than left silently passing under a stale marker. Also tightens test_several_cubes_round_trip to assert the exact count of three cubes (the two saved plus the orography cube the cube fixture's hybrid-height factory always brings along) and that both saved names are present, rather than a bare count that said nothing about which cubes they were. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 6582234f-93d1-42d0-94ce-b54ee4a252af * Refuse non-integer packing, and make pack() exactly netCDF4's transform A non-integer packing dtype reached the store silently mishandled: nothing validated the packspec before writing, and pack()'s unconditional rounding mangled a float target instead of merely narrowing it. The netCDF saver already refuses the same input outright, so the Zarr saver now does too, with the same message, checked before the store is created. pack() itself now rounds only for an integer target, matching what netCDF4-python actually does on assignment rather than a rounding version of it applied regardless. Also closes two smaller gaps in the same function: scale_factor=0 now raises naming it, rather than dividing by zero in silence, and the final cast is reconstructed from a block's underlying data and mask rather than calling MaskedArray.astype() directly, which otherwise tries to cast the block's inherited (often very large) floating fill_value into the narrow target dtype as well, and numpy warns about that even though every real value casts cleanly - a warning a lazy masked packed write would otherwise have carried all the way out to the user with no connection to anything they wrote. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 6582234f-93d1-42d0-94ce-b54ee4a252af * Make three round-trip tests able to fail for what they claim to test test_a_missing_chunk_reads_back_filled asserted only the shape, which is unchanged whether a missing chunk reads back in silence or with a warning - so it could not have caught a future zarr-python release that started warning about exactly that. It now also asserts no warning was raised, and that the masked count went up against the original cube's, which is the actual "data silently went missing" the test exists to record. test_a_group_round_trips saved the same cube to the root and to a named group, so reading the root would have satisfied every one of its assertions - a loader that ignored group= entirely would have passed. It now saves a differently-named cube into the group and asserts that name comes back, so ignoring group= raises instead. test_packed_data_round_trips checked only that scale_factor was present, not add_offset, and compared at a tolerance loose enough that a systematic one-count bias would have passed. It now checks both attributes and tightens the tolerance to one stored count - loose enough for the quantisation error correct packing is allowed to introduce, tight enough to fail a one-count bias. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 6582234f-93d1-42d0-94ce-b54ee4a252af * Check that xarray reads what Iris writes, and Iris reads xarray's Every Zarr test so far has checked Iris against Iris. This checks it against the rest of the world: xarray opens every store Iris writes, in both directions. It is the only thing that can catch a swap of the base64 _FillValue and the JSON-native missing_value encodings, since Iris's own reader accepts either shape for either attribute. xarray goes into the test section of each Python version's environment file, not the lock files: the human developer regenerates those by hand once the Zarr dependency PR has landed, so until then continuous integration has no xarray and these tests skip there. They run locally, which is where they are needed while the work is being done. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 6582234f-93d1-42d0-94ce-b54ee4a252af * Prove the missing_value half of the same encoding decision The cross-reader module proved that a floating _FillValue has to be base64, because xarray fails the whole dataset open on a plain number. It did not yet prove the other half of the same pair: that missing_value has to stay a plain number, because base64 there opens cleanly and silently masks nothing. A unit test on the encoder alone cannot see that failure, since it only checks what the function returns, not what a reader does with it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 6582234f-93d1-42d0-94ce-b54ee4a252af * Document saving to Zarr The Saving section of the Zarr I/O page said Iris could not do this; it now can, so the section needed replacing rather than touching up. Covers modes, storage keywords, deferred saving, consolidated metadata, the coordinate-renaming behaviour of mode="a", and the mesh save/load asymmetry. Extends the Missing data section with the save-side _FillValue behaviour, and adds the S3 saving example the loading-only text was missing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 6582234f-93d1-42d0-94ce-b54ee4a252af * Fix Zarr saving docs: a working mode example, scoped collision rules, and group= for saving The mode="w"/"a" example replayed the same cubes at the same path, so mode="a" raised instead of adding, the opposite of what it showed. The same reuse broke the Storage keywords/Deferred saving pair the same way, caught while re-running the section end to end. Fixed both by giving each block its own cube or path. The two collision paragraphs were both true but unscoped, so a reader could not tell an error on the cube's own data variable from the generic saver's silent rename of everything else. Named each paragraph's own scope. group= saves to a store sub-path, same as it does for loading, but was undocumented on the saving side; added it next to the existing loading text in the Groups section. Also added the order keyword, missing from the Storage keywords list. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 6582234f-93d1-42d0-94ce-b54ee4a252af * Mention native Zarr saving in iris.save's own docstring iris.save's docstring enumerated netCDF, GRIB2, PP and Zarr-via- NcZarr as the four formats it saves, with no mention of writing a native Zarr store at all - the first thing help(iris.save) and the API reference show was silently wrong about this work's headline capability. Native Zarr is dispatched by filename extension exactly like netCDF, GRIB2 and PP, confirmed in _check_init_savers(), so it joins that group rather than the NcZarr entry, which is a different code path reached by URL fragment and stays as it was. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 6582234f-93d1-42d0-94ce-b54ee4a252af * Record Zarr saving in the design document PR 6 is built and ready to draft as a pull request, so the design document's progress record needs to describe what the branch actually does before that pull request opens. Fourteen decisions are logged in §12.4: the seven foreseen in planning, and seven more settled during the work itself, covering packing on the Zarr write path, saver registration and the version-floor check on save, the mesh round-trip gap, coordinate collisions under mode="a", a masked string variable's fill value, and the deferred-completion function's picklability. Q5 and Q8 in §12.3 gain their Zarr-side answers, and §12.1 and §12.6 record the branch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 6582234f-93d1-42d0-94ce-b54ee4a252af * Fix three errors in the Zarr saving record Review of the previous commit found the record describing itself wrongly in three places. §12.1 called PR 6 "Drafted", which undersells a branch that is fully built and passing, with no pull request opened yet; the document's own legend and its PR 4 precedent both call that state "Built, awaiting PR N", so PR 6's row now matches, naming the branch and what it sits on. §12.6 said "Fourteen decisions logged" where §12.4's new entry holds fifteen bullets, an off-by-one that defeated the one thing that line exists to let a reader check. And §12.4 claimed zarr/_encode.py had been added to §4.1's module layout, which had not actually happened; §4.1 now lists it, so the claim is true rather than aspirational. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 6582234f-93d1-42d0-94ce-b54ee4a252af * Stop mode="a" truncating a cube whose dimension name collides A cube dimension with no dim coord is named "dim0", "dim1", and so on, whatever cube it came from, so two unrelated cubes routinely claim the same name. The generic saver skips declaring a dimension it already sees in the store, so under mode="a" a name the store already carries was never declared, and create_variable sized the new array from the store's length rather than the cube's. create_variable never sees the data, so nothing noticed, and zarr-python truncated the oversized write in silence; the other order raised a bare numpy broadcasting message naming neither cube nor array. Seeding the generic saver's _existing_dim from the group's own dimensions puts those names where _get_dim_names can see them. Its disambiguation loops then increment "dim0" to "dim0_0" for a conflicting length and reuse it for a matching one, which is what the design already promises for coordinates. Under "w-" and "w" the group is new, so the seed is empty and nothing moves. ZarrDatasetVariable.__setitem__ now also refuses a value whose shape does not match the selection, naming the array. Zarr neither resizes an array nor reports an oversized write, so the whole class of defect is worth turning from silent truncation into a loud failure. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 6582234f-93d1-42d0-94ce-b54ee4a252af * Share the cube-attribute marshalling with the netCDF saver A store holds one value of a global attribute, and a list of cubes may carry several. Settling that is cube logic, not storage logic, but it was written inside netcdf.saver.save and the Zarr saver did not have it, writing only Conventions. So every cube global attribute was dropped on the Zarr path under split-attribute saving, where CFSaver.write deliberately leaves them to the caller: a cube with title and institution saved to netCDF with both and to Zarr with neither. Under the legacy default, CFSaver.write writes each cube's globals as it goes, and nothing computed the local_keys that keep disagreeing cubes apart, so a CubeList collapsed to the last cube's value where netCDF kept both on their own data variables. The block moves to cf.saver.marshal_attributes, which both savers now call. Its only change is that the legacy mode's deprecation message names its format, so the netCDF text is exactly what it was. The Zarr path now emits that deprecation too, which it should: the legacy mode is deprecated for every format, and saying so only for netCDF left Zarr users on it without a word. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 6582234f-93d1-42d0-94ce-b54ee4a252af * Accept the attribute and dtype forms netCDF already accepts Two inputs an ordinary cube carries were refused on the Zarr path and saved on the netCDF one. json_native converted a numpy scalar and a numpy array but passed a list or tuple through unexamined, so json.dumps met the numpy inside it. The message then told the user to supply a list when they had supplied one. It now recurses into a sequence, converting each element, which is what the "a numpy array becomes a list" rule already implied. default_fill_value raised for any dtype outside netCDF's default-fill table, and create_variable consulted it whether or not the data was masked - so a float16 cube could not be saved at all, and an S1 one was refused where netCDF saves it, both with a message blaming a mask that was not there. The table is netCDF's, and ZarrDataset.supports_dtype answers True for every numpy dtype, so a store that refused what it said it could hold contradicted itself. The function now returns None for a dtype it has no entry for, the array takes zarr-python's own default, and the refusal moves to _backend_write, which is the one place that knows the data is masked and can say so truthfully. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 6582234f-93d1-42d0-94ce-b54ee4a252af * Refuse the keywords that did nothing, and say what is true Seven small corrections found by the whole-branch review, all in the seam between the generic saver and the Zarr one. order is in zarr-python's own signature but has no effect on a version 3 array, which is all Iris writes; zarr-python warns and ignores it. It is dropped from the accepted set and given a message saying why, because a keyword Iris accepts and then discards is worse than one it refuses. append gets the message the design document already supplies: there is no append, and mode="a" adds new arrays. chunks no longer defaults to the cube's own lazy chunking when shards is given. A shard holds a whole number of inner chunks, and Dask's chunking has no reason to divide one the user chose for storage, so the pair failed over a number the user never passed. The Zarr documentation page also now says that the default applies to lazy data alone, where before it claimed data is always written in the shape it is held in. The missing-dependency message said "Unable to load a Zarr store" and iris.save raises it too, so it says "use". The consolidated-metadata paragraph said Iris rewrites a document it finds, where declining consolidation removes it; that is the safe outcome and now the one described. An unused pathlib import goes, since the Pyflakes rules are in the ignore list and nothing else would catch it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 6582234f-93d1-42d0-94ce-b54ee4a252af * Record the whole-branch review fixes in the design document Seven findings, all reproduced and all in the seam between the generic saver and the Zarr one, go into the progress record in §12.4, with the row in §12.6 that points at them. Two statements in the PR 6 entry are corrected where these fixes moved them: the default fill now covers the dtypes netCDF's table knows and refuses the rest only when the data is masked, and the chunks default applies to a lazy cube with no shards given. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 6582234f-93d1-42d0-94ce-b54ee4a252af * Say why a masked string cannot be stored, and record it Folding the string carve-out into the general fill-value rule meant a string variable with any masked point reached the new refusal, where before it wrote _FillValue: ''. The refusal is right and stays: the only sentinel a Zarr string array could carry is the empty string, which is also a legitimate label, and zarr/_decode.py does not restore a mask from one - it warns and reads the empty strings back as data. Writing it lost the mask in silence. The netCDF saver cannot store a masked string coordinate either. What was wrong was the message. It named StringDType() and advised an explicit fill_value, which iris.save offers for the data variable alone - and the array that fails here is a string coordinate. A text variable now gets its own sentence, naming the variable and saying that a string has no sentinel, with no advice that cannot be followed. The numeric case keeps the fill_value advice, which does work there. The design document recorded the old behaviour, and now records this one and the reason for it. A test covers the coordinate case, whose absence is what let the change through. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 6582234f-93d1-42d0-94ce-b54ee4a252af * Default the Zarr version to None, and say what each mode does Made `zarr_format` default to None rather than 3, matching `NetCDFDataset.__init__`'s `netcdf_format=None`: a write resolves None to `_DEFAULT_ZARR_FORMAT`, and a read drops whatever it was given. Reading had always ignored the value, so a default of 3 was a number the caller could not act on. A read drops it rather than passing it on because zarr-python does not ignore `zarr_format` on a read, it constrains detection: against zarr-python 3.4.1, opening a version 2 store while asking for version 3 raises GroupNotFoundError, which names the wrong problem. Pinned that in a test. Turned `_WRITABLE_ZARR_FORMAT` into the tuple `_WRITABLE_ZARR_FORMATS`, so the refusal can list what Iris does write, and added `_DEFAULT_ZARR_FORMAT` alongside it rather than reading the default out of the tuple's order. Covered `mode="r+"` writing into an existing store, which had only been tested through the errors it raises. Listed NetCDFDataset's open modes in its docstring, which named none of them, and said what ZarrDataset's "r+" does. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 02169076-de34-48ad-9108-3ec66c3890f3 * Drop the parameter count from the grid-mapping prose A count of call sites is exactly what .github/WRITING.md says never to cite: nothing checks it, and it goes stale the moment a CF grid-mapping parameter is added or removed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 22ea9714-39f5-4300-94ab-9c88882e2367 * Write grid-mapping parameters through a function, not a proxy `grid_variable.false_easting = cs.false_easting` read as an attribute set on a variable. It was not: it was a dispatch whose destination depends on the store, and the statement hid that completely. Every parameter now goes through `gm_write_param(name, value)`, so the store-dependent step is visible where it happens. This removes `_GridMappingAttributeWriter`, whose `__setattr__` caught every assignment, so a line meant as a local would have become a CF attribute in the file with nothing to catch it. Nothing in any store changes. netCDF's writer still sets the attribute on the netCDF4 variable, which is the same operation the statements performed, so `crs_wkt` stays NC_STRING and `grid_mapping_name` stays NC_CHAR. Verified by saving every coordinate system the method handles, to netCDF and to Zarr, with extended grid mapping on and off, and comparing each parameter and its type against the same capture taken before the change. The call sites now pass a name and a value, which is the shape Q8's regularisation needs, reducing that work to deleting netCDF's override. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 22ea9714-39f5-4300-94ab-9c88882e2367 * Reach the Zarr saver through an iris.io wrapper, like every other format `zarr._save_cubes` called `_check_zarr_version()` and rewrote ImportError by hand. It only had to because it imported `from .saver import save`, which reaches the submodule directly and so skips the package's `__getattr__`. Importing `save` from the package instead runs that `__getattr__`, and with it the version check and the error wrapping, so the shim had nothing left to do. What remains is `iris.io._zarr_save`, a three-line deferred-import wrapper beside `_dot_save` and `_grib_save`, which is where this project already puts them. Registering the savers no longer imports the `iris.fileformats.zarr` package at all, where it used to. `_CUBELIST_SAVER_MODULES` becomes `_CUBELIST_SAVERS` and names savers rather than modules. It could not stay keyed on the module: `_zarr_save` shares `iris.io` with `_dot_save`, which must keep being handed one cube at a time, and naming the module would have silently fed it a whole CubeList instead. A test now pins that. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 22ea9714-39f5-4300-94ab-9c88882e2367 * Recognise the Zarr saver when a caller supplies it directly A caller reaches the same save two ways: `iris.save(cubes, "out.zarr")` dispatches to the `_zarr_save` wrapper, and `iris.save(cubes, target, saver=iris.fileformats.zarr.save)` hands over the saver itself. Only the first was recognised as taking a whole CubeList, so the second looped over the cubes and the second cube met a store that mode="w-" refuses. The second route never worked. The entry it needed named the package, `iris.fileformats.zarr`, while the saver's `__module__` is `iris.fileformats.zarr.saver`, so it has never matched. Naming both the wrapper and the saver fixes it. Entries stay strings rather than the functions themselves: reading this set must not import zarr. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 22ea9714-39f5-4300-94ab-9c88882e2367 * Clearer _declared_dimensions docstring. Session-Id: 22ea9714-39f5-4300-94ab-9c88882e2367 * Appropriate use of currently. Session-Id: f1bc382d-935f-43e1-97e0-dffeeebf7912 * Trim justification from the Zarr docs, and box the two hazards. Removed the sentences arguing for the saver's choices on compression keywords, `order`, and chunk defaults; noted that the NCZarr path writes version 2; moved the interrupted-save text into a warning and the mesh-loading gap into a note. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 30f358dc-3032-41d8-a422-018a1c125354 * Pin the three Zarr saving behaviours review found untested. A derived coordinate survives a round trip as a factory, not as a plain AuxCoord that happens to share its name; a dataless cube reaches the store in its marked form and comes back dataless, as the netCDF integration test already checks for its own format; and the save lands under every dask scheduler, including a distributed client whose workers are separate processes - which the existing pickle test, computing in one process, cannot show. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 26369e2e-3668-4af6-93a8-a7d6184d78d7 * Better error message for writing version 2. Session-Id: 26369e2e-3668-4af6-93a8-a7d6184d78d7 * Move the Zarr interrupted-save warning to where saving is introduced. The previous commit boxed the whole interrupted-save section, leaving a section whose entire body was a warning and an opening line - "A partial store, and Iris offers no rollback." - that only parsed as an answer to the heading above it. Review objected to both. The section is now plain prose, opening with a sentence that stands on its own. The warning moves up to the top of Saving, cut down to the part that actually bites, and links back to the section for the reasoning. A reader meets the hazard before the mechanics rather than after them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 1361c342-bb59-4681-8387-ebda1615f2f3 * Mesh todo comment. Session-Id: 1361c342-bb59-4681-8387-ebda1615f2f3 * Fix test. Session-Id: 1361c342-bb59-4681-8387-ebda1615f2f3 * Fix docs build. Session-Id: 1361c342-bb59-4681-8387-ebda1615f2f3 * Follow brownfield's names, and drop the mesh xfail it fixed The rebase onto brownfield brought three changes made while this branch was being built, each of which this work has to meet. The generic saver is `_CFSaver`, renamed and made private at review of #7338. Every reference here follows it, and the private name is written as a literal rather than a Sphinx role, since there is no page to link to. `iris.io` no longer keys the cubelist savers by module name: it holds the saver functions themselves, in `_savers.cubelist_savers`. `iris.io._zarr_save` is registered there beside the netCDF saver, and `iris.fileformats.zarr.save` registers itself at the foot of its own module - the only place it can be named by identity without `iris.io` importing zarr to do it. Both routes a caller can take to a Zarr save are covered by tests. UGRID meshes load from Zarr as of #7336, so the mesh round trip passes and its strict `xfail` is gone, with the documentation and the design record that described the gap. `ZarrDataset.storage_settings` is the member the interface settled on, so the write tests ask for it by that name. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 96841741-ed31-4388-9014-e3f94eb82651 * Record PR 6 as raised, and what the rebase met Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 96841741-ed31-4388-9014-e3f94eb82651 * Include Xarray test dependency in lock files. Session-Id: 96841741-ed31-4388-9014-e3f94eb82651 * Record that the lock files now carry xarray The entry said they would be regenerated by hand once #7336 landed, and said CI would skip the cross-reader conformance tests until that happened. Both have now been done, so it says so. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 96841741-ed31-4388-9014-e3f94eb82651 * Parenthesise a string the newer ruff refuses Not this branch's code, and not its defect: `.hooks/check_docs_page_metadata.py` came with #7308 and ruff moved from 0.15.20 to 0.16.6 with the `main` merge `brownfield` has just taken, which flags ISC004 on the implicit concatenation in it. Every pull request targeting `brownfield` meets the same failure. It is fixed here because it is one line and this pull request is otherwise green; fixing it on `brownfield` as well would be no bad thing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 96841741-ed31-4388-9014-e3f94eb82651 * Guard the Zarr import in a netCDF test module The new CF grid-mapping test imported the Zarr saver with nothing to skip on, so an Iris without zarr installed gained a test error -- the one thing section 6 of the design says adding Zarr must not do. The guard is per test rather than at the top of the module, because the module's other two tests are netCDF's: skipping those where Zarr is missing would trade a new failure for a silent gap. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 91b685bc-5a5a-4512-a687-b8bc67b9e3fe * Catch a dimension size an existing array already holds create_dimension checked a new declaration against the declarations made in this session, and not against the arrays already in the group. Reopening a store whose "x" is two long and declaring "x" as three succeeded, leaving dimensions reporting one length for a name two arrays hold at two. Section 4.5's create-once rule means the array that is already there is never resized, so there is no outcome where both sizes are true. It now checks `dimensions`, which is the declarations and the arrays both. The saver already renames its own dimension when it meets a collision; this is what lets it see one. The mapping is updated rather than dropped afterwards, because dropping it reassembled it from every array in the group on the next declaration, over a store that may be remote. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 91b685bc-5a5a-4512-a687-b8bc67b9e3fe * Make create_variable's refusals match what it can do Two places where the Zarr dataset said one thing and did another. A fill value was cast to the array's dtype for storage and written to '_FillValue' unconverted, so an int16 array asked for fill_value=1.5 stored 1 and declared 1.5. Iris read that back correctly, because it narrows a declared sentinel to the stored dtype before comparing, which is what hid it; xarray compares against the attribute and would have masked nothing. Section 4.5 asks for one value in both places, so a sentinel the dtype cannot hold is now refused. A floating sentinel may still lose precision, and must: every float32 fill written as a Python float is a float64. supports_dtype answered True for every dtype, including the ones create_variable then refused -- object arrays, and the extended precisions NumPy sizes by platform. Section 4.2 says it answers for what Zarr has a data type for, and now it does. create_variable asks it rather than letting zarr-python raise, so the two cannot disagree again, and the refusal is in Iris's words. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 91b685bc-5a5a-4512-a687-b8bc67b9e3fe * Drop a stale consolidated document when consolidation is declined Saving into a subgroup with consolidate_metadata=False left the root's consolidated document untouched. A default reader believes that document, and it does not mention the group just written: the cubes were in the store and invisible. The user guide already promised the document was removed, and nothing removed it. The root-level case passes today only by side effect -- writing an array at the root makes zarr-python rewrite the root metadata, and the consolidated section goes with it -- which is why the existing test did not see this. A promise kept by side effect is kept only where the side effect reaches. ZarrDataset gains drop_consolidated_metadata, the other half of finalise, working from the whole store's root as finalise does. The saver's completion now calls one or the other, rather than one or nothing, so it waits for the data under compute=False like the consolidation it replaces. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 91b685bc-5a5a-4512-a687-b8bc67b9e3fe * Run the shared write contract against Zarr Section 6 asks for one test body run against every CFDataset implementation, and the write half had only netCDF's subclass. This module's own docstring said the Zarr half arrives with the saver, which it now has. It passes unchanged: the refusal of an unlimited dimension is the one answer the contract sanctions two of, and Zarr gives the other one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 91b685bc-5a5a-4512-a687-b8bc67b9e3fe * Reconcile the design document with itself The review read the specification as current and found four places where it contradicts itself. Each is a change made in section 4.2 or in the decision log and never carried through to the sections that repeat it. The CFDataset member renamed storage_settings on 2026-10-20 kept its old name, `encoding`, in sections 5 and 6, where it names no member of anything. The rename was never logged at all, which is what let it rot. Section 6 still asked for the read-side alignment guard that was removed the same day: that dask chunks tile the store grid in every case, and that a chunk larger than the target is not subdivided. Both now say what the loader does, and what its tests assert -- including that a pinned CHUNK_CONTROL setting is honoured without a warning, because a caller who pins has already taken the trade. Section 6's skipping rule described one importorskip per Zarr test package, which no longer covers everything after a test outside those packages came to exercise the Zarr backend. Q10 was closed into the decision log on 2026-10-16 but its row was left standing in the open questions, still saying the NETCDF3_64BIT defect was preserved rather than fixed. Removed; a question is closed by moving it, and half a move leaves the table disagreeing with the log. Q4 is closed here. It asked whether the netCDF read path should get the same alignment guard as Zarr, and there is no such guard on either path any more, so the asymmetry it existed to resolve cannot arise. Also records PR 4 as merged, and what this review found in the code. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 91b685bc-5a5a-4512-a687-b8bc67b9e3fe * Answer supports_dtype from what Zarr has, not what it lacks The first fix listed the dtypes to exclude, and review found two more it had missed: a structured dtype with an object field, and a zero-length void. Both answered True and then failed in create_variable. A list of exclusions is the wrong shape of answer, and the version floor proves it. zarr-python 3.1 replaced a fixed enum of fifteen data types with an extensible registry, so 3.4 stores a structured dtype and a datetime64 that 3.0.8 -- the oldest Iris accepts -- has no data type for. Any list written against the installed version is wrong on the declared one. So the answer now comes from a table of kinds and item sizes: bool, the integers and unsigneds, floats to 8 bytes, complex to 16, and fixed-length text and bytes of non-zero length. That is the intersection across the versions Iris accepts, which makes a few answers conservative -- datetime64 is False although zarr-python 3.4 would store it. Conservative is the safe direction, because create_variable asks this before it creates anything: a False is a refusal in Iris's words, while a True the store goes back on is the thing section 4.2's rule exists to prevent. Byte order stays out of the answer, although 3.0.8's table is little-endian only. A PP or GRIB load yields big-endian arrays, and a False there would send a big-endian int64 into the saver's narrowing path on every version, including the ones that would have stored it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 91b685bc-5a5a-4512-a687-b8bc67b9e3fe * Correct the header, and say in it that it tracks §12.1 The header said PR 4 was in review and was the next action, while §12.1 recorded it as merged. That is the second time the header has gone stale against §12.1, so it now carries the rule as well as the summary: it is a summary of §12.1 and nothing more, and a reader who stops at the header must not be told something §12 contradicts. §4.2 gains what supports_dtype answers from, and why: a table of what Zarr has rather than a list of what it lacks, the intersection across the zarr-python versions Iris accepts rather than what the newest holds, and byte order left out of it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 91b685bc-5a5a-4512-a687-b8bc67b9e3fe * Create Zarr arrays in the platform's own byte order supports_dtype answered True for a big-endian dtype, which zarr-python 3.0.8 has no data type for at all: `>i8` raises KeyError. The last round found this and recorded it as an acknowledged exception rather than fixing it, on the grounds that answering False would be worse. That much was right. supports_dtype is consulted for int64 and unsigned, so a False would have narrowed big-endian int64 data, and create_variable -- which now asks before it creates -- would have refused big-endian floats outright. A PP or GRIB load produces both, so Iris would have stopped saving ordinary cubes. But the choice was never between those two. create_variable now creates in the platform's own byte order whatever order it is handed, and NumPy converts on assignment: the values stored are the caller's and only the layout is the store's. supports_dtype can then answer True for `>i8` on every supported version, because every supported version can keep it. This is what the netCDF saver has always done with the same data -- a `>f4` cube is float32 in the file it writes -- so the Zarr path is matching it rather than adding a second rule. Checked by running it, not by reading zarr-python's source: zarr-python 3.0.8 installed in a throwaway environment, and every dtype the table answers True for created in turn, on 3.0.8 and 3.4.1 both. Every one creates on both. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 91b685bc-5a5a-4512-a687-b8bc67b9e3fe * Move the Zarr dtype facts out of the comments lib/iris/AGENTS.md puts prose that explains a subsystem in the module docstring, holds a comment to three lines, and says a fact a test can pin belongs in that test with one line in the source naming it. Three of the comments added with the dtype work were essays instead: twelve lines on byte order at the assignment, and twenty on the constant. What Zarr can hold and how that varies by zarr-python version is now a fourth trap in the module docstring, beside the three already there. The constants carry a label each, create_variable keeps one line pointing at the docstring and at TestByteOrder, and the docstrings lose the sentences that defended the design rather than stating it. No behaviour change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 91b685bc-5a5a-4512-a687-b8bc67b9e3fe * Stop the design document citing rounds of work docs/AGENTS.md: published prose must never cite an artefact of how the work was done -- a review finding, a task or plan number, a phase. The decision log and the history table had entries turning on "the third round of the same finding" and "the second round", which resolve to nothing for anyone who was not there. Each now states the change and leaves the dates and the pull request numbers to anchor it, as the surrounding entries already do. No design change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 91b685bc-5a5a-4512-a687-b8bc67b9e3fe * Reconcile three Zarr details with their reviews The mesh paragraph in the saving section claimed that loading a store gives the mesh back. That sentence replaced a caveat saying loading did not yet rebuild the mesh, and outlived the reason it existed: the loading section states the same fact three lines from the top. Removed. The note about a Zarr attribute netCDF cannot hold moves from the mesh subsection, where it has nothing to do with meshes, to the loading section, where the cube it describes comes from. _EXACT_FILL_KINDS and _STORABLE_TEXT_KINDS are frozensets rather than bare strings, matching _TEXT_KINDS in zarr/saver.py. _STORABLE_TEXT_KINDS gains "T", so the two agree on what text is: NumPy's variable-width StringDType creates on zarr-python 3.0.8 and 3.4.1 alike, and supports_dtype had been answering False for it. The dtype contract test asserted that a created array reports the dtype it was given, which holds for the numeric kinds but not the text ones: zarr 3.0.8 reports StringDType() for U5 and object for S5, where 3.4 reports the fixed width. For text it now asserts what both versions promise, that the values written come back. Verified on both: 156 passed in the dataset write tests against zarr 3.0.8, and the Zarr and CF suites pass against 3.4.1. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 76a82181-7fe1-4063-9fd7-deb10fb0d658 * Spell the dtype-kind sets as tuples, not strings `frozenset("biu")` is the same object as `frozenset(("b", "i", "u"))`, but only the second reads as a collection of three things. Review raised this twice; apply it to all five places the Zarr code tests a `numpy.dtype.kind` against a run-together string. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Cut the argument out of two keyword comments Both comments defended a decision instead of stating a fact, which WRITING.md forbids and which pushed one of them to six lines against the three-line guidance in lib/iris/AGENTS.md. Review supplied the replacements; these follow them, adjusted where the suggested wording would have said something untrue. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Raise the Zarr version floor to 3.1.0 The declared floor of 3.0.8 was chosen for being above zarr-python's known data-loss bug, and was never executed: this code was written and tested against 3.4.1, and the review audit recorded 3.0.8 compatibility as unverified. Running the Zarr suite against every release from 3.0.8 to 3.4.0 puts the true floor at 3.1.0. On 3.0.x, `cache_key` serialises `Array.metadata.to_dict()`, which holds a `DataType` enum, a codec object or a `numpy.float32` fill value there - none of them JSON. It is read on every lazy read, so every array over `_LAZYVAR_MIN_BYTES` fails. 3.1.0 replaced the fixed enum of data types with a registry, and with it made `to_dict()` return plain JSON values. `requirements/readthedocs.yml` and `environment.yml` are symlinks that resolve to `requirements/py314.yml`, so the floor is declared in the three `py3*.yml` files alone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Give CountingStore the read-only copy zarr 3.1.6 supplies `WrapperStore` only grew `_with_store` and `with_read_only` in zarr-python 3.1.6. Below that, opening a wrapped store read-only raises, so `Test_sharding` failed on every release from 3.1.0 to 3.1.5 - the helper, not the library. Supplying the two lines here is what puts the floor at 3.1.0 rather than 3.1.6. The behaviour under test is unaffected: an inner-chunk read fetches 2093 bytes against 7300 for a whole shard on 3.1.0, the same shape of answer as 3.4.1. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Answer supports_dtype with True, and delete the dtype table `supports_dtype` is the question `_CFSaver._ensure_valid_dtype` asks before narrowing an integer, and that is the only generic caller. It asks only about `int64` and the unsigned integers. Zarr holds every one of them, so nothing is ever narrowed for a Zarr store and no Zarr save consulted the rest of the table. Only the tests ever supplied a `datetime64`, an object array or a `float128`. Removing the table removes four wrong answers with it. It refused `datetime64`, an ordinary structured dtype, a zero-length `U0` and an eight-byte void; zarr-python stores all four, and `U0` as a variable-width string. A dtype it genuinely has no data type for is now refused by zarr-python inside `create_variable`, which leaves the group as it was. The byte-order coercion stays, with the justification rewritten. 3.0.8 having no big-endian data type was why it was written. The lasting reason is that Zarr version 3…

pp-mo

👍

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 <noreply@anthropic.com> Session-Id: 511ac21a-e935-4d77-baa6-1b8cb42dac17

Every <U test in this package was on the write side. SciTools#6961 records that these dtypes are not strictly Zarr-supported, and the ESA EOPF Sentinel-2 products carry <U7 and <U3 arrays under conditions/geometry, so the read side needs a defined answer rather than an accident. Covers version 2 and version 3, since the two record dimension names differently. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 511ac21a-e935-4d77-baa6-1b8cb42dac17

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 <noreply@anthropic.com> Session-Id: 511ac21a-e935-4d77-baa6-1b8cb42dac17

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 <noreply@anthropic.com> Session-Id: 511ac21a-e935-4d77-baa6-1b8cb42dac17

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 <noreply@anthropic.com> Session-Id: 511ac21a-e935-4d77-baa6-1b8cb42dac17

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 511ac21a-e935-4d77-baa6-1b8cb42dac17

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 <noreply@anthropic.com> Session-Id: 511ac21a-e935-4d77-baa6-1b8cb42dac17

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 <noreply@anthropic.com> Session-Id: 511ac21a-e935-4d77-baa6-1b8cb42dac17

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 <noreply@anthropic.com> Session-Id: 0022a6ec-642a-4844-ae3b-6b8f4421303a

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 0022a6ec-642a-4844-ae3b-6b8f4421303a

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 <noreply@anthropic.com> Session-Id: 0022a6ec-642a-4844-ae3b-6b8f4421303a

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 <noreply@anthropic.com> Session-Id: 0022a6ec-642a-4844-ae3b-6b8f4421303a

…in 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 <noreply@anthropic.com> Session-Id: 0022a6ec-642a-4844-ae3b-6b8f4421303a

The unit test for <U dtypes writes an array with zarr-python and reads it back with Iris, so both halves move whenever zarr-python's encoding moves: change it and the test still passes while a file written years earlier stops loading. That is the reader-against-writer question, not the one <U support actually answers. These stores were written once and committed, so they pin the second. One of the tests asserts the on-disk dtype strings, so regenerating the fixtures - which would destroy their only value - fails rather than passing quietly. These tests FAIL until SciTools/iris-test-data#100 merges, releases, and IRIS_TEST_DATA_VERSION is bumped. Committed red deliberately, so that the new file can be reviewed alongside the fixtures it reads. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 0022a6ec-642a-4844-ae3b-6b8f4421303a

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 0022a6ec-642a-4844-ae3b-6b8f4421303a

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: c18b0b04-01bf-4ba9-96b0-4d8c8b5cfa02

trexfeathers · 2026-10-27T12:27:33Z

(posted by Claude Opus)

Superseded by SciTools#7349, which is the same branch rebased onto brownfield now that SciTools#7346 has merged. This preview existed so the diff could be read while SciTools#7346 was still in review, and it has done that job — thank you @pp-mo for reviewing it here; your comments are answered in the commits that SciTools#7349 carries. Closing rather than retargeting, because a pull request cannot move between repositories.

The review threads stay here and are worth keeping: the save-benchmark parametrisation, the skip guard, and the stored unicode fixtures were all settled in them.

trexfeathers marked this pull request as draft October 26, 2026 14:49

pp-mo suggested changes Oct 26, 2026

View reviewed changes

pp-mo approved these changes Oct 27, 2026

View reviewed changes

trexfeathers and others added 15 commits October 27, 2026 12:15

Record the preview pull request number

833a108

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 511ac21a-e935-4d77-baa6-1b8cb42dac17

Record the test-data release that unblocks PR 7's tests

dadeac3

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 0022a6ec-642a-4844-ae3b-6b8f4421303a

Say that PR 7 is red, and why that was chosen

a5cbc4b

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 0022a6ec-642a-4844-ae3b-6b8f4421303a

trexfeathers force-pushed the zarr-io-pr-7 branch from 4ed92b2 to a5cbc4b Compare October 27, 2026 12:24

trexfeathers mentioned this pull request Oct 27, 2026

Zarr I/O PR 7: real-world test data and benchmarks SciTools/iris#7349

Merged

Record the pull request PR 7 was raised as

9d2a396

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: c18b0b04-01bf-4ba9-96b0-4d8c8b5cfa02

trexfeathers closed this Oct 27, 2026

Navigation Menu

Repository navigation

PREVIEW - Zarr I/O PR 7: real-world test data and benchmarks - #101

PREVIEW - Zarr I/O PR 7: real-world test data and benchmarks#101
trexfeathers wants to merge 17 commits into
zarr-io-pr-6from
zarr-io-pr-7

trexfeathers commented Oct 26, 2026 •

edited

Loading

Uh oh!

pp-mo left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pp-mo left a comment

Uh oh!

trexfeathers commented Oct 27, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

trexfeathers commented Oct 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is here

Where I would look first

What I rejected

Uh oh!

pp-mo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pp-mo left a comment

Choose a reason for hiding this comment

Uh oh!

trexfeathers commented Oct 27, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

trexfeathers commented Oct 26, 2026 •

edited

Loading