Repository navigation
PREVIEW: Zarr I/O PR 6: native Zarr saving - #100
trexfeathers wants to merge 59 commits into
Conversation
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
left a comment
There was a problem hiding this comment.
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.
stephenworsley
left a comment
There was a problem hiding this comment.
Review covers cf/saver.py and mostly concerns a question of what belongs in cf vs zarr.
stephenworsley
left a comment
There was a problem hiding this comment.
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.
stephenworsley
left a comment
There was a problem hiding this comment.
A couple important cases which I think are lacking in test coverage at the moment.
* 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
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
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
9b7c202 to
9fd0c0a
Compare
|
(posted by Claude Opus) This preview has done its job: the real pull request is SciTools#7346, open against Closing rather than retargeting, because the diff above is now misleading. The branch has been rebased from Three things changed in that rebase beyond replaying the commits, all described in SciTools#7346's body: the generic saver is |
(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, withcompute=Falsereturning a realdask.delayed.Delayed, Zarr's own storage keywords (chunks,shards,compressors,filters,serializer), agroup=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 inzarr/_encode.pyandZarrDatasetVariable.__setitem__, and the fill-value declaration inzarr/saver.py's_backend_write, are where I would look first.Public API added.
iris.fileformats.zarr.saveandiris.fileformats.zarr.Saver, andiris.savenow reaches them for any target ending.zarr, local or remote, through a new"zarr"entry iniris.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'szarr/_dataset.pyand subclasses PR 5'sCFSaver. This pull request is opened againstzarr-io-pr-4-5-baserather thanbrownfield, 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 inbrownfield, this branch is rebased withgit rebase --onto brownfield zarr-io-pr-4-5-base zarr-io-pr-6and 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 onbrownfield— the same rule PRs 1 through 5 followed.The lock files are not regenerated.
xarrayis added to the test section ofrequirements/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, solib/iris/tests/integration/zarr/test_cross_reader.pyskips there. It passes locally:pytest lib/iris/tests/integration/zarr/test_cross_reader.py -vgives 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.
CFSavergains 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 returnscf_var.variable, the samenetCDF4.Variableobject 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 insidenetcdf.saver.saveand nothing else could call it — so on the Zarr path every cube global attribute was simply dropped. Withiris.FUTURE.save_split_attrs = True,CFSaver.writedeliberately writes no globals and leaves them to the caller: netCDF wrote them, Zarr did not, andtitle,institution,history,sourceandreferencesvanished from a Zarr save. In the legacy default mode it is thelocal_keyscomputation that was missing, so aCubeListwhose cubes disagreed about an attribute kept only the last cube's value where netCDF gave each data variable its own. The block is nowcf.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 — andlib/iris/tests/integration/test_netcdf__loadsaveattrs.pygives 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 reportlib/iris/fileformats/cf/saver.pyas their source rather thanlib/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_attrsstill defaults toFalse. 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 setsscale_factorandadd_offsetas 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.pynow has apackfunction, the write-side mirror of the existing_decode.unpack, andZarrDatasetVariable.__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 messagenetcdf/saver.pyalready 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, andiris.mesh.load_meshesstill filters to the netCDF handler, so the round trip test carries a strictxfailrather 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 ownlatitudewrites 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, withNPY_DISABLE_CPU_FEATURESset 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 intest_coord_systems.py, all one pre-existing fixture race — a module-scopedtmp_path_factoryfixture shared acrosspytest-xdistworkers — and not something this pull request touches: the file is byte-identical to its state onzarr-io-pr-4-5-base, and the same ten tests pass when the file is run on its own, without-n auto.