Skip to content

PREVIEW: Zarr I/O PR 6: native Zarr saving - #100

Closed
trexfeathers wants to merge 59 commits into
zarr-io-pr-4-5-basefrom
zarr-io-pr-6
Closed

trexfeathers wants to merge 59 commits into
zarr-io-pr-4-5-basefrom
zarr-io-pr-6

Conversation

@trexfeathers

@trexfeathers trexfeathers commented Oct 16, 2026 •

Copy link
Copy Markdown
Owner

(posted by Claude Opus)

This is the sixth of eight pull requests delivering native Zarr support for Iris, and the subject of SciTools#6980: iris.save(cube, "out.zarr") now writes a Zarr version 3 store, with compute=False returning a real dask.delayed.Delayed, Zarr's own storage keywords (chunks, shards, compressors, filters, serializer), a group= keyword, consolidate_metadata, and the CF masking, packing and fill-value handling that zarr-python does not do for free. Part of SciTools#6980. If any of that arithmetic is wrong the store still opens cleanly and reads back the wrong numbers rather than raising, so the masking and packing logic in zarr/_encode.py and ZarrDatasetVariable.__setitem__, and the fill-value declaration in zarr/saver.py's _backend_write, are where I would look first.

Public API added. iris.fileformats.zarr.save and iris.fileformats.zarr.Saver, and iris.save now reaches them for any target ending .zarr, local or remote, through a new "zarr" entry in iris.io._savers.

The base, and what is still outstanding. This branch is built on zarr-io-pr-4-5-base, a branch that merges PR 4 (SciTools#7336, Zarr loading) and PR 5 (SciTools#7338, the CF saver relocation) into one tree, because this pull request both edits PR 4's zarr/_dataset.py and subclasses PR 5's CFSaver. This pull request is opened against zarr-io-pr-4-5-base rather than brownfield, so the diff below is this pull request's own work and nothing else — PR 4's and PR 5's commits are in the base, not in the diff. It is a draft because its two predecessors have not merged yet: until they do there is nothing to retarget it onto. Once both have landed in brownfield, this branch is rebased with git rebase --onto brownfield zarr-io-pr-4-5-base zarr-io-pr-6 and retargeted there, and the diff is unchanged by that move.

No changelog fragment. This programme writes its changelog fragments once, at the merge-back into main, not on the feature branches that land on brownfield — the same rule PRs 1 through 5 followed.

The lock files are not regenerated. xarray is added to the test section of requirements/py3{12,13,14}.yml, as a dependency the cross-reader conformance tests need, but the lock files stay as they are until a human regenerates them by hand once PR 4 lands. Until then, CI's test environment has no xarray, so lib/iris/tests/integration/zarr/test_cross_reader.py skips there. It passes locally: pytest lib/iris/tests/integration/zarr/test_cross_reader.py -v gives 15 passed, against xarray 2026.9.0 and zarr-python 3.4.1.

The first of two changes that touch the netCDF path, and why it does not touch a netCDF file. CFSaver gains a fifth hook, _grid_mapping_writer, so the sixty-three CF grid-mapping parameters can reach a non-netCDF store at all — the base implementation hands them a plain writer onto .attributes, which is what Zarr gets. netcdf.saver.Saver's override returns cf_var.variable, the same netCDF4.Variable object those parameters have always been assigned onto, so what lands in a netCDF file is unchanged: the override is an identity, not a behaviour change. The ASCII-to-bytes coercion that object still bypasses is Q8 in the design document, unblocked by this hook but not closed by it.

The second, and the one to look at hardest: a block moved out of netcdf.saver.save. Deciding which of a cube's attributes a store can hold globally is cube logic, not storage logic, but it was written inside netcdf.saver.save and nothing else could call it — so on the Zarr path every cube global attribute was simply dropped. With iris.FUTURE.save_split_attrs = True, CFSaver.write deliberately writes no globals and leaves them to the caller: netCDF wrote them, Zarr did not, and title, institution, history, source and references vanished from a Zarr save. In the legacy default mode it is the local_keys computation that was missing, so a CubeList whose cubes disagreed about an attribute kept only the last cube's value where netCDF gave each data variable its own. The block is now cf.saver.marshal_attributes, called by both savers. The extraction is behaviour-preserving — its only edit is that the legacy deprecation message takes the format name, so netCDF passes "netcdf" and gets back the exact string it had — and lib/iris/tests/integration/test_netcdf__loadsaveattrs.py gives 2059 passed before and 2059 passed after, as does the whole netCDF suite. One thing did change, and it is worth saying so nobody discovers it in a traceback: those warnings now report lib/iris/fileformats/cf/saver.py as their source rather than lib/iris/fileformats/netcdf/saver.py. Their text, category and count are unchanged, and nothing in the repository filters on the location.

One consequence for a maintainer to rule on. Because the Zarr saver now shares that marshalling, a legacy-mode Zarr save emits the same attribute-handling deprecation netCDF has emitted since 3.8 — consistent with netCDF, and correct in that the legacy mode is deprecated for every format, but it does mean a brand-new format ships warning on its default path, since save_split_attrs still defaults to False. Suppressing it for Zarr alone would mean a flag on the shared function whose only purpose is to hide a deprecation from one format, which is the divergence the extraction exists to prevent. Left as it is, deliberately, for an owner to decide.

Packing needed its own implementation here, not a reuse of netCDF's. The generic CF saver's packing= support sets scale_factor and add_offset as attributes and streams the data through unchanged, because netCDF4-python applies that transform itself the moment the attributes are set on a variable it is about to receive data for. zarr-python has no such behaviour: left alone, packing= on a Zarr save would have set the attributes correctly and then written physical-unit values straight into a narrower, usually integer, stored dtype — numbers with no relationship to what a reader would reconstruct from them. zarr/_encode.py now has a pack function, the write-side mirror of the existing _decode.unpack, and ZarrDatasetVariable.__setitem__ applies it before the masked fill, because the fill value lives in packed space and packing an already-filled sentinel would send it through the same transform as real data. The Zarr saver also refuses a non-integer packing dtype, matching the check and the message netcdf/saver.py already has.

What this does not do. A cube with a mesh saves the full CF-UGRID structure, but cannot yet be loaded back: nothing in the Zarr loader rebuilds a MeshXY, and iris.mesh.load_meshes still filters to the netCDF handler, so the round trip test carries a strict xfail rather than being left unwritten, and closing the gap will announce itself when it does. mode="a" adds new arrays to an existing store; it does not match an incoming coordinate against one the store already holds, so saving a second cube with its own latitude writes a second array, latitude_0, rather than reusing the first, because deciding when two coordinates are "the same" across unequal values, differing units and different calendars is its own piece of work. And a masked string variable is refused rather than written: the only sentinel a Zarr string array carries is the empty string, which is also a legitimate label, and Iris's own loader does not restore a mask from one — so writing it would lose the mask in silence. The netCDF saver cannot store a masked string coordinate either, so this is parity with it rather than a step back from it.

The full suite. pytest lib/iris/tests/unit lib/iris/tests/integration -q -n auto, with NPY_DISABLE_CPU_FEATURES set as the repository's own instructions ask: 11418 passed, 59 skipped, 1 xfailed (the mesh round trip above), 10 errors, 202.28s. The 10 errors are all in test_coord_systems.py, all one pre-existing fixture race — a module-scoped tmp_path_factory fixture shared across pytest-xdist workers — and not something this pull request touches: the file is byte-identical to its state on zarr-io-pr-4-5-base, and the same ten tests pass when the file is run on its own, without -n auto.

trexfeathers and others added 2 commits October 16, 2026 16:40
Two instructions for future agents, neither affecting behaviour.

lib/iris/AGENTS.md gains a TODO Comments subsection. Agents avoid the
marker because the wider community discourages it, but Iris uses
`# TODO: ` throughout and puts Ruff's TD and FIX rule families in the
ignore list, so nothing here flags one. The rule says to use it, to
leave it in the file rather than only in the pull request body, and to
name the event that removes it.

The Zarr spec gains PR 8, a terms-of-art sweep over everything the
branch adds. The spec and the plans were written before the rule in
.github/WRITING.md, so they may carry coinages it would now reject. A
later agent decides whether a term is safe by checking whether the
repository already uses it, so a coinage that reaches main cannot be
told apart from an established one. The programme count goes from eight
to nine, and the header's stale Last updated is corrected.


Session-Id: 208cb6e9-cc1d-426c-9a1e-2dceff1d432b

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…iTools#7338)

* Start the PR 5 plan: constraints, scope, findings, file structure

The research half of the plan for relocating the CF saver. Records the
five places the saver still names netCDF, the sibling relationship with
PR 4, and the interface the tasks will leave behind.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 64bc7b40-f56b-470a-9d2a-ee65e6554380

* Add PR 5 plan task 1: the dataset answers for its dtypes

Also records the NETCDF3_64BIT finding: netCDF4 reports that format as
NETCDF3_64BIT_OFFSET, so the saver's narrowing never fires for it and an
int64 save fails in the C library. Verified against the environment. The
plan preserves it and raises an issue rather than fixing it here.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 64bc7b40-f56b-470a-9d2a-ee65e6554380

* Add PR 5 plan task 2: relocate the dask lock toolkit

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 64bc7b40-f56b-470a-9d2a-ee65e6554380

* Add PR 5 plan task 3: name the saver's four netCDF hooks

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 64bc7b40-f56b-470a-9d2a-ee65e6554380

* Add PR 5 plan task 4: the move itself

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 64bc7b40-f56b-470a-9d2a-ee65e6554380

* Finish the PR 5 plan: repointing, the spec record, the pull request

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 64bc7b40-f56b-470a-9d2a-ee65e6554380

* Self-review the PR 5 plan: fix the signatures and the pointers

The interface sketch disagreed with the tasks on three hook signatures,
and four Review Focus entries named tests and steps that the tasks did
not have. Also records which files look as though they need changing and
do not.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 64bc7b40-f56b-470a-9d2a-ee65e6554380

* Correct the PR 5 plan against PR 2.5, now merged

IrisDeprecation lives in iris._deprecation and is re-exported from
neither iris.exceptions nor iris.warnings, so the test file's import was
wrong and a -W filter naming either would have failed open.

The no-deprecation load test promoted the warning to an error, which
SciTools#7333 showed passes against an implementation that still warns:
iris.load captures the exception into LOAD_PROBLEMS and returns a
degraded cube. It now records warnings and asserts the file contributed
no load problems.

Also picks up the new one-exit rule in lib/iris/AGENTS.md, and moves the
baseline to the tip of brownfield.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 64bc7b40-f56b-470a-9d2a-ee65e6554380

* Stop the no-deprecation tests tripping the save_split_attrs warning

A plain iris.save emits an IrisDeprecation of its own: the legacy
attribute mode deprecated in Iris 3.8. Verified by recording warnings
around a save on the current brownfield. Both tests now set the FUTURE
flag, so what they assert is what they mean to assert.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 64bc7b40-f56b-470a-9d2a-ee65e6554380

* Ask the dataset whether it holds a dtype

The saver read the open file's netCDF format string to decide whether to
narrow an integer to int32. That read would become generic code naming
netCDF formats once the CF saver moves, so the question now goes to the
dataset as a capability, and the format survives only in the message the
user reads.

A file created as NETCDF3_64BIT reports itself as NETCDF3_64BIT_OFFSET
and so escapes the narrowing, which fails the save. That is today's
behaviour and is kept, with a test saying so.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 1aa0b859-b88c-47c7-b496-f2f17c513935

* Move the dask lock toolkit to iris.fileformats.cf

The module coordinates dask workers writing to one target, and does it
by asking dask which scheduler is in play. It imports nothing from Iris
and nothing from netCDF4, so it was only ever netCDF's by where it sat.
A second saver needing worker coordination should find it rather than
grow its own.

The code is unchanged. The docstring no longer describes the netCDF
saver as the only caller.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 1aa0b859-b88c-47c7-b496-f2f17c513935

* Gather the saver's netCDF-specific code into four methods

The class mixes the CF data model with netCDF4's own vocabulary. Four
blocks are the whole of the second part: creating a char variable over a
string dimension, writing to an emulating variable or an NCZarr target,
flushing those writes before the file closes, and handing the
site-configured patch hook its unwrapped netCDF4 objects.

Each is now a method with a name, called from the generic code that used
to contain it. Nothing moves modules and nothing changes behaviour.

_backend_write still types its cf_var parameter as NetCDFDatasetVariable,
since its body reaches for netCDF-only members (is_emulated,
unencoded_variable, emulated_data_array); _lazy_stream_data's own
parameter widens to CFDatasetVariable ahead of the move, so the one call
between them needs a type: ignore[arg-type].

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Session-Id: 1aa0b859-b88c-47c7-b496-f2f17c513935

* Relocate the CF saver to iris.fileformats.cf.saver

Everything about writing a cube that does not depend on what it is
written into now lives in iris.fileformats.cf.saver.CFSaver: the CF data
model, and storage calls that go through CFDataset.

iris.fileformats.netcdf.saver keeps what is netCDF's. The file to write
into, the keywords netCDF4 accepts, and the four things netCDF does
differently from a generic store. Saver, save and save_mesh stay exactly
where they are, and Saver.write keeps its signature.

Four names that are not netCDF's - CFNameCoordMap,
CF_CONVENTIONS_VERSION, MESH_ELEMENTS and SPATIO_TEMPORAL_AXES - are
reachable from both old homes, warning on use.

The private constants moved too, and are imported back into
iris.fileformats.netcdf.saver rather than forwarded. Code elsewhere in
Iris still reads them from there, on the load path, where a deprecation
per cube would be intolerable - and they are private, so owed no
deprecation cycle. The next commit repoints those readers and the import
goes with them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 1aa0b859-b88c-47c7-b496-f2f17c513935

* Keep file_write_lock on the Saver API page

Sphinx attaches a "#:" comment to an assignment only when the comment
block runs straight into it. A plain "#" line sat between the two, so
Saver.file_write_lock - a public attribute of a public class - dropped
off the API page in the relocation. Reorder the three lines.

sphinx.pycode.ModuleAnalyzer confirms it: before, ('Saver',
'file_write_lock') was absent from attr_docs; after, it is back with the
text it had before the move.

Also say in TestNothingInsideIrisUsesTheOldNames what the class really
covers. A save and a load are quiet, which is not the same as nothing in
the repository using an old name - two test modules still do, and the
next commit repoints them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 1aa0b859-b88c-47c7-b496-f2f17c513935

* Point every reader at the CF saver's new home

The CF loader's two reaches into the netCDF saver were deferred with a
comment saying they would become plain imports once the saver moved. It
has, so they have.

Four test modules read a relocated name through the netCDF package,
which now warns. They read it from iris.fileformats.cf.saver instead, so
that a full test run raises no deprecation of its own.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 1aa0b859-b88c-47c7-b496-f2f17c513935

* Record the CF saver relocation in the design document

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 1aa0b859-b88c-47c7-b496-f2f17c513935

* Record the pull request number in the design document

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 1aa0b859-b88c-47c7-b496-f2f17c513935

* Correct the prose the relocation left untrue

Four statements no longer held. A test docstring named two modules as
still importing CF_CONVENTIONS_VERSION from iris.fileformats.netcdf,
which 63accb7 repointed. Two comments in integration/test_cf.py still
placed _CF_ATTRS in netcdf.saver. test__relocated_names called its list
the methods the netCDF Saver answered to before, which counts the four
hooks this branch invented. cf/_reader.py called itself the one module
in iris.fileformats.cf coupled to the netCDF package, which cf/saver.py
importing ATTRIBUTE_HANDLERS ended.

The cf/saver.py module docstring promised that every storage call goes
through CFDataset. _add_grid_mapping_to_dataset does not, so the
docstring names the exception now, and the call site names what it costs
a non-netCDF store: a bare AttributeError, because the line does not use
deprecated_netcdf_member().

The import-cycle explanation was wrong in all three places it appeared.
Tracing the module bodies from five entry points shows cf.saver running
while the netCDF package is itself part-way through importing, not after
it is fully loaded. Patching the eager netcdf import out of
lib/iris/fileformats/__init__.py in memory, with its module-level
load_cubes references made lazy, leaves all nine modules importing first
cleanly, so that import is not what makes the cycle work. What does is
netcdf/_attribute_handlers.py importing nothing from the CF package. The
module docstring, the test docstring and the design document say that
now, as does the pull request body.

No code changed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 1aa0b859-b88c-47c7-b496-f2f17c513935

* Make the generic saver private

The class a format subclasses is _CFSaver, not CFSaver. CFDataset is
public because every member of it is something a format implements, and
because a user can reach one of its variables through CFVariable.cf_data.
The saver is the other shape: what a format implements is four private
hooks, one of which the Zarr saver will supply and three of which it will
inherit, while everything public on the class is the caller's API that
each format's own public Saver inherits unchanged.

Naming it publicly would promise the shape of thirty-six methods on the
evidence of a single subclass, and only a deprecation cycle could take
that back. The module stays public: it holds the four relocated names,
and its docstring is where a format author is told what to subclass.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 9b0539e4-6e83-4516-ae5b-c0800f4d83d7

* Fix the int64 narrowing on 64-bit-offset netCDF files

netCDF4 reports a file created as "NETCDF3_64BIT" under the name
"NETCDF3_64BIT_OFFSET", which was missing from the list of formats that
cannot hold every dtype. Saving int64 data to that format therefore
skipped the narrowing to int32 and failed inside the netCDF C library.

The relocated saver is untouched: it asks supports_dtype, and the list it
answers from is netCDF's own. Two tests existed only to pin the failure
and are gone; the too-large-value test now runs over all three restricted
formats, which pins that the message names the format of the file that
was opened rather than the keyword the caller passed.

Fixes SciTools#7337, at @stephenworsley's suggestion in review of
SciTools#7338. The closing keyword goes on the merge-back, where the
programme keeps them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 594a4b04-068d-461d-b814-86a0d6fea022

* Cut the import test to the one reachable order

The test ran nine subprocesses, one per module in the CF and netCDF
packages, to pin that each survives being the module a process asks for
first. Python runs iris.fileformats.__init__ before any submodule body,
and that imports the netCDF package, which draws in the whole cascade.
Every one of the nine subprocesses therefore did identical work: the
module it named was already in sys.modules by the time the interpreter
reached it, so the final step was a dictionary lookup.

One import of iris.fileformats covers every module in both packages.
What the subprocess still buys is the cold start, with nothing
pre-loaded, which an import inside the test process cannot give.

Measured at 1.4s per case, so this returns about 11s of CPU to the
suite.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 362bdfd8-761e-4373-80a0-f16d413f133e

* Add missing pass.

Session-Id: 362bdfd8-761e-4373-80a0-f16d413f133e

* Mark the netCDF import as going, and say when

The ATTRIBUTE_HANDLERS import leaves when the handlers themselves
relocate into the CF package, which happens with Zarr saving. The
docstring explained why the import is here but not that it is
temporary, and nothing at the import site was greppable as a TODO.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 3ba642d6-761d-45d6-b741-6f172366fd86

* Don't attempt to exhaustively list possible format values.

Session-Id: 87566283-5999-4e68-b805-e9275930f3e4

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

@stephenworsley stephenworsley left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review covers parts of zarr/_dataset.py. The main concerns here are for the API of the ZarrDataset class, specifically the mode and zarr_format keywords which are slightly inconsistent with their NetCDFDataset counterparts.

Comment thread lib/iris/fileformats/zarr/_dataset.py Outdated
Comment thread lib/iris/fileformats/zarr/_dataset.py Outdated
Comment thread lib/iris/fileformats/zarr/_dataset.py Outdated
Comment thread lib/iris/fileformats/zarr/_dataset.py Outdated
Comment thread lib/iris/fileformats/zarr/_dataset.py
Comment thread lib/iris/fileformats/zarr/_dataset.py Outdated
Comment thread lib/iris/fileformats/zarr/_dataset.py Outdated
Comment thread lib/iris/fileformats/zarr/_dataset.py Outdated
Comment thread lib/iris/fileformats/zarr/_dataset.py Outdated

@stephenworsley stephenworsley left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review covers cf/saver.py and mostly concerns a question of what belongs in cf vs zarr.

Comment thread lib/iris/fileformats/cf/saver.py Outdated
Comment thread lib/iris/fileformats/cf/saver.py Outdated
Comment thread lib/iris/fileformats/cf/saver.py Outdated
Comment thread lib/iris/fileformats/cf/saver.py Outdated
Comment thread lib/iris/fileformats/cf/saver.py

@stephenworsley stephenworsley left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review covers io/__init__.py and zarr/__init__.py. This covers the wrapping of a function in a non-standard way which seems to be unnecessarily complicated. A simpler existing pattern exists for such cases where a delayed import is desired and it doesn't look like calling zarr._save_cubes does anything that a wrapped import and call of zarr.save wouldn't already do.

Comment thread lib/iris/io/__init__.py Outdated
Comment thread lib/iris/fileformats/zarr/__init__.py Outdated
Comment thread lib/iris/fileformats/cf/saver.py Outdated
Comment thread lib/iris/io/__init__.py Outdated
Comment thread lib/iris/tests/unit/io/test_find_saver__zarr.py

@stephenworsley stephenworsley left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review covers the docs.

Comment thread docs/src/user_manual/explanation/zarr_io.rst
Comment thread docs/src/user_manual/explanation/zarr_io.rst Outdated
Comment thread docs/src/user_manual/explanation/zarr_io.rst Outdated
Comment thread docs/src/user_manual/explanation/zarr_io.rst Outdated
Comment thread docs/src/user_manual/explanation/zarr_io.rst Outdated
Comment thread docs/src/user_manual/explanation/zarr_io.rst Outdated
Comment thread docs/src/user_manual/explanation/zarr_io.rst Outdated
Comment thread docs/src/user_manual/explanation/zarr_io.rst Outdated
Comment thread docs/src/user_manual/explanation/zarr_io.rst Outdated
Comment thread docs/src/user_manual/explanation/zarr_io.rst

@stephenworsley stephenworsley left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple important cases which I think are lacking in test coverage at the moment.

Comment thread lib/iris/tests/integration/zarr/test_save.py
Comment thread lib/iris/tests/unit/fileformats/zarr/saver/test_Saver.py
Comment thread lib/iris/tests/integration/zarr/test_save.py Outdated
trexfeathers and others added 20 commits October 22, 2026 16:10
* Start the PR 4 plan: header, constraints, scope and findings

Zarr loading. Sections through "Findings that refine the spec", which
records seventeen points checked against zarr-python 3.4.1 while the plan
was written -- including the answer to the spec's open question Q2, and
three places the spec needs amending.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 91122c91-913f-44c8-841f-f69ed66ac830

* Add the PR 4 plan's file structure and interface design

Every file created or modified, the order the tasks run in and why, and
the signatures of ZarrDataset, ZarrDatasetVariable, the decode functions
and the load-chain additions -- so that each task below can be read on
its own without guessing a neighbour's names.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 91122c91-913f-44c8-841f-f69ed66ac830

* Add PR 4 plan tasks 1-3: the dependency, the interface, the variable

Task 1 makes zarr optional and proves Iris imports without it. Task 2 adds
format and encoding to the CF dataset interface and splits the shared
contract into read and write halves. Task 3 presents a Zarr array through
the interface, which is where the dimension contract lands.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 91122c91-913f-44c8-841f-f69ed66ac830

* Add PR 4 plan tasks 4-5: the Zarr group, and the shared contract

Task 4 assembles CFDataset.dimensions from the arrays and decides what a
name used at two sizes means. Task 5 asks the shared read contract of both
Zarr versions, and adds the check that the cf package names no
implementation class.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 91122c91-913f-44c8-841f-f69ed66ac830

* Add PR 4 plan tasks 6-7: CF decoding, and the lazy read

Task 6 implements masking, valid-range masking and unpacking as pure
functions, with the version-aware fill-value precedence and both
_FillValue encodings. Task 7 is the lazy read: no proxy, chunking rounded
up to the store's own, and a cache key that carries the array's metadata.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 91122c91-913f-44c8-841f-f69ed66ac830

* Add PR 4 plan tasks 8-10: spans, CFReader, and the Zarr loader

Task 8 is the one change a netCDF file could notice, so it runs its suite
on both backends. Task 9 lets CFReader take an already-open CF dataset of
any format. Task 10 builds cubes, and decides what a group holding no data
variables says to the user.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 91122c91-913f-44c8-841f-f69ed66ac830

* Add PR 4 plan tasks 11-12: format detection and the load chain

Task 11 recognises a store -- by a marker file locally, by scheme and
suffix remotely, and never by the suffix alone. It also has to say in code
that an NCZarr URL is still NCZarr's, because the store it names is real.
Task 12 is the chain: load_stores, the partition after glob expansion, and
the two saver-side changes section 4.7 asks for.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 91122c91-913f-44c8-841f-f69ed66ac830

* Finish the PR 4 plan: integration, docs, the pull request, verification

Task 13 loads realistic stores both ways round -- version 2 through Iris's
own NCZarr writer, version 3 through a fixture shaped like the NOAA GFS
archive. Task 14 documents loading. Task 15 updates the spec, including
closing its open question about fixed-length unicode. Sections 6 and 7 say
what done is measured against.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 91122c91-913f-44c8-841f-f69ed66ac830

* Self-review the PR 4 plan: fix the pointers, close the gaps

Review Focus now points at the tasks it means. The interface section's
_align_chunks and decode_parameters signatures match what the tasks write.
Adds F18, which is the gap the review found: this pull request creates the
first way to get a nested attribute onto a cube, and the netCDF saver
cannot write one -- so Task 10 characterises it and Task 14 documents it.
Also adds the version-aware masking tests at the loaded-cube level, the
async-concurrency note the spec asks for, and a step to read the generated
API pages.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 91122c91-913f-44c8-841f-f69ed66ac830

* Correct the PR 4 plan against the files that are actually there

The netCDF contract subclass lives in test_NetCDFDataset__contract.py, not
test_NetCDFDataset.py. The sequencing paragraph now uses the task numbers
the tasks ended up with, and the file table lists every test module the
tasks create.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 91122c91-913f-44c8-841f-f69ed66ac830

* Point the PR 4 plan's baseline at the merged spec

The spec review merged as SciTools#7330. Its tree is identical to the spec
branch the plan was written against, so every [verified] claim stands
unchanged; only the baseline pointer moves.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 436a543d-0543-48a7-92b3-f8bef5d37d8a

* Base the PR 4 plan on zarr-io-pr-3, with a fork preview first

PR 3 is still in review, so PR 4 is cut from its branch rather than
waiting for brownfield. The base branch of a pull request has to live in
the repository it is raised against, and zarr-io-pr-3 is on the fork, so
the draft is raised fork-internally: its diff is PR 4's work alone
instead of PR 4 plus PR 3's ten commits. Task 15 gains a sixth step for
when SciTools#7316 merges - rebase onto brownfield with the three-argument form,
re-run the two tasks that read PR 3's code, re-compare the full suite
against brownfield, raise the real pull request and close the preview.

The full-suite baseline moves to zarr-io-pr-3 for the same reason: it is
the only comparison that isolates this pull request's effect.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 4aeb6748-437c-4241-b20c-c5e1be85150e

* Add zarr as an optional dependency, and the package that uses it

The package holds only its docstring for now: iris.fileformats does not
import it, so Iris still installs and imports without zarr-python. The
test package skips itself when the dependency is absent.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 7a880931-8be4-4c5c-a2cd-afa8e486f9e4

* Split the Zarr store-builder tests into their own module

test_optional_dependency.py now checks only what its name says: Iris
importing safely without zarr-python. The two tests proving make_store
builds both Zarr format versions move to test_stores.py, named for the
module they exercise, with their imports at the top where they belong.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 7a880931-8be4-4c5c-a2cd-afa8e486f9e4

* Add format and encoding to the CF dataset interface

format names the concrete format for a message or a diagnostic; encoding
reports the storage settings a store actually holds, readable but never
interpreted. Splits the shared contract into its read and write halves, so
that an implementation arriving in two pieces can be held to the first.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 7a880931-8be4-4c5c-a2cd-afa8e486f9e4

* Keep close and context-manager coverage in the read half of the contract

close, closed and the context manager are read-side behaviour per spec
section 4.2, not write-side: a read-only implementation must still answer
them correctly, and the whole point of splitting the contract is that a
read-only Zarr store gets checked against every read-side question. Moving
these three tests to the write half, as the first cut did, would let that
divergence through unchecked.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 7a880931-8be4-4c5c-a2cd-afa8e486f9e4

* Reflow CFDataset.format's docstring under the 88-character line limit

The one-line summary copied from the plan's interface sketch ran to 91
characters; nothing in the pre-commit toolchain catches an over-length
docstring line (E501 is disabled repo-wide, and W505 needs a max-doc-length
the repository does not set). Splitting it into a short summary plus an
extended description also fixes it ahead of the Zarr docstrings a later
task writes from the same sketch.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 7a880931-8be4-4c5c-a2cd-afa8e486f9e4

* Present a Zarr array through the CF dataset interface

The dimension contract is the substance: a Zarr dimension may be unnamed,
and an unnamed one is anonymous rather than an error. _ARRAY_DIMENSIONS is
storage, so it is withheld from the CF attributes, and fill_value is
reported as the storage fact it is rather than as a missing-value flag.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 7a880931-8be4-4c5c-a2cd-afa8e486f9e4

* Present a Zarr group through the CF dataset interface

Dimensions are assembled from the arrays, and a name used at two sizes is
omitted with a warning rather than merged: Zarr places no restriction on
reusing a name across arrays, so a store meaning two things by one name is
conforming, and merging them would produce a wrong cube.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 7a880931-8be4-4c5c-a2cd-afa8e486f9e4

* Assert variables keep an omitted dimension name in their own dimensions

The inconsistent-dimension test checked the warning and the mapping's
omission but not that the two arrays still report the name themselves,
which is the fact that lets a later (name, size) match tell them apart.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 7a880931-8be4-4c5c-a2cd-afa8e486f9e4

* Implement the CF decoding Zarr does not do for itself

Masking, valid-range masking and scale/offset unpacking, as pure functions
of values. The two rules worth reading twice: a floating _FillValue is
base64 on version 3, which is xarray's convention rather than a defect;
and Array.fill_value is a mask source on version 2 only, and only when the
variable declares no CF attribute of its own.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 7a880931-8be4-4c5c-a2cd-afa8e486f9e4

* Stop an out-of-range missing-value attribute crashing the load

decode_fill_value narrowed a decoded sentinel to the variable's dtype
unconditionally, so a value that overflowed it - signed, negative or
unsigned - raised OverflowError out of the whole load instead of warning
and degrading, same as any other undecodable sentinel. Also adds the
end-to-end test for the rule the module exists for: a version 3 array of
zeros with no CF attributes comes back through decode_parameters and
mask_and_scale with its zeros intact, not just with an empty sentinels
tuple.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 7a880931-8be4-4c5c-a2cd-afa8e486f9e4

* Read Zarr data lazily, aligned to the store's own chunks

No proxy: a zarr.Array is picklable and a store is a namespace, so the
array goes to as_lazy_data directly and CF decoding is a visible map_blocks
layer. Chunking is rounded up to whole multiples of the store's own, never
below, because reading any part of a Zarr chunk fetches the whole object.
The Dask cache key carries the array's metadata document rather than its
address, which alone goes stale when a store is rewritten in place.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 7a880931-8be4-4c5c-a2cd-afa8e486f9e4

* Prove the Zarr read uses the cache, and stop a warning that cried wolf

The cache test compared two Dask names, which Dask makes equal by
deterministic tokenisation whether or not a cache key is passed, so it
passed with the key removed from read_data entirely. It now replaces the
process-wide cache and asserts the first read fills it and the second
reuses it.

Chunk alignment warned whenever rounding up overshot the array extent and
the extent did not tile the store grid. Capping at the extent leaves one
block spanning the dimension, which reads each store chunk exactly once, so
nothing was subdivided and the warning was false - on an ordinary daily
(365, 500, 1000) store, on every load. The two conditions that do mean
something, an oversize store chunk and a subdivided fixed dimension, are
kept and now have tests saying so.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 7a880931-8be4-4c5c-a2cd-afa8e486f9e4

* Ask the shared CFDataset read contract of the Zarr store

Both Zarr versions answer the same questions the netCDF implementation
does, so a divergence between the two fails here rather than somewhere in
the loader. Adds the check that no module in the cf package names an
implementation class.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 7a880931-8be4-4c5c-a2cd-afa8e486f9e4

* Match CF dimensions on (name, size), not on the name alone

Zarr places no restriction on reusing a dimension name across arrays, so a
name alone can link two variables that mean different things by it. An
unnamed dimension is anonymous and matches nothing, including another
anonymous dimension. netCDF names are unique within a group, so this is a
widening that no netCDF file can observe.

Two pre-existing unit-test doubles modelled `dimensions` but not `shape`,
which the old bare-name `spans()` never read and the new pair-based one
does: `_NetCDFVarWithDimensions` (shared by the CFBoundaryVariable,
CFClimatologyVariable and CFLabelVariable span tests) gains a `shape`
property derived from a deterministic per-name size map, and the two
`test_CFVariable.py` cases that compared against a bare `MagicMock` now
compare against the same concrete stub on both sides - a MagicMock
collaborator was auto-answering the new `_dimension_identities()` call with
something that iterates as empty, silently flipping an assertion instead of
failing loudly.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 7a880931-8be4-4c5c-a2cd-afa8e486f9e4

* Let CFReader read any already-open CF dataset

A loader that has opened a store hands it straight over, so the choice of
backend stays with the loader rather than becoming a branch inside the
reader. A borrowed dataset is not closed by the reader that borrowed it.

Also wires the canonical Zarr test store's data variable to its grid
mapping variable by name, which CF classification needs to recognise the
grid mapping variable as one at all - it was present but unreferenced.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 7a880931-8be4-4c5c-a2cd-afa8e486f9e4

* Build cubes from a Zarr store

Open the store, hand it to CFReader, and run the same cube-building loop
the netCDF loader runs. One group per call: a store can hold unrelated
datasets, so a group with no data variables names the groups that have
some rather than returning nothing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 7a880931-8be4-4c5c-a2cd-afa8e486f9e4

* Teach the format agent to recognise a Zarr store

Locally by the marker file a store puts at its root, remotely by the URI
scheme and the .zarr suffix. Not by the suffix alone: that would claim any
directory someone named .zarr, and joined with the remote rule it would
leave the scheme test unreachable. An NCZarr URL is still NCZarr's, which
needs saying in code because the store it names is a real one.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 7a880931-8be4-4c5c-a2cd-afa8e486f9e4

* Skip the NetCDF-detection test when iris-test-data is absent

test_the_agent_still_picks_netcdf_for_a_netcdf_file opened a real file with
no skip_data guard, so missing test data raised FileNotFoundError instead of
skipping - the opposite of lib/iris/tests/AGENTS.md's "missing data skips
affected tests by design, silently, and without failing". Follows the same
@_shared_utils.skip_data idiom already used in test_load_meshes.py.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 7a880931-8be4-4c5c-a2cd-afa8e486f9e4

* Let the load chain reach a target that is not a file

A new iris.io.load_stores answers 'this target is not a file', once, for a
local directory and a remote URL alike, rather than teaching load_files
about directories in one place and the scheme dispatch in another. The
partition happens after glob expansion, so iris.load('data/*') still
accepts a mix of netCDF files and Zarr stores.

load_files no longer re-expands its input: _generate_cubes already runs
expand_filespecs before partitioning files from stores, and expanding an
already-expanded list silently collapses two identical paths produced by
two different glob patterns into one, which broke the existing duplicate-
match loading behaviour.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 7a880931-8be4-4c5c-a2cd-afa8e486f9e4

* Update the known-formats snapshot for the registered Zarr spec

test_known_formats compares iris.fileformats.FORMAT_AGENT's registered
specs against this snapshot. Registering the "Zarr" FormatSpecification
added an entry the snapshot never gained, because the sweep that landed it
covered lib/iris/tests/unit/io/ and lib/iris/tests/unit/fileformats/, and
this file lives outside both. One line, in the alphabetical position
FormatSpecification._sort_key already puts it: after the other priority-5
entries and before the first priority-4 one.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 7a880931-8be4-4c5c-a2cd-afa8e486f9e4

* Restore load_files's public contract, and guard the dispatch split

load_files is public API with no leading underscore and a documented
promise that filenames may be globs; narrowing that silently to
already-expanded paths broke the promise for any future caller. Its body
moves to a private _load_files that expects pre-expanded paths - which is
what the duplicate-collapsing fix actually needed - and public load_files
goes back to expanding once, then delegating. _generate_cubes, the only
caller, now calls _load_files directly with its own already-expanded,
partitioned list, so the double-expansion bug stays fixed.

Cross-reference comments at both independent file-or-store checks
(loading.py's _generate_cubes and ugrid_load.py's load_meshes) name each
other, so an edit to one is more likely to carry to the other - the
duplication itself is intentional and out of scope here.

The remote-URL dispatch test no longer calls iris.load against a real
S3 URL: with s3fs installed (a common transitive Dask dependency) that
would issue a genuine outbound request. It now mocks load_stores and
asserts the URL reaches it unmangled, which is the actual claim under
test, and was verified to fail under a reintroduced mangling bug.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 7a880931-8be4-4c5c-a2cd-afa8e486f9e4

* Load realistic Zarr stores end to end, both versions

Version 2 comes from Iris's own NCZarr writer, so the native reader and
netCDF-c are checked against each other. Version 3 comes from a fixture
shaped like the NOAA GFS archive: sharded, packed, with a scalar grid
mapping, a two-dimensional auxiliary coordinate and a base64 fill value.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 7a880931-8be4-4c5c-a2cd-afa8e486f9e4

* Withhold netCDF-c's _NCProperties stamp, and add bounds to the Zarr fixture

_NCProperties is netCDF-c's own bookkeeping attribute, stamped onto the
root group of every store it writes; netCDF4-python hides it from
ncattrs() by convention, so it has never reached a cube loaded through
netCDF. Zarr has no equivalent convention, so the native reader surfaced
it as a plain attribute, which would have put a stray provenance string
on cube.attributes for every cube loaded from an NCZarr-written store.
Withheld alongside _ARRAY_DIMENSIONS for the same reason, with its own
unit test; the integration comparison no longer strips it by hand.

Also adds bounds to the version 3 fixture's latitude coordinate:
CFBoundaryVariable.spans is one of the overrides this pull request
rewrote against the dimension contract, so a bounded coordinate
exercises code this branch actually changed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 7a880931-8be4-4c5c-a2cd-afa8e486f9e4

* Document Zarr loading

The page a user comparing formats will look for, beside the netCDF one:
what a store is, how a remote one is reached, what a Zarr dimension name
does and does not guarantee, why the array fill value is not a
missing-value flag, and why chunking rounds up. The S3 page no longer
begins by saying this is impossible.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 7a880931-8be4-4c5c-a2cd-afa8e486f9e4

* Fix review findings on the Zarr documentation page

Remove branch-relative phrasing from the NCZarr section ("unchanged",
"now uses", "has long been able to") and from the optional-dependency
paragraph ("behaves exactly as before"): a reader with no memory of an
Iris before native Zarr support has nothing for those words to contrast
against. State both as plain facts about how Iris works instead.

Pin the install example to the version floor Iris actually enforces,
zarr >=3.0.8 per requirements/py3*.yml and the ImportError it raises,
so following the page's own instruction cannot leave a reader on a
version Iris still refuses.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 7a880931-8be4-4c5c-a2cd-afa8e486f9e4

* Record PR 4 in the spec, and what implementation changed

Closes open question Q2: fixed-length unicode arrays read as themselves on
both Zarr versions, so SciTools#6961's note describes the specification rather than
zarr-python. Logs the two decisions that move work between pull requests:
format lands here rather than with the saver, and the shared dataset
contract splits into read and write halves.

Corrects two findings implementation disproved. zarr-python validates
dimension_names against shape on read as well as on write, so a wrong-length
version 3 store cannot be opened at all and the case is reachable only on
version 2. The netCDF saver's refusal of a nested attribute names the
attribute, though not the cube.

Records two behaviours the design did not describe: _NCProperties is withheld
from a Zarr dataset's attributes, matching what netCDF4 has always hidden on
the netCDF path; and load_files keeps its glob-expanding contract as a wrapper
over a private form, because the load chain expands once before partitioning
files from stores.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 7a880931-8be4-4c5c-a2cd-afa8e486f9e4

* Give both Zarr load routes the same clear ImportError

Calling iris.fileformats.zarr.load_cubes directly - the documented route
for the group= keyword - raised a bare "No module named zarr" instead of
the documented message, and nothing checked zarr's version, so a zarr 2.x
install reached ZarrDataset and failed with an unrelated AttributeError
instead of a clear error naming the version floor.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 7a880931-8be4-4c5c-a2cd-afa8e486f9e4

* Fix the shared Zarr fixture's standing warning, and warn when a version 2
store's fill value will mask data

stores.py gave its grid mapping no ellipsoid, so every test that loaded
through it recorded a load problem and raised the warning that announces
one, burying the one signal that would reveal a genuine Zarr load problem
under a standing false positive; three independent reviews found this.

The version 2 fill-value masking rule itself is unchanged - the design
document's call to make, not this pass's - but nothing told a reader it
had applied, so a store missing a real zero because of it looked the same
as one that wasn't. Warn once per variable, from where the decode
parameters are chosen rather than the per-chunk decode layer, naming the
variable and the value.

Also: corrects a design-document sentence that had the storage-attribute
withholding backwards, records two deviations the implementation plan's
own log had missed, updates a generic docstring left describing the
bare-name matching this branch replaced, and corrects a comment that
blamed a non-existent import cycle for a deferred import that no longer
saves anything.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 7a880931-8be4-4c5c-a2cd-afa8e486f9e4

* Narrow the version 2 fill-value warning to a storage fill value of zero

The masking rule is unchanged, but a realistic round trip through
netCDF-c's NCZarr writer - a documented, supported path - raised the
previous warning fourteen times across ten variables in one load, because
NCZarr stamps its own pre-convention sentinel onto every variable
regardless of CF attributes. That sentinel is a writer's deliberate
choice, which is exactly the case design document section 4.4 keeps the
rule for; zero is the case it calls out as the implausible one, since it
is zarr-python's own default for a fill value nobody set. Warning only
when the adopted sentinel is zero keeps the signal without training
readers to ignore the whole class of warning.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 7a880931-8be4-4c5c-a2cd-afa8e486f9e4

* Updated lock files.

Session-Id: 9ef4e3ac-2bfa-4661-98fa-c932129b9c36

* Decode every sentinel in a vector missing_value

CF section 2.5.1 makes `missing_value` "a scalar or vector containing
values indicating missing data", and netCDF4 masks on every element of
one. The Zarr decoder passed the whole attribute to the scalar decoder,
so a JSON array warned and was dropped - and on a version 2 store the
variable then looked as though it declared no CF attribute at all, which
handed the storage fill value the sentinel role and masked every valid
zero.

`_FillValue` is still read as a scalar, which is what the same section
of the convention makes it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 1847daff-208b-4cdf-9506-90fe16f8a998

* Size Zarr chunks on the decoded dtype, not the stored one

A packed variable is unpacked block by block, on the way out of the
graph, so what a task holds is the decoded block. Sized on the stored
dtype, a (512, 512, 512) int8 array with float64 unpacking got one chunk
spanning the array: 128 MiB stored, 1 GiB decoded, against Dask's 128
MiB target.

The netCDF path reaches the same answer from the other end, its data
proxy declaring the decoded dtype. For a variable with no packing the
decoded dtype is the stored one, so nothing moves in the usual case.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 1847daff-208b-4cdf-9506-90fe16f8a998

* Carry the (name, size) rule through coordinate attachment

`CFVariable.spans` matches a dimension on (name, size), but the loading
rules then chose the cube dimension by name alone, with
`cf_var.dimensions.index(name)`. Given a Zarr data variable whose
dimensions are ('d', 'd') at sizes (2, 3) and a three-point coordinate
on 'd', `spans` said yes and `.index('d')` answered 0, so the coordinate
was rejected as the wrong length and recorded in LOAD_PROBLEMS - with
the axis it belonged on uniquely identifiable.

The identity rule becomes a function rather than a method, so that
`spans` and the four `_add_*` routines decide by one rule rather than
two that can drift apart.

No netCDF file can observe the difference: a netCDF dimension name is
unique within its group and carries one size. The mocked data variables
in the load-rule tests gain the shape they would always have had.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 1847daff-208b-4cdf-9506-90fe16f8a998

* Rule the Zarr store element out when a file handle is supplied

`requires_fh = False` means "consult me without a handle", not "consult
me with one". File dispatch passes `pathlib.Path(fn).name` - a basename,
not a path - so the store test resolved it against the working
directory: a NIMROD file named `weather` was classified as Zarr whenever
the working directory happened to hold a `weather/zarr.json`, the
priority-5 Zarr specification being consulted before priority-3 NIMROD.

A store is a directory or a URL, so a target that opened as a single
seekable file has settled the question already.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 1847daff-208b-4cdf-9506-90fe16f8a998

* Record the review findings in the spec

Four amendments, one per finding: the (name, size) rule carrying through
coordinate attachment and the identity rule becoming a function (4.4);
vector `missing_value` (4.4); chunks sized on the decoded dtype (4.4);
the store element ruled out when a file handle is supplied (4.7).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 1847daff-208b-4cdf-9506-90fe16f8a998

* Size as_dask() chunks on the decoded dtype too

`CHUNK_CONTROL.as_dask()` hands sizing to Dask, and `da.from_array`
sizes on the array it is given - the stored one - so the packed
variable's 8x survived there after the default path was fixed. Dask's
policy is now called directly, as `normalize_chunks("auto", shape,
dtype=decoded_dtype)`, and the result passed on with every dimension
fixed so that `as_lazy_data` cannot re-size it on the stored dtype.

The algorithm and the configured target are still Dask's; only the dtype
it budgets against is supplied rather than inferred. The (512, 512, 512)
packed int8 probe now gives (255, 255, 255) under `as_dask()`: 126.5 MiB
decoded, against the 128 MiB target.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: 1847daff-208b-4cdf-9506-90fe16f8a998

* Trim the Zarr docs to what a user acts on

Review on SciTools#7336 asked for the user manual to carry practical matters and
leave the rationale out. Cut the sections that only explained why, kept the
ones a reader hits, and corrected three statements that were wrong.

Dimension names: `_ARRAY_DIMENSIONS` is read, so version 2 dimensions are
named. The old "does not pass it on to the cube" meant only that the
attribute is withheld from `cube.attributes`, and read as though the
dimensions came out anonymous.

Dimension names: a name used at two lengths does not reach the user as the
warning the text promised. `ZarrDataset.dimensions` is a lazy property and
the CF layer reads per-variable `dimensions` instead, so nothing computes
it during a load. What a user does see is the orphaned coordinate in
LOAD_PROBLEMS, so say that.

Attributes: keep the netCDF save failure, including the error text verbatim,
because the netCDF library names the attribute and not the cube and a user
has nothing else to search for. Trimmed to the symptom and the fix, and
pointed at it from Saving.

Dropped the `_NCProperties` section, the version 3 justification for ignoring
the storage fill value, and the 9.96921e+36 example. All of that is in the
module docstrings of `_decode.py` and `_dataset.py`, where a developer looks.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: a1b138dd-45fa-4794-b53d-6b56018d4128

* Assemble the Zarr dimension mapping when the store is opened

A dimension name used at two different lengths warned from
`ZarrDataset.dimensions`, a lazy property. A load never read it: the CF
layer works from each variable's own `dimensions`, so nothing assembled the
mapping and the warning reached the unit tests and no user. Spying on
`_assemble_dimensions` through `iris.load` of a conflicting store counted
zero calls.

Assembling in `__init__` costs no extra reads, because `variables` is
materialised by everything that opens a store for loading, and it removes
the failure mode rather than papering over it: a warning raised from a lazy
property nobody triggers is unreachable by construction.

Without the mapping the clash still reaches the user, but only as the
orphaned coordinate in LOAD_PROBLEMS, which names the coordinate and not the
cause. The warning is the only thing that says why.

The unit test passed either way, because it read the property itself. Added
an integration test that loads a conflicting store through `iris.load`, which
is the path that was broken.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: a1b138dd-45fa-4794-b53d-6b56018d4128

* Action the local review points on the Zarr reader

`_DimensionIdentity` replaces the pair-or-sentinel that `_dimension_identities`
returned, so the function has one documented element type instead of
`tuple[str, int] | _AnonymousDimension`. Equality and hashing are now written
out: a named dimension compares on `(name, size)`, an anonymous one on
identity. `set.issubset`, `in` and `tuple.index` all reach them, so they are
pinned by their own tests.

`_store_chunk_exceeds_dask_target` moves above `_align_chunks`, its only
caller.

`ugrid_load` had two branches with identical bodies, one listing http schemes
and one `REMOTE_STORE_SCHEMES`. They are one branch over the union now. The
matching pair in `iris.loading` cannot merge the same way: those two call
different `iris.io` entry points, one of which takes no constraints.

`ZarrDataset.attributes` asserts the group is open before building
`_ZarrAttributes`, matching `encoding` next to it, and `_ZarrAttributes`
takes a typed node and says in its docstring that writes reach the store.

`_dimension_names` keeps two branches rather than gaining a third for an
unsupported version. zarr-python parses `zarr_format` as a `Literal` and
raises before Iris sees the node - `expected Literal[3], got 4` for an array
in a version 3 group - so a third branch would be unreachable. Said so at the
`else`, and pinned the guarantee with a test, which is what fails if
zarr-python ever widens it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: a1b138dd-45fa-4794-b53d-6b56018d4128

* Share the URI categoriser, and name the Zarr versions read

`iris.loading._generate_cubes` and `ugrid_load.load_meshes` each defined an
identical `_categorise` closure, so "what counts as an NCZarr target" was
written twice in two modules. It is `iris.io._categorise_uri` now, with its
own tests.

`decode_uri` keeps its signature. It is public, documented with doctests, and
a native Zarr store cannot be categorised from the URI anyway: remotely it is
the scheme, locally it is a directory that only a marker file tells from any
other, and reading that marker inside a string function would mean a stat
per decode.

`REMOTE_STORE_SCHEMES` becomes `_REMOTE_STORE_SCHEMES`. Neither the design
document nor the plan argues for it being public; it was written without an
underscore and nothing outside Iris reads it.

`_dimension_names` now names both versions it reads and raises on anything
else. zarr-python rejects such an array first, so the branch is reached only
through a stand-in in the tests, but naming 2 and 3 in the code is what a
reader new to Zarr needs - the previous `else` asked them to already know
there was no third option.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: a1b138dd-45fa-4794-b53d-6b56018d4128

* Drop chunk alignment; warn for both backends instead

Iris sized Dask chunks on the Zarr path and then rounded them back up to
whole multiples of the store's chunking. That is gone. Chunking is now
`_optimum_chunksize` and nothing further, the same as netCDF.

The measurements supported the alignment, so this is a proportionality call
rather than a correction. Against a 33.6 MB store chunk divided into
quarters, aligning held lower peak RSS at every worker count above one (305
MB against 446 MB at four workers), ran in half the time, and fetched each
store chunk 1.0 times against 3.2.

What decided it is that alignment only ever changed the answer in one case.
Wherever `_optimum_chunksize` expands it already lands on a whole multiple,
so alignment is a no-op; the only case it altered was a stored chunk larger
than Dask's target. netCDF passes the file's own chunking into that same
function and has always accepted the division, so correcting it for Zarr
alone made two formats behave differently over physics they share, and made
CHUNK_CONTROL mean different things depending on what was loaded.

The warning alignment carried is kept, because it is what makes leaving the
choice to the user a fair one: it says the situation exists and names the
tool. It lives in `iris.fileformats.cf.loader` now, on the path both
backends take, and stays quiet when the caller has pinned a chunking --
they have already made this decision. New tests run it over netCDF and Zarr
so the two cannot drift.

`test__align_chunks.py` goes. The read_data test that pinned rounding-up is
inverted to pin the division, with the reasoning in its docstring.

Recorded in spec section 4.4 and the decision log, which reverses the
2026-09-22 entry that added alignment, and in the plan's record of what
changed during implementation.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: a1b138dd-45fa-4794-b53d-6b56018d4128

* Action four more review points

`encoding` on the `CFDataset` interface becomes `storage_settings`, on both
the dataset and the variable, and `create_variable`'s `**encoding` with them.
Iris already uses "encoding" for character encoding -- `EncodedVariable`,
`VariableEncoder`, the `_Encoding` attribute, and a test fixture of that name
-- so the member was the odd one out. Neither class is in
`iris.fileformats.cf.__all__`, so nothing public moves.

`_ZarrAttributes` no longer documents which attributes are withheld. That is
the caller's decision, made where the mapping is built, and both constants
already carry the reason; the class docstring was a second copy that could
go stale on its own.

`_CUBELIST_SAVER_MODULES` becomes `_savers.cubelist_savers`, holding the
saver functions rather than module-name strings. Identity is what the check
actually wants: `save` is also reached with a saver that was never looked up
by extension -- one the caller passed, and the one chosen directly for an
NCZarr target -- and `netcdf.save` has no `append` parameter for those to
fall back to. `_check_init_savers()` is called before the check so the
registry is populated on those paths too. The test that pinned the module
set moves to `test_save.py`, which is where `iris.io.save` is tested.

Zarr tests use `mocker` rather than `monkeypatch`, which is what the
developer guide asks for and what 151 of the 168 test files that mock
already do. Every use here was a plain attribute swap.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: a1b138dd-45fa-4794-b53d-6b56018d4128

* Load UGRID meshes from Zarr, and action the rest of PR 4's review

iris.load on a UGRID Zarr store returned a cube that had silently dropped
its mesh coordinates: CFReader classifies them as UGRID coords, so the
ordinary aux-coord path skipped them and nothing built the mesh. mesh and
location were left on the cube as stray attributes. No warning, nothing in
LOAD_PROBLEMS. iris.mesh.load_meshes reached a store and then rejected it,
because the handler test named only nc_loader.load_cubes.

The design document (section 4.7) already called for load_meshes to accept
Zarr stores, so this is an implementation gap rather than a decision. The
capability was already present: _meshes_from_cf builds a correct MeshXY from
a ZarrDataset unmodified.

The inline mesh block in netcdf/loader.py becomes _prepare_mesh_coords and
_attach_mesh_coords in ugrid_load.py, called by both loaders. Two functions
rather than one because the MeshCoords must be built before _load_cube and
attached after it - built after, _add_unused_attributes has already taken
the variable's mesh and location attributes. The "mesh could not be found"
message keeps being raised on the iris.fileformats.netcdf logger, which
ugrid_load's own logger would drop at INFO.

Also from the review:

- read_data restructured to a single exit, and the duplicated as_lazy_data
  call hoisted out of both branches.
- _decode's surface narrowed to decode_parameters, decoding_required and
  mask_and_scale; every other name underscored, and the rule written into
  the module docstring.
- The CFDataset contract asserts the chunk shape under a per-backend
  chunking_key, rather than searching storage_settings for it by value.
  netCDF4 spells the key chunksizes, Zarr spells it chunks.
- zarr/stores.py imports the contract's constants instead of restating them.
- _dimension_names tested through ZarrDatasetVariable.__init__.
- test_one_cube_per_data_variable given three real data variables; the
  canonical store's second array is a grid mapping.
- A unit test for the constraint var_callback.
- Six test modules regrouped into one class per tested thing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: bd070a33-3348-4ab3-bd65-d7a1f9b60ae8

* Say why a netCDF-named module names Zarr

load_meshes carries its own copy of the format dispatch (design document
section 2.3), so every format the main load chain learns has to be taught to
this copy too - and the copy lives under iris/fileformats/netcdf/. The mesh
code that ugrid_load shares with the Zarr loader names no format at all; the
dispatch is the whole reason Zarr appears here.

Add a TODO naming the event that removes it, and record in the design
document that unifying the two cube-building loops takes this duplication
with it. Also fold two comments that were both saying "do not import
zarr eagerly" into the one that needed to.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Session-Id: bd070a33-3348-4ab3-bd65-d7a1f9b60ae8

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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
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
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
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
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
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
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
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
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
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
_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
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
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
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
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
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
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
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
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 SciTools#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
trexfeathers and others added 25 commits October 23, 2026 12:13
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
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
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
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
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
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
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
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
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
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
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
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
`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
`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
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
Session-Id: 22ea9714-39f5-4300-94ab-9c88882e2367
Session-Id: f1bc382d-935f-43e1-97e0-dffeeebf7912
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
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
Session-Id: 26369e2e-3668-4af6-93a8-a7d6184d78d7
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
Session-Id: 1361c342-bb59-4681-8387-ebda1615f2f3
Session-Id: 1361c342-bb59-4681-8387-ebda1615f2f3
Session-Id: 1361c342-bb59-4681-8387-ebda1615f2f3
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
SciTools#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 SciTools#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
@trexfeathers

Copy link
Copy Markdown
Owner Author

(posted by Claude Opus)

This preview has done its job: the real pull request is SciTools#7346, open against brownfield, and every comment here is answered in the commits it carries.

Closing rather than retargeting, because the diff above is now misleading. The branch has been rebased from zarr-io-pr-4-5-base onto brownfield now that SciTools#7336 and SciTools#7338 have both merged, so this pull request's base and head no longer share a sensible ancestor and GitHub would show PR 4's and PR 5's merged content as though it were this work.

Three things changed in that rebase beyond replaying the commits, all described in SciTools#7346's body: the generic saver is _CFSaver now, iris.io registers cubelist savers by identity rather than by module name, and the mesh round-trip test is no longer an xfail because UGRID meshes load from Zarr as of SciTools#7336.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants