From 54a50499f287ad81c55cea3d616bf8b619c03206 Mon Sep 17 00:00:00 2001 From: Martin Yeo Date: Thu, 15 Oct 2026 13:27:51 +0000 Subject: [PATCH 01/78] Plan PR 6, stage 1: framing, constraints and file structure The parts of the plan that every task reads: what PR 6 delivers, the constraints copied verbatim from the design document, the scope boundary, how a branch over two unmerged pull requests gets rebased, nine findings that refine the spec, the file structure and the interface the tasks leave behind. Four decisions taken with the user and recorded as findings: grid-mapping writing gets a fifth hook rather than the regularisation Q8 defers (F2); _attribute_handlers moves here, in its own commit (F3); the write-side value conversions get their own module (F4); xarray goes in the requirement files but the lock files are regenerated by hand later (F5). Every zarr-python and xarray claim the write path leans on was re-checked against zarr 3.4.1 and xarray 2026.9.0 and reproduces (F6). Tasks follow in later stages. Review Focus is written last, when there are tasks to pin each line to. Co-Authored-By: Claude Opus 5 Session-Id: b8474d78-fdfa-43e5-b907-4bc3fe833fb3 --- .../plans/2026-10-15-zarr-io-pr6.md | 395 ++++++++++++++++++ 1 file changed, 395 insertions(+) create mode 100644 docs/superpowers/plans/2026-10-15-zarr-io-pr6.md diff --git a/docs/superpowers/plans/2026-10-15-zarr-io-pr6.md b/docs/superpowers/plans/2026-10-15-zarr-io-pr6.md new file mode 100644 index 0000000000..24d9d606a4 --- /dev/null +++ b/docs/superpowers/plans/2026-10-15-zarr-io-pr6.md @@ -0,0 +1,395 @@ +# Zarr I/O PR 6 — Zarr saving: Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +| | | +|---|---| +| **Spec** | [`docs/superpowers/specs/2026-09-21-zarr-io-design.md`](../specs/2026-09-21-zarr-io-design.md) §4.1, §4.2, §4.5, §4.6, §4.7, §5 (PR 6), §6 | +| **Implements** | Pull request 6 of 8, the subject of [#6980](https://github.com/SciTools/iris/issues/6980). `Part of #6980` — never a closing keyword. | +| **Branch** | `zarr-io-pr-6`, branched from `zarr-io-pr-4-5-base`, targeting `brownfield` | +| **Baseline** | `52c478bf9`, PR 4 ([#7336](https://github.com/SciTools/iris/pull/7336)) and PR 5 ([#7338](https://github.com/SciTools/iris/pull/7338)) merged together on a local branch, neither yet in `brownfield`; Iris `3.17.0.dev16`; zarr `3.4.1`; xarray `2026.9.0`; dask `2026.8.0`; numpy `2.5.2` | +| **Delivers** | `iris.save(cubes, "out.zarr")` and `iris.save(cubes, "s3://bucket/out.zarr")`, writing Zarr version 3 stores that xarray and every other Zarr reader can open | +| **Behaviour change** | New capability only. `iris.save(cube, "out.zarr")` raises `ValueError("Cannot save; no saver")` at the baseline, so nothing that works today changes meaning. The one edit to existing netCDF behaviour is a hook that leaves what lands in a netCDF file byte for byte unchanged (F2). | +| **Public API change** | `iris.fileformats.zarr.save` and `iris.fileformats.zarr.Saver` are new. `iris.fileformats.netcdf._attribute_handlers` moves to `iris.fileformats.cf._attribute_handlers`; it is private and has no deprecation obligation (F3). | + +**Goal:** Make `iris.save(cubes, "out.zarr")` write a CF-conforming Zarr version 3 store, by adding a write path to the `ZarrDataset` PR 4 built and a `Saver` subclassing the `CFSaver` PR 5 extracted. + +**Architecture:** PR 5 left `cf/saver.py` holding every format-agnostic part of saving a cube — dimension naming, coordinate, bounds, mesh, cell-measure and grid-mapping variables, attribute compliance, lazy write collection — driven through the `CFDataset` interface and four hooks. PR 4 left `zarr/_dataset.py` holding a read-only `ZarrDataset` whose write members raise `NotImplementedError`. This plan fills in those members, adds a fifth hook for grid mappings so that nothing in generic code reaches for a netCDF4 object, and adds `zarr/saver.py`: a `Saver` that opens a store, a `save()` that takes Zarr's own storage vocabulary, and a `delayed_completion()` that returns a real `Delayed` owning the final metadata consolidation. + +**Tech Stack:** Python 3.12–3.14, zarr-python ≥3.0.8, NumPy, Dask, xarray (test-only, for cross-reader conformance), pytest + pytest-mock, pre-commit (ruff, numpydoc, mypy, sort-all, and the repository's local hooks), Sphinx. + +--- + +## Global Constraints + +Every task's requirements implicitly include this section. + +| Constraint | Value, verbatim from its source | +|---|---| +| Target branch | `brownfield`, not `main` (spec §5) | +| PR labels | `Agentic` and `Type: Feature Branch` (spec §5) | +| Issue reference | `Part of #6980`. **Never** a closing keyword — the issue closes at merge-back (spec §5) | +| Changelog | No `changelog/` fragment in this PR. Fragments are written once at merge-back; the PR body must say so (spec §5) | +| GitHub posts | Every PR description, comment and review reply opens with `(posted by Claude Opus)`, before anything else. This **replaces** any trailing harness attribution (root `AGENTS.md`) | +| Commit messages | Out of scope for the above; they keep `Co-Authored-By:` trailers | +| Zarr version written | **Version 3 only.** "`ZarrDataset.__init__` takes `zarr_format: int = 3`. Today it raises `NotImplementedError` for anything but 3, in one place" (spec §4.5) | +| No version in generic code | "`cf/saver.py` never mentions a Zarr version. Everything version-specific lives in `ZarrDataset`" (spec §4.5) | +| Zarr's vocabulary, not netCDF's | "`compressors=`, `chunks=`, `shards=`, `filters=`, `serializer=` — the names zarr-python uses — plus `consolidate_metadata=True`. A netCDF keyword passed to the Zarr saver raises `TypeError` naming the Zarr equivalent where there is one, and saying there is none where there is not" (spec §4.5) | +| No keyword translation | "**Do not translate netCDF keywords onto Zarr codecs** — `zlib`/`complevel` onto Blosc or Gzip, `shuffle` onto Blosc shuffle, `chunksizes` onto `chunks`, `fletcher32` onto Crc32c" (spec §4.5) | +| Write-alignment invariant | "Every Zarr array Iris writes has a *write grid* that the lazy source's dask chunking tiles exactly, where the write grid is `Array.shards` when the array is sharded and `Array.chunks` otherwise. Each concurrent task owns at least one whole write-grid cell; no cell is touched by two tasks. A truncated final cell is permitted" (spec §4.5) | +| Create-once invariant | "Array metadata is written when the array is created and never edited afterwards. Iris does not resize, rechunk, or change the dtype, `fill_value`, codecs or dimension names of an array that already exists" (spec §4.5) | +| Default mode | "`"w-"`: create, and raise if the target already exists." "`mode="w"` opts back in to replacement; `mode="a"` adds to an existing store and is what `group=` needs. The error message names `mode`" (spec §4.5) | +| No append | "It never extends, rewrites, resizes or re-chunks an array that is already there... There is no `mode="a+"` and no `append=` keyword" (spec §4.5) | +| Attributes are JSON-native | "NumPy scalar to its Python equivalent, NumPy array to a list, `bytes` to `str` via ASCII, everything else unchanged. Anything still unserialisable raises with the attribute name in the message" (spec §4.5) | +| Converted before handing over | "Values are converted to JSON-native types **before** being handed to zarr, never after, because a rejected value poisons the attributes mapping" (spec §4.5, reproduced as F6) | +| No stringify-and-warn | "An attribute the format cannot hold is an error, and is not quietly turned into a string" (spec §4.5) | +| `_FillValue` encoding | floating: "base64 of `struct.pack("=3.0.8`, in `requirements/py3{12,13,14}.yml` and in `_MINIMUM_VERSION` (PR 4) | +| netCDF4 imports | No direct `import netCDF4`; always `iris.fileformats.netcdf._thread_safe_nc` (root `AGENTS.md`, enforced by the `check-netcdf4-imports` hook) | +| Copyright header | Every new Python file opens with the four-line BSD header (root `AGENTS.md`) | +| Line length | 88 characters (Ruff default) | +| Docstrings | NumPy style, strictly validated by the `numpydoc-validation` hook | +| Public module docstrings | Must open with a `z_reference` item within the first 25 lines (`lib/iris/AGENTS.md`, enforced by `check-docs-page-metadata`) | +| New module size | Aim under ~1000 lines (`lib/iris/AGENTS.md`). F4 is why `_encode.py` exists rather than more of `_dataset.py` | +| No network in tests | "**Remote stores** are covered by an in-memory store and a local directory store only. No test touches the network. The URL path is exercised by checking that the right store is constructed, not by fetching" (spec §6) | +| Test skipping | "A single `pytest.importorskip("zarr")` fixture in the Zarr test package. Iris without zarr installed must show no new failures" (spec §6) | +| Published prose | "must never cite an artefact of how the work was done — a review finding, a task or plan number, a phase" (`docs/AGENTS.md`). Cite the issue, the pull request or the design-document section instead | +| Warnings | Every `UserWarning` subclass inherits from `IrisUserWarning`; the `check-categorised-warnings` hook enforces categorisation | +| Before finishing | Run the repository's pre-commit hooks and fix everything they report (root `AGENTS.md`) | + +### Running anything + +The environment is the conda environment `iris-dev`. Either activate it, or prefix +commands with `conda run -n iris-dev`. Do **not** install or remove packages +without asking the user first — `xarray` is already installed there for this work +(F5). + +Set `NPY_DISABLE_CPU_FEATURES="AVX512F,AVX512CD,AVX512_SKX"` before running the +suite, per the root `AGENTS.md`. + +--- + +## 1. Scope boundary + +**In scope** (spec §5, PR 6): + +- `zarr/_dataset.py` gains the write half of `CFDataset`: opening for writing, + `create_dimension`, `create_variable`, writable attributes and `finalise`. +- `zarr/_encode.py`, new: the pure value conversions the write path needs (F4). +- `zarr/saver.py`, new: `Saver` and `save`. +- The save chain: a `zarr` entry in `iris.io._savers`, and + `iris.fileformats.zarr.saver` added to `_CUBELIST_SAVER_MODULES` (F8). +- One fifth hook on `CFSaver` so that grid-mapping parameters reach a store + that has no netCDF4 object behind it (F2). +- `netcdf/_attribute_handlers.py` moves to `cf/_attribute_handlers.py` (F3). +- `xarray` added to the test section of `requirements/py3{12,13,14}.yml` (F5). +- Documentation: saving in `docs/src/user_manual/explanation/zarr_io.rst` and + `docs/src/user_manual/tutorial/s3_io.rst`, both of which PR 4 created or + rewrote for loading. +- The §6 write tests: the write-alignment invariant on a sharded target, the + existing-store refusals, the Zarr-native encoding keywords, the masked + round trip, the deferred save, and the bidirectional cross-reader + conformance test. + +**Out of scope**, and each has a reason: + +- **Version 2 writing.** Spec §4.5: "Writing is version 3 only." The guard is + one `NotImplementedError` in `ZarrDataset.__init__`. +- **Appending to an array.** Spec §4.5 and §9. `mode="a"` adds a *new* array or + group and nothing else. +- **Multi-process writes.** Spec §4.5 and §9. The provisions that keep it + reachable are already in place; this plan adds none and removes none. +- **Regularising the netCDF grid-mapping attribute encoding.** Q8, spec §12.3: + "belongs in its own pull request". F2 is how this plan unblocks Zarr without + touching what lands in a netCDF file. +- **The netCDF `compute=False` return type.** Q5, [#7291](https://github.com/SciTools/iris/issues/7291). + The Zarr path returns a real `Delayed`; the netCDF path is left as it is. +- **The two-dimensional attribute check on the netCDF saver.** + [#7329](https://github.com/SciTools/iris/issues/7329). Spec §4.5 puts it + outside this programme. +- **A Zarr `save_mesh`.** Spec §5 asks for `save`, not for the mesh-only entry + point `iris.mesh.save_mesh` offers for netCDF. A cube that *has* a mesh saves + its mesh, because that is `CFSaver.write`'s job and it is backend-agnostic + already; what is missing is only the standalone "save this mesh and nothing + else" call. Adding it is a later, small pull request. +- **Real-world data and benchmarks.** PR 7. +- **Changelog fragments and closing keywords.** The merge-back owns both. +- **Regenerating `requirements/locks/*.lock`.** F5: the human developer does + this by hand, after PR 4 lands. + +--- + +## 2. Git strategy: a branch over two unmerged pull requests + +PR 6 needs both of its predecessors in one tree. It edits `zarr/_dataset.py`, +which exists only on PR 4, and it subclasses `CFSaver`, which exists only on +PR 5. Neither has merged. + +So `zarr-io-pr-4-5-base` carries a merge of the two, and `zarr-io-pr-6` +branches from it. The merge commit, `52c478bf9`, resolved five conflicts and +recorded two things neither branch could have had on its own; read it before +starting, because one of them is a gap that will reappear: + +> `ZarrDataset.supports_dtype`, the abstract method PR 5 adds to `CFDataset` +> after PR 4 wrote `ZarrDataset`. Both Zarr versions hold every NumPy numeric +> dtype, so it answers `True`. **Whichever of PR 4 and PR 5 rebases second must +> carry this**, or `ZarrDataset` stays abstract and every Zarr test errors. + +That method is therefore *not* part of PR 6's diff, even though PR 6's branch +contains it. The same goes for every other line of the merge commit. + +**When PR 4 and PR 5 have both landed in `brownfield`:** + +```bash +git fetch upstream brownfield +git rebase --onto upstream/brownfield zarr-io-pr-4-5-base zarr-io-pr-6 +``` + +That replays PR 6's own commits and drops the merge, which is why the merge is +a separate branch rather than the first commit of this one. Expect conflicts +only where review changed PR 4's `ZarrDataset` or PR 5's `CFSaver` hooks under +this work. + +**Opening the pull request before that rebase is possible** means GitHub would +show PR 4's and PR 5's diffs inside PR 6. Open it as a draft against +`brownfield` only once at least one predecessor has landed, or open it against +whichever predecessor branch is still outstanding and retarget later. The +pull-request body says which it is. + +--- + +## 3. Findings that refine the spec + +Numbered so that tasks can cite them. Each is something checked against the +code or against zarr-python 3.4.1 and xarray 2026.9.0, not something the spec +already says. + +**F1. The base is green, and the suite is the proof.** `11188 passed, 59 +skipped` across `lib/iris/tests/unit` and `lib/iris/tests/integration` on +`52c478bf9`. Any failure outside the files a task touches is a rebase artefact, +not a bug this plan introduced. + +**F2. Grid mappings need a fifth hook, and the body of the method does not +change.** `CFSaver._add_grid_mapping_to_dataset` binds +`grid_variable = cf_var_grid.variable` and then makes **63** plain attribute +assignments to it. `.variable` is netCDF4's own object; `ZarrDatasetVariable` +has no such member, so a Zarr save of any cube with a coord system raises +`AttributeError` today. Q8 refuses to regularise the encoding in this +programme, because routing those 63 through `.attributes` changes what lands in +a netCDF file — `crs_wkt` is `NC_STRING` while `grid_mapping_name` is +`NC_CHAR`. The resolution is one hook, `_grid_mapping_writer(cf_var)`, and one +changed line in the 230-line method. netCDF returns `cf_var.variable`, so its +output is unchanged byte for byte. The base returns an adapter whose +`__setattr__` writes into `cf_var.attributes`. Decided with the user, +2026-10-15. + +**F3. `_attribute_handlers` moves to `cf/`, in this pull request.** Spec §12.4 +assigns it to PR 6: `cf/saver.py` imports `ATTRIBUTE_HANDLERS` from the netCDF +package, which imports `cf.saver` back, and the cycle survives only because +`_attribute_handlers` imports nothing from `iris.fileformats.cf`. Two source +files and four test files name it. It is private — no deprecation alias is +owed. It is the first task, and its own commit, so that the relocation and the +behaviour work stay separately reviewable. Decided with the user, 2026-10-15. + +**F4. `zarr/_encode.py` is a new module the spec's §4.1 layout does not list.** +The write side needs the same kind of pure value conversion the read side has +in `_decode.py`: attributes to JSON-native types, `_FillValue` to its +dtype-dependent encoding, masked arrays to filled ones. Putting them in +`_dataset.py` would push a module that is already 780 lines before any write +code well past the ~1000-line aim in `lib/iris/AGENTS.md`, and would mix pure +functions in with the zarr-python calls, which is what makes `_decode.py` +unit-testable without a store. Decided with the user, 2026-10-15. + +**F5. xarray goes in the requirement files now; the lock files are the human +developer's.** Spec §6 puts xarray in with PR 6. PR 4 already regenerates all +three lock files to add `zarr`, so PR 6 regenerating them guarantees a conflict +and a second slow rebuild on rebase. So: `xarray` is added to the test section +of `requirements/py3{12,13,14}.yml` in this pull request, and +`requirements/locks/*.lock` is left alone for the human developer to regenerate +by hand once PR 4 has landed. The consequence to state in the pull-request +body: **until those locks are regenerated, CI has no xarray, so every +cross-reader conformance test skips.** It is installed in `iris-dev` — one +package, exactly as spec §6 measured — so the tests run locally while the work +is being done. Decided with the user, 2026-10-15. + +**F6. Every zarr-python and xarray fact the write path leans on reproduces on +today's versions.** Checked against zarr 3.4.1 and xarray 2026.9.0, +2026-10-15: + +| Claim | Result | +|---|---| +| `mode="w-"` on an existing store raises | `FileExistsError: Cannot create '' with mode 'w-' because it already contains data. Use mode 'w' to overwrite.` | +| Creating an array over an existing name raises | `zarr.errors.ContainsArrayError: An array exists in store ... at path 'data'.` | +| A non-JSON attribute value poisons the mapping | `TypeError: Object of type float32 is not JSON serializable`, **and** `dict(group.attrs)` afterwards still contains the rejected value | +| A floating `_FillValue` written as a plain JSON number | xarray's open fails for the **whole dataset**: `TypeError: Failed to decode fill_value: expected str or bytes for dtype float32, got float` | +| The same value written as base64 of `struct.pack(" None: ... + def create_variable( + self, + name: str, + dtype, + dimensions=(), + *, + fill_value=None, + chunks=None, # Zarr's own names, from here down + shards=None, + compressors="auto", + filters="auto", + serializer="auto", + **unsupported, # anything left raises TypeError + ) -> "ZarrDatasetVariable": ... + def finalise(self) -> None: ... # consolidate_metadata, once + +class ZarrDatasetVariable(CFDatasetVariable): + def __setitem__(self, keys, values) -> None: ... # encodes, then writes + def write_handle(self) -> "ZarrDatasetVariable": ... # self; F7 + + +# iris/fileformats/zarr/_encode.py + +def json_native(value, *, name: str): ... # NumPy scalar -> Python, array -> list, + # bytes -> ASCII str; TypeError names `name` +def encode_fill_value(value, dtype): ... # base64 str for floating, int for integer +def filled(data, fill_value): ... # masked -> plain, lazy stays lazy + + +# iris/fileformats/zarr/saver.py + +class Saver(CFSaver): + def __init__( + self, + filename, + compute: bool = True, + *, + mode: str = "w-", + group: str | None = None, + consolidate_metadata: bool = True, + ): ... + def _create_string_variable(self, cube_or_mesh, cf_name, element, element_dims, data): ... + def delayed_completion(self) -> Delayed: ... # a real Delayed, owning finalise() + +def save( + cube, # Cube or CubeList; F8 + filename, + *, + mode: str = "w-", + group: str | None = None, + consolidate_metadata: bool = True, + local_keys=None, + fill_value=None, + packing=None, + compute: bool = True, + **encoding, # chunks, shards, compressors, filters, serializer +): ... +# `unlimited_dimensions` is deliberately absent: it lands in **encoding and +# gets the TypeError that names why Zarr has no such thing. + + +# iris/fileformats/cf/saver.py + +class CFSaver: + def _grid_mapping_writer(self, cf_var): # F2; the fifth hook + """Return the object the 63 grid-mapping parameters are assigned to.""" +``` + +--- From c9c191acf9bfd3f84ff03b93af0b2f7b9c60f4a8 Mon Sep 17 00:00:00 2001 From: Martin Yeo Date: Thu, 15 Oct 2026 13:30:50 +0000 Subject: [PATCH 02/78] Plan PR 6, stage 2: the two changes to generic code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Session-Id: b8474d78-fdfa-43e5-b907-4bc3fe833fb3 --- .../plans/2026-10-15-zarr-io-pr6.md | 452 ++++++++++++++++++ 1 file changed, 452 insertions(+) diff --git a/docs/superpowers/plans/2026-10-15-zarr-io-pr6.md b/docs/superpowers/plans/2026-10-15-zarr-io-pr6.md index 24d9d606a4..7e5263a72c 100644 --- a/docs/superpowers/plans/2026-10-15-zarr-io-pr6.md +++ b/docs/superpowers/plans/2026-10-15-zarr-io-pr6.md @@ -393,3 +393,455 @@ class CFSaver: ``` --- + +## Task 1: `_attribute_handlers` moves to `cf/` + +Spec §12.4, the PR 5 entry: "Relocating `_attribute_handlers` is PR 6's, to keep +this relocation's overlap with the parallel Zarr loading work small." The module +serialises the three Iris-only attribute types that *every* CF store needs — +STASH, GRIB_PARAM, `ukmo__process_flags` — so its home is the CF package, and +moving it closes the one two-way import `cf/saver.py` still has. No behaviour +change: the module imports nothing from `iris.fileformats.cf`, and nothing it +exports is public API. + +It is first because it is the one part of this pull request reviewable without +reading a line of Zarr. + +**Files:** +- Move: `lib/iris/fileformats/netcdf/_attribute_handlers.py` → `lib/iris/fileformats/cf/_attribute_handlers.py` +- Move: `lib/iris/tests/unit/fileformats/netcdf/attribute_handlers/` → `lib/iris/tests/unit/fileformats/cf/attribute_handlers/` +- Modify: `lib/iris/fileformats/cf/saver.py:32-40` (module docstring), `:83` (the import) +- Modify: `lib/iris/fileformats/_nc_load_rules/actions.py:552` +- Modify: `lib/iris/tests/unit/fileformats/cf/test__imports.py` + +**Interfaces:** +- Consumes: nothing. +- Produces: `iris.fileformats.cf._attribute_handlers`, exporting + `AttributeHandler`, `StashHandler`, `UkmoProcessFlagsHandler`, + `GribParamHandler` and `ATTRIBUTE_HANDLERS`, all unchanged. + `iris.fileformats.netcdf._attribute_handlers` stops existing; it is private, + so spec §4.8 gives it no alias. + +- [ ] **Step 1: Move the tests, and repoint their imports** + +```bash +git mv lib/iris/tests/unit/fileformats/netcdf/attribute_handlers \ + lib/iris/tests/unit/fileformats/cf/attribute_handlers +``` + +In all three test modules, change the import: + +```python +from iris.fileformats.cf._attribute_handlers import ATTRIBUTE_HANDLERS +``` + +and the docstring reference that names the old module, in each of +`test_StashHandler.py`, `test_UkmoProcessFlagsHandler.py` and +`test_GribParamHandler.py`: + +```python +"""Unit tests for :class:`iris.fileformats.cf._attribute_handlers.StashHandler`.""" +``` + +`test_GribParamHandler.py`'s docstring names `StashHandler` and always has; +leave that alone, or fix it in passing — it is one word and nobody is served by +a separate pull request for it. + +The package `__init__.py` docstring currently reads "the +:mod:`iris.fileformats._attribute_handlers` module", which names neither home +correctly. Make it: + +```python +"""Unit tests for the :mod:`iris.fileformats.cf._attribute_handlers` module.""" +``` + +- [ ] **Step 2: Run them, and watch them fail** + +```bash +pytest lib/iris/tests/unit/fileformats/cf/attribute_handlers -x +``` + +Expected: collection error, `ModuleNotFoundError: No module named +'iris.fileformats.cf._attribute_handlers'`. + +- [ ] **Step 3: Move the module** + +```bash +git mv lib/iris/fileformats/netcdf/_attribute_handlers.py \ + lib/iris/fileformats/cf/_attribute_handlers.py +``` + +Replace the first line of the module docstring, which calls these translations +netCDF's. Nothing else in the file changes — not the classes, not the registry, +not `from iris.fileformats.pp import STASH`: + +```python +"""Attribute translations for Iris attributes with special convenience types. + +These are things which are stored differently in an Iris cube attribute from how they +are actually stored in a file. E.G. a STASH code is stored as a special object, +but in a file it is just a string. +``` + +Leave the rest of that docstring, including the list of the three handlers, +exactly as it is. + +- [ ] **Step 4: Repoint the library imports** + +`lib/iris/fileformats/cf/saver.py:83` becomes a relative import, because it is +now a sibling: + +```python +from ._attribute_handlers import ATTRIBUTE_HANDLERS +``` + +Place it with the other relative imports, where ruff's sorting wants it. + +`lib/iris/fileformats/_nc_load_rules/actions.py:552` keeps its deferred form — +it is inside a function, and the comment above it says why — and changes only +its path: + +```python + from iris.fileformats.cf._attribute_handlers import ATTRIBUTE_HANDLERS +``` + +- [ ] **Step 5: Rewrite the paragraph in `cf/saver.py` that the move makes untrue** + +Lines 32-40 of the module docstring describe a two-way import that no longer +exists. Replace that whole paragraph with: + +```python +``ATTRIBUTE_HANDLERS`` lives beside this module, in +:mod:`iris.fileformats.cf._attribute_handlers`. The handlers serialise the +Iris-only attribute types that every CF store needs - STASH, GRIB_PARAM, +ukmo__process_flags - and the netCDF load rules read them from there too. +``` + +The surrounding paragraphs stay. The grid-mapping one goes in Task 2. + +- [ ] **Step 6: Add the new module to the import-order test** + +`lib/iris/tests/unit/fileformats/cf/test__imports.py` lists nine modules. Add a +tenth, and a sentence saying what is new about this one: + +```python +MODULES = [ + "iris.fileformats.cf", + "iris.fileformats.cf.dataset", + "iris.fileformats.cf.loader", + "iris.fileformats.cf.saver", + "iris.fileformats.cf._attribute_handlers", + "iris.fileformats.cf._dask_locks", + "iris.fileformats.netcdf", + "iris.fileformats.netcdf._dataset", + "iris.fileformats.netcdf.loader", + "iris.fileformats.netcdf.saver", +] +``` + +and append to the module docstring: + +```python +``_attribute_handlers`` imports :mod:`iris.fileformats.pp`, so the CF package +now reaches the PP package while the ``iris.fileformats`` package is itself +part-way through importing. That was already true of the netCDF package, which +is where this module used to live and which ``iris.fileformats.__init__`` +imports before ``pp``. +``` + +- [ ] **Step 7: Run everything that names the handlers** + +```bash +pytest lib/iris/tests/unit/fileformats/cf \ + lib/iris/tests/unit/fileformats/netcdf \ + lib/iris/tests/unit/fileformats/nc_load_rules \ + lib/iris/tests/integration/netcdf \ + lib/iris/tests/unit/fileformats/pp -q -n auto +``` + +Expected: all pass. `test_GribParamHandler.py` skips without `iris_grib`, as it +does today. + +Then check nothing anywhere still names the old path: + +```bash +grep -rn "netcdf._attribute_handlers\|netcdf import _attribute_handlers" lib/ docs/ benchmarks/ +``` + +Expected: no output. + +- [ ] **Step 8: Commit** + +```bash +git add -A +git commit -m "Move the CF attribute handlers into the CF package" +``` + +--- + +## Task 2: Grid-mapping parameters reach a store that is not netCDF + +F2, and Q8 in spec §12.3. `CFSaver._add_grid_mapping_to_dataset` binds +`grid_variable = cf_var_grid.variable` at `cf/saver.py:1883` and then makes 63 +plain attribute assignments to it. `.variable` is netCDF4's object; a Zarr +store has none, so every cube with a coordinate system raises `AttributeError` +there. Q8 refuses to regularise the encoding in this programme, because routing +those 63 through `.attributes` changes what lands in a netCDF file. + +So: one hook, one changed line, and the netCDF path keeps writing to the very +same object it writes to today. + +**Files:** +- Modify: `lib/iris/fileformats/cf/saver.py:25-31` (docstring), `:1883`, and a new method beside the other hooks +- Modify: `lib/iris/fileformats/netcdf/saver.py` (the override, beside `_patch_dataset`) +- Create: `lib/iris/tests/unit/fileformats/cf/saver/test_grid_mapping_writer.py` + +**Interfaces:** +- Consumes: nothing from Task 1. +- Produces: `CFSaver._grid_mapping_writer(cf_var) -> object`, the fifth hook. + The base returns `_GridMappingAttributeWriter(cf_var)`, whose `__setattr__` + writes `cf_var.attributes[name] = value`. `netcdf.saver.Saver` overrides it to + return `cf_var.variable`, unchanged. Task 10's Zarr saver inherits the base + and implements nothing. + +- [ ] **Step 1: Write the failing tests** + +Create `lib/iris/tests/unit/fileformats/cf/saver/test_grid_mapping_writer.py`: + +```python +# Copyright Iris contributors +# +# This file is part of Iris and is released under the BSD license. +# See LICENSE in the root of the repository for full licensing details. +"""Unit tests for :meth:`iris.fileformats.cf.saver.CFSaver._grid_mapping_writer`. + +The hook exists because the 63 grid-mapping parameters are written by plain +attribute assignment, and netCDF4's own object is what receives them today. +Changing where they land would change what is in a netCDF file, which Q8 of +the design document rules out of this programme, so netCDF keeps that object +and everything else gets a writer that uses the ``attributes`` mapping. +""" + +import numpy as np +import pytest + +import iris +from iris.coord_systems import GeogCS +from iris.cube import Cube +from iris.fileformats.cf.saver import CFSaver, _GridMappingAttributeWriter +from iris.fileformats.netcdf._thread_safe_nc import DatasetWrapper + + +@pytest.fixture +def cube(): + cube = Cube(np.arange(6, dtype=np.float32).reshape(2, 3), var_name="air") + cube.add_dim_coord( + iris.coords.DimCoord( + np.array([0.0, 1.0]), + standard_name="latitude", + units="degrees", + coord_system=GeogCS(6371229.0), + ), + 0, + ) + cube.add_dim_coord( + iris.coords.DimCoord( + np.array([0.0, 1.0, 2.0]), + standard_name="longitude", + units="degrees", + coord_system=GeogCS(6371229.0), + ), + 1, + ) + return cube + + +class TestTheWriterItself: + def test_it_writes_through_the_attributes_mapping(self): + recorded = {} + + class FakeVariable: + attributes = recorded + + writer = _GridMappingAttributeWriter(FakeVariable()) + writer.earth_radius = 6371229.0 + writer.horizontal_datum_name = "OSGB 1936" + + assert recorded == { + "earth_radius": 6371229.0, + "horizontal_datum_name": "OSGB 1936", + } + + def test_it_does_not_grow_members_of_its_own(self): + # Every assignment must reach the variable. An attribute that stuck to + # the writer instead would be silently dropped from the file. + class FakeVariable: + attributes: dict = {} + + writer = _GridMappingAttributeWriter(FakeVariable()) + writer.anything = 1 + assert "anything" not in vars(writer) + + +class TestTheNetCDFOverride: + def test_it_is_the_netcdf4_variable_itself(self, tmp_path, cube): + # Not "writes the same values": the same object, so the 63 assignments + # take the identical path they take today and the file cannot change. + from iris.fileformats.netcdf.saver import Saver + + with Saver(tmp_path / "out.nc", "NETCDF4") as saver: + cf_var = saver._dataset.create_variable("probe", np.int32) + assert saver._grid_mapping_writer(cf_var) is cf_var.variable + + +class TestTheFileIsUnchanged: + def test_grid_mapping_attributes_land_where_they_did(self, tmp_path, cube): + path = tmp_path / "out.nc" + iris.save(cube, path) + + with DatasetWrapper(path) as dataset: + grid = dataset.variables["latitude_longitude"] + assert grid.getncattr("grid_mapping_name") == "latitude_longitude" + assert grid.getncattr("earth_radius") == pytest.approx(6371229.0) + assert grid.getncattr("longitude_of_prime_meridian") == pytest.approx(0.0) +``` + +- [ ] **Step 2: Run them, and watch them fail** + +```bash +pytest lib/iris/tests/unit/fileformats/cf/saver/test_grid_mapping_writer.py -x +``` + +Expected: collection error, `ImportError: cannot import name +'_GridMappingAttributeWriter' from 'iris.fileformats.cf.saver'`. + +- [ ] **Step 3: Add the writer and the hook** + +In `lib/iris/fileformats/cf/saver.py`, above `class CFSaver`: + +```python +class _GridMappingAttributeWriter: + """An assignment target that writes into a variable's attributes. + + :meth:`CFSaver._add_grid_mapping_to_dataset` sets sixty-three CF + grid-mapping parameters by plain attribute assignment. On netCDF those + assignments go to the netCDF4 variable itself and have done since long + before the CF layer existed, and that decides how each one is typed in + the file. Regularising it is its own piece of work -- see Q8 in §12.3 of + the native Zarr I/O design, + ``docs/superpowers/specs/2026-09-21-zarr-io-design.md``. Every other store + needs them to arrive in the ``attributes`` mapping instead, and this is the + object that puts them there. + + """ + + def __init__(self, cf_var): + # Bypass this class's own __setattr__, which would otherwise write + # the variable into the variable. + object.__setattr__(self, "_cf_var", cf_var) + + def __setattr__(self, name, value): + """Write one grid-mapping parameter into the variable's attributes.""" + self._cf_var.attributes[name] = value +``` + +and, beside the other hooks on `CFSaver` — next to `_backend_write`, which is +the nearest neighbour in kind: + +```python + def _grid_mapping_writer(self, cf_var): + """Return the object the grid-mapping parameters are assigned to. + + Parameters + ---------- + cf_var : :class:`~iris.fileformats.cf.dataset.CFDatasetVariable` + The grid-mapping variable, newly created and otherwise empty. + + Returns + ------- + object + Something that accepts a plain attribute assignment per CF + grid-mapping parameter. Here, a writer onto ``cf_var.attributes``. + A store that types those attributes some other way overrides this. + + """ + return _GridMappingAttributeWriter(cf_var) +``` + +- [ ] **Step 4: Change the one line in the method** + +`cf/saver.py:1883`, and the comment above it that explains the netCDF4 reach: + +```python + # The 63 assignments below decide how each parameter is typed in the + # file, and on netCDF they go to the netCDF4 variable itself, exactly + # as they always have; see Q8 in the design document. Every other + # store gets a writer onto `.attributes`. + grid_variable = self._grid_mapping_writer(cf_var_grid) +``` + +Nothing else in the 230-line method changes. + +- [ ] **Step 5: Add the netCDF override** + +In `lib/iris/fileformats/netcdf/saver.py`, beside `_patch_dataset`: + +```python + def _grid_mapping_writer(self, cf_var): + """Return the netCDF4 variable itself, which is what has always received these. + + Assignments to a netCDF4 variable bypass the ASCII-to-bytes coercion + that ``.attributes`` applies, so ``crs_wkt`` is written as + ``NC_STRING`` while ``grid_mapping_name`` is ``NC_CHAR``. Regularising + that is Q8 in §12.3 of the native Zarr I/O design, + ``docs/superpowers/specs/2026-09-21-zarr-io-design.md``; until then, + this returns the same object the parameters have always been written + to, so the file is unchanged. + + Parameters + ---------- + cf_var : :class:`~iris.fileformats.netcdf._dataset.NetCDFDatasetVariable` + The grid-mapping variable. + + Returns + ------- + :class:`netCDF4.Variable` + + """ + return cf_var.variable +``` + +- [ ] **Step 6: Rewrite the docstring paragraph that said this was impossible** + +`cf/saver.py:25-31` currently tells a reader that a non-netCDF subclass fails on +grid mappings. Replace that paragraph with: + +```python +``_add_grid_mapping_to_dataset`` writes the CF grid-mapping parameters by plain +attribute assignment, to whatever ``_grid_mapping_writer`` hands back. netCDF +hands back its own variable, because what that does to the type of each +attribute in the file is long-standing behaviour and not this module's to +change -- see Q8 in §12.3 of the native Zarr I/O design, +``docs/superpowers/specs/2026-09-21-zarr-io-design.md``. Every other store gets +a writer onto the ``attributes`` mapping. +``` + +- [ ] **Step 7: Run the tests** + +```bash +pytest lib/iris/tests/unit/fileformats/cf/saver/test_grid_mapping_writer.py -v +pytest lib/iris/tests/unit/fileformats/netcdf \ + lib/iris/tests/integration/netcdf -q -n auto +``` + +Expected: the new file passes, and the netCDF suites are unchanged — the +grid-mapping save tests are the ones that matter, and there are many. + +- [ ] **Step 8: Commit** + +```bash +git add -A +git commit -m "Let grid-mapping parameters reach a store that is not netCDF" +``` + +--- From ead323b4f74e2a2b5cb8c1e73a697a421d508ec2 Mon Sep 17 00:00:00 2001 From: Martin Yeo Date: Thu, 15 Oct 2026 13:39:05 +0000 Subject: [PATCH 03/78] Plan PR 6, stage 3: four more findings, and the encoding module Task 3 adds zarr/_encode.py: JSON-native attribute values, the _FillValue and missing_value encodings, the default fill table and masked-array filling. Pure functions, so the task tests them on plain values with no store in sight. Four findings behind it, each checked against zarr 3.4.1 and xarray 2026.9.0 rather than assumed: - missing_value must be a plain number. Written as base64, like _FillValue, xarray opens the store and masks nothing (F10). - zarr's default fill value is 0 for every dtype, floats included, so a masked cube saved with no fill_value would write zeros and declare nothing. Iris writes netCDF's default fills instead, and always declares them as _FillValue (F11). Decided with the user. - create_variable never sees the data, so the spec's "derive the write grid from the source's chunking" cannot happen there. The invariant is enforced by rechunking at write time instead, with the data variable's chunks defaulted from the cube (F12). - A masked array written straight to a zarr array raises inside numpy.ma, so filling is not an optimisation (F13). Co-Authored-By: Claude Opus 5 Session-Id: b8474d78-fdfa-43e5-b907-4bc3fe833fb3 --- .../plans/2026-10-15-zarr-io-pr6.md | 554 ++++++++++++++++++ 1 file changed, 554 insertions(+) diff --git a/docs/superpowers/plans/2026-10-15-zarr-io-pr6.md b/docs/superpowers/plans/2026-10-15-zarr-io-pr6.md index 7e5263a72c..9070538914 100644 --- a/docs/superpowers/plans/2026-10-15-zarr-io-pr6.md +++ b/docs/superpowers/plans/2026-10-15-zarr-io-pr6.md @@ -261,6 +261,74 @@ separator, so `iris.save(cube, "out.zarr/")` works. What is missing is only the `_savers` entry that maps the suffix to a module — nothing in the dispatch needs further teaching. +**F10. `missing_value` must be a plain number, and only `_FillValue` is +base64.** Spec §4.5 exempts "these two attributes" together from the JSON-native +conversion and gives one dtype-dependent rule. Written that way, the two +attributes behave differently in xarray, checked 2026-10-15: + +| Attribute, on a `float32` array | xarray's reading | +|---|---| +| `_FillValue` as base64 of `struct.pack(" Any` — a JSON-serialisable value, or + `TypeError` naming `name`. + - `encode_fill_value(value, dtype: np.dtype) -> str | int | float` — base64 + for floating, a plain number otherwise. + - `encode_missing_values(value, dtype: np.dtype) -> float | int | list` — plain + numbers, scalar or vector (F10). + - `default_fill_value(dtype: np.dtype) -> np.generic` — netCDF's default fill + for that dtype (F11). + - `filled(data, fill_value)` — a plain array, lazily if `data` was lazy (F13). + - `DEFAULT_FILL_VALUES: dict[str, Any]`, keyed by `dtype.str[1:]`. + +- [ ] **Step 1: Write the failing tests** + +Create `lib/iris/tests/unit/fileformats/zarr/test__encode.py`: + +```python +# Copyright Iris contributors +# +# This file is part of Iris and is released under the BSD license. +# See LICENSE in the root of the repository for full licensing details. +"""Unit tests for :mod:`iris.fileformats.zarr._encode`.""" + +import base64 +import struct + +import dask.array as da +import numpy as np +import pytest + +from iris.fileformats.zarr._encode import ( + DEFAULT_FILL_VALUES, + default_fill_value, + encode_fill_value, + encode_missing_values, + filled, + json_native, +) + + +def unpack(encoded): + """Return the float in a base64 little-endian float64.""" + return struct.unpack(" Any: + """Return `value` as something zarr's JSON encoder accepts. + + Parameters + ---------- + value : Any + The attribute value, as the cube holds it. + name : str + The attribute's name, for the error message. A failure here is a + value the user wrote, so the message has to say which one. + + Returns + ------- + Any + A JSON-serialisable value: NumPy scalars become Python ones, NumPy + arrays become lists, ``bytes`` become ``str`` via ASCII, everything + else is returned unchanged. + + Raises + ------ + TypeError + If the value cannot be stored. It is **not** converted to its string + form: a string that cannot be read back is a worse answer than a + refusal, and a warning would not stop the save. + + """ + if isinstance(value, bytes): + try: + converted = value.decode("ascii") + except UnicodeDecodeError as exception: + message = ( + f"Cannot save attribute {name!r}: its bytes are not ascii, " + "and a Zarr attribute is text." + ) + raise TypeError(message) from exception + elif isinstance(value, np.generic): + converted = value.item() + elif isinstance(value, np.ndarray): + converted = value.tolist() + else: + converted = value + + try: + json.dumps(converted) + except TypeError as exception: + message = ( + f"Cannot save attribute {name!r}: a value of type " + f"{type(value).__name__} has no JSON form. Convert it to a number, " + "a string, or a list of either." + ) + raise TypeError(message) from exception + + return converted + + +def encode_fill_value(value: Any, dtype: np.dtype) -> Any: + """Return a ``_FillValue`` attribute in the encoding readers expect. + + Parameters + ---------- + value : Any + The sentinel, in any numeric form. + dtype : numpy.dtype + The array's dtype, which decides the encoding. + + Returns + ------- + str or int or float + For a floating array, the base64 of the little-endian ``float64`` + form - including for ``float32``, which is what released xarray + reads. For anything else, a plain JSON value. + + """ + if dtype.kind == "f": + packed = struct.pack(_PACKED_DOUBLE, float(value)) + return base64.b64encode(packed).decode("ascii") + return json_native(value, name="_FillValue") + + +def encode_missing_values(value: Any, dtype: np.dtype) -> Any: + """Return a ``missing_value`` attribute as plain JSON numbers. + + Never base64, whatever the dtype: xarray reads this attribute as an + ordinary value, so a packed string there opens cleanly and masks nothing. + CF section 2.5.1 allows a vector here, so a sequence stays a sequence. + + Parameters + ---------- + value : Any + One sentinel, or a sequence of them. + dtype : numpy.dtype + The array's dtype. Unused, and named so that the two encoders have + one signature: a caller choosing between them should not have to + learn which one wants the dtype. + + Returns + ------- + int or float or list + + """ + del dtype + return json_native(value, name="missing_value") + + +def default_fill_value(dtype: np.dtype) -> np.generic: + """Return the sentinel to use for a masked source that declares none. + + Parameters + ---------- + dtype : numpy.dtype + The array's dtype. + + Returns + ------- + numpy.generic + The netCDF default fill for that dtype, as a value of it. + + Raises + ------ + ValueError + For a dtype with no entry - a complex or structured array, say, + which CF cannot express either. + + """ + key = dtype.str[1:] + if key not in DEFAULT_FILL_VALUES: + message = ( + f"No default fill value for dtype {dtype}, so masked data of that " + "type cannot be saved to Zarr. Pass an explicit 'fill_value'." + ) + raise ValueError(message) + return dtype.type(DEFAULT_FILL_VALUES[key]) + + +def filled(data, fill_value): + """Return `data` with any masked points replaced by `fill_value`. + + A Zarr array stores the fill in the data, as spec §4.5 settles: no + separate mask array. Writing a masked array to one raises inside numpy, + so this is not optional. + + Parameters + ---------- + data : numpy.ndarray or dask.array.Array + Real or lazy, masked or not. + fill_value : Any + What replaces the masked points. + + Returns + ------- + numpy.ndarray or dask.array.Array + Lazy if `data` was lazy. An unmasked input is returned unchanged, + not copied. + + """ + if is_lazy_data(data): + return da.ma.filled(data, fill_value) + if np.ma.isMaskedArray(data): + return data.filled(fill_value) + return data +``` + +Note the `import json` that `json_native` needs: put it with the other imports +at the top, not inside the function. The test imports it locally only because a +test may. + +- [ ] **Step 4: Run the tests** + +```bash +pytest lib/iris/tests/unit/fileformats/zarr/test__encode.py -v +``` + +Expected: all pass. + +- [ ] **Step 5: Check it against xarray, once, by hand** + +The encodings exist to satisfy a reader that is not Iris, so look at one with +your own eyes before building on them: + +```python +import base64, struct, numpy as np, xarray as xr, zarr +from iris.fileformats.zarr._encode import encode_fill_value + +group = zarr.open_group(store="/tmp/check.zarr", mode="w", zarr_format=3) +array = group.create_array( + "data", shape=(3,), dtype="f4", fill_value=np.float32(-99.0), + dimension_names=("x",), +) +array[:] = np.array([1.0, -99.0, 3.0], dtype="f4") +array.attrs["_FillValue"] = encode_fill_value(np.float32(-99.0), np.dtype("f4")) +print(xr.open_zarr("/tmp/check.zarr", consolidated=False)["data"].values) +``` + +Expected: `[ 1. nan 3.]`. If instead the open raises `TypeError: Failed to +decode fill_value`, the encoder is writing a plain number and Task 14's +conformance test would have caught it later, painfully. + +- [ ] **Step 6: Commit** + +```bash +git add lib/iris/fileformats/zarr/_encode.py \ + lib/iris/tests/unit/fileformats/zarr/test__encode.py +git commit -m "Add the Zarr write-side value conversions" +``` + +--- From a04f7c3cdaa17fc1547be43049f3ba2e85852a27 Mon Sep 17 00:00:00 2001 From: Martin Yeo Date: Thu, 15 Oct 2026 13:43:00 +0000 Subject: [PATCH 04/78] Plan PR 6, stage 4: opening for writing, and creating arrays Task 4 opens the four write modes, defends the w- default that makes a clobber deliberate, and puts the version 3 guard in one place. Task 5 turns the two creation calls CFSaver makes into Zarr arrays: a declared dimension is a dict entry, an array carries its dimension names, and a name that already exists is refused rather than overwritten. The keyword table is here too. A netCDF storage keyword raises and names the Zarr replacement where there is one, and says there is none where there is not - which is what stops the translation layer the spec rules out from growing back by accident. The fill value a create gets when the caller names none is netCDFs default for the dtype, and no _FillValue attribute goes with it. Who declares the attribute, and when, is Task 11: it has to happen before the metadata is consolidated, and only the saver knows the data is masked. Co-Authored-By: Claude Opus 5 Session-Id: b8474d78-fdfa-43e5-b907-4bc3fe833fb3 --- .../plans/2026-10-15-zarr-io-pr6.md | 688 ++++++++++++++++++ 1 file changed, 688 insertions(+) diff --git a/docs/superpowers/plans/2026-10-15-zarr-io-pr6.md b/docs/superpowers/plans/2026-10-15-zarr-io-pr6.md index 9070538914..eb3f617950 100644 --- a/docs/superpowers/plans/2026-10-15-zarr-io-pr6.md +++ b/docs/superpowers/plans/2026-10-15-zarr-io-pr6.md @@ -1399,3 +1399,691 @@ git commit -m "Add the Zarr write-side value conversions" ``` --- + +## Task 4: Open a Zarr store for writing + +Spec §4.5 and §4.6. PR 4's `ZarrDataset.__init__` accepts `mode="r"` and +nothing else. This opens the other four, defends the default that makes a +clobber deliberate, and refuses to write any version but 3 — in one place, so +that adding version 2 writing later is that guard and the three version-specific +points §4.5 lists. + +**Files:** +- Modify: `lib/iris/fileformats/zarr/_dataset.py` — `_READ_MODES` and the + `__init__` guard, around `:592-648` +- Create: `lib/iris/tests/unit/fileformats/zarr/dataset/test_ZarrDataset__write.py` + +**Interfaces:** +- Consumes: nothing from Tasks 1-3. +- Produces: `ZarrDataset(location, mode="r", *, group=None, zarr_format=3)`, + accepting `"r"`, `"r+"`, `"a"`, `"w"` and `"w-"`. `self._declared_dimensions: + dict[str, int]`, which Task 5 fills. The module constants `_WRITE_MODES`, + `_MODES` and `_WRITABLE_ZARR_FORMAT`. + +- [ ] **Step 1: Write the failing tests** + +Create `lib/iris/tests/unit/fileformats/zarr/dataset/test_ZarrDataset__write.py`: + +```python +# Copyright Iris contributors +# +# This file is part of Iris and is released under the BSD license. +# See LICENSE in the root of the repository for full licensing details. +"""Unit tests for the write half of :class:`iris.fileformats.zarr._dataset.ZarrDataset`.""" + +import numpy as np +import pytest + +zarr = pytest.importorskip("zarr") + +from iris.fileformats.zarr._dataset import ZarrDataset + + +@pytest.fixture +def existing(tmp_path): + """Return the path of a store that already holds one array.""" + path = tmp_path / "existing.zarr" + group = zarr.open_group(store=path, mode="w-", zarr_format=3) + array = group.create_array( + "already_here", shape=(2,), dtype="f4", dimension_names=("x",) + ) + array[:] = np.array([1.0, 2.0], dtype="f4") + return path + + +class TestOpening: + def test_a_new_store_is_created(self, tmp_path): + with ZarrDataset(tmp_path / "new.zarr", mode="w-") as dataset: + assert dataset.mode == "w-" + assert dataset.format == "ZARR_V3" + assert (tmp_path / "new.zarr" / "zarr.json").is_file() + + def test_the_default_refuses_an_existing_store(self, existing): + # The default differs from the netCDF saver's deliberately: a Zarr + # clobber is many non-atomic deletes, so it is opted into. + with pytest.raises(FileExistsError, match="mode"): + ZarrDataset(existing, mode="w-") + + def test_the_refusal_names_the_way_out(self, existing): + with pytest.raises(FileExistsError, match="'w'"): + ZarrDataset(existing, mode="w-") + + def test_w_replaces_an_existing_store(self, existing): + with ZarrDataset(existing, mode="w") as dataset: + assert list(dataset.variables) == [] + + def test_a_adds_to_an_existing_store(self, existing): + with ZarrDataset(existing, mode="a") as dataset: + assert list(dataset.variables) == ["already_here"] + + def test_a_opens_a_new_group_in_an_existing_store(self, existing): + with ZarrDataset(existing, mode="a", group="sub") as dataset: + assert dataset.location.endswith("existing.zarr/sub") + assert list(dataset.variables) == [] + # The root is untouched. + with ZarrDataset(existing) as dataset: + assert list(dataset.variables) == ["already_here"] + + def test_r_plus_needs_the_store_to_exist(self, tmp_path): + with pytest.raises(FileNotFoundError): + ZarrDataset(tmp_path / "missing.zarr", mode="r+") + + def test_an_unknown_mode_names_itself(self, tmp_path): + with pytest.raises(ValueError, match="'a\\+'"): + ZarrDataset(tmp_path / "new.zarr", mode="a+") + + def test_reading_is_still_the_default(self, existing): + with ZarrDataset(existing) as dataset: + assert dataset.mode == "r" + + +class TestTheVersionGuard: + @pytest.mark.parametrize("mode", ["w-", "w", "a", "r+"]) + def test_writing_version_2_is_refused(self, tmp_path, mode): + with pytest.raises(NotImplementedError, match="version 3"): + ZarrDataset(tmp_path / "new.zarr", mode=mode, zarr_format=2) + + def test_reading_version_2_is_unaffected(self, tmp_path): + path = tmp_path / "v2.zarr" + group = zarr.open_group(store=path, mode="w-", zarr_format=2) + array = group.create_array("data", shape=(2,), dtype="f4") + array.attrs["_ARRAY_DIMENSIONS"] = ["x"] + with ZarrDataset(path, mode="r", zarr_format=2) as dataset: + assert dataset.format == "ZARR_V2" + assert list(dataset.variables) == ["data"] + + def test_a_version_3_store_is_what_a_write_produces(self, tmp_path): + with ZarrDataset(tmp_path / "new.zarr", mode="w-") as dataset: + assert dataset.encoding["zarr_format"] == 3 +``` + +- [ ] **Step 2: Run them, and watch them fail** + +```bash +pytest lib/iris/tests/unit/fileformats/zarr/dataset/test_ZarrDataset__write.py -x +``` + +Expected: `NotImplementedError: Saving to Zarr is not supported. Cannot open +with mode='w-'.` + +- [ ] **Step 3: Replace the mode constants** + +In `lib/iris/fileformats/zarr/_dataset.py`, the block above `class ZarrDataset` +currently reads `_READ_MODES = ("r",)` and `_NO_SAVING`. Replace both with: + +```python +#: The modes that only read. Writing through one of these is refused before +#: zarr-python gets the chance, so the message names Iris's keyword. +_READ_MODES = ("r",) + +#: The modes that may write. ``"r+"`` is here for the region write a future +#: multi-process saver needs: open an existing store for update, creating +#: nothing. See §4.5 of the design document. +_WRITE_MODES = ("w-", "w", "a", "r+") + +#: Every mode, in the order the error message lists them. +_MODES = _READ_MODES + _WRITE_MODES + +#: The only Zarr version Iris writes. Reading handles both. Adding version 2 +#: writing means the three version-specific points of §4.5 and deleting this. +_WRITABLE_ZARR_FORMAT = 3 +``` + +Delete `_NO_SAVING`; Tasks 5 and 6 implement every member that raised it. + +- [ ] **Step 4: Open for writing** + +Replace the guard and the open in `ZarrDataset.__init__`: + +```python + if mode not in _MODES: + message = f"Unknown mode {mode!r} for a Zarr store. Expected one of {_MODES}." + raise ValueError(message) + writing = mode in _WRITE_MODES + if writing and zarr_format != _WRITABLE_ZARR_FORMAT: + message = ( + f"Iris writes Zarr version {_WRITABLE_ZARR_FORMAT} stores only, " + f"so mode={mode!r} cannot be used with zarr_format={zarr_format!r}." + ) + raise NotImplementedError(message) + + self._mode = mode + target = str(location) + self._location = target if group is None else f"{target.rstrip('/')}/{group}" + + # `location` itself, not its stringified `target`, is handed to + # zarr-python: stringifying a store object such as a `MemoryStore` + # loses the store, leaving only a name zarr-python cannot reopen. + try: + self._group = zarr.open_group( + store=location, + mode=mode, + path=group, + zarr_format=zarr_format if writing else None, + ) + except FileExistsError as exception: + message = ( + f"Cannot create the Zarr store {self._location!r}: it already " + f"exists, and mode={mode!r} will not overwrite it. Pass " + "mode='w' to replace it, or mode='a' to add to it." + ) + raise FileExistsError(message) from exception + self._format = _FORMAT_BY_ZARR_VERSION[self._group.metadata.zarr_format] +``` + +and add, with the other initialised state above the guard: + +```python + #: The dimensions `create_dimension` has declared, by name. Zarr has + #: no dimension object, so this dict is the whole of the concept on + #: the write side, and what lets a size conflict be caught. + self._declared_dimensions: dict[str, int] = {} +``` + +Extend the `mode` and `Raises` entries of the docstring: + +```python + mode : str, default="r" + One of ``"r"``, ``"r+"``, ``"a"``, ``"w"`` or ``"w-"``, with + zarr-python's meanings. ``"w-"`` creates and refuses an existing + target; ``"w"`` replaces one; ``"a"`` adds to one. + zarr_format : int, default=3 + The version to create. Only version 3 can be written. Reading + handles either version, and ignores this. +``` + +```python + Raises + ------ + ValueError + For an unknown `mode`. + NotImplementedError + For a write `mode` with any `zarr_format` but 3. + FileExistsError + For ``mode="w-"`` onto a target that already exists. + FileNotFoundError + If `location` does not exist and `mode` does not create it. + zarr.errors.GroupNotFoundError + If `location` is not a Zarr store, or holds no such `group`. +``` + +- [ ] **Step 5: Run the tests** + +```bash +pytest lib/iris/tests/unit/fileformats/zarr/dataset/test_ZarrDataset__write.py -v +pytest lib/iris/tests/unit/fileformats/zarr lib/iris/tests/integration/zarr -q -n auto +``` + +Expected: the new file passes and PR 4's Zarr tests are untouched — the read +path takes the same `mode="r"` branch it always did. + +- [ ] **Step 6: Commit** + +```bash +git add -A +git commit -m "Open a Zarr store for writing" +``` + +--- + +## Task 5: Declare dimensions, and create arrays + +Spec §4.5, "Dimensions", and the create-once invariant. This is the task that +turns `CFSaver`'s two creation calls into Zarr arrays. + +**Files:** +- Modify: `lib/iris/fileformats/zarr/_dataset.py` — `create_dimension`, + `create_variable`, `dimensions`, and the caches both invalidate +- Modify: `lib/iris/tests/unit/fileformats/zarr/dataset/test_ZarrDataset__write.py` + +**Interfaces:** +- Consumes: Task 3's `default_fill_value` and `encode_fill_value`; Task 4's + `_declared_dimensions` and `_WRITE_MODES`. +- Produces: `ZarrDataset.create_dimension(name, size)` and + `ZarrDataset.create_variable(name, dtype, dimensions=(), *, fill_value=None, + **encoding) -> ZarrDatasetVariable`, with `_ZARR_ENCODING_KEYWORDS` naming + what `**encoding` accepts. + +- [ ] **Step 1: Write the failing tests** + +Append to `test_ZarrDataset__write.py`: + +```python +@pytest.fixture +def writable(tmp_path): + """Return an open, empty, writable dataset.""" + with ZarrDataset(tmp_path / "out.zarr", mode="w-") as dataset: + yield dataset + + +class TestCreateDimension: + def test_a_dimension_is_declared(self, writable): + writable.create_dimension("time", 3) + assert dict(writable.dimensions) == {"time": 3} + + def test_declaring_it_twice_at_one_size_is_harmless(self, writable): + writable.create_dimension("time", 3) + writable.create_dimension("time", 3) + assert dict(writable.dimensions) == {"time": 3} + + def test_a_conflicting_size_raises_naming_both(self, writable): + writable.create_dimension("time", 3) + with pytest.raises(ValueError, match="time.*3.*4|time.*4.*3"): + writable.create_dimension("time", 4) + + def test_unlimited_is_refused(self, writable): + # Spec §4.2: a store with no such concept raises NotImplementedError. + with pytest.raises(NotImplementedError, match="unlimited"): + writable.create_dimension("time", None) + + def test_a_read_only_dataset_refuses(self, existing): + with ZarrDataset(existing) as dataset: + with pytest.raises(ValueError, match="mode='r'"): + dataset.create_dimension("time", 3) + + +class TestCreateVariable: + def test_an_array_is_created_with_its_dimension_names(self, writable): + writable.create_dimension("x", 3) + variable = writable.create_variable("data", np.dtype("f4"), ["x"]) + assert variable.name == "data" + assert variable.dimensions == ("x",) + assert variable.shape == (3,) + assert variable.dtype == np.dtype("f4") + + def test_it_appears_in_the_dataset(self, writable): + writable.create_dimension("x", 3) + writable.create_variable("data", np.dtype("f4"), ["x"]) + assert list(writable.variables) == ["data"] + + def test_a_scalar_variable_has_no_dimensions(self, writable): + variable = writable.create_variable("grid", np.dtype("i4")) + assert variable.shape == () + + def test_an_undeclared_dimension_raises_naming_it(self, writable): + with pytest.raises(ValueError, match="nowhere"): + writable.create_variable("data", np.dtype("f4"), ["nowhere"]) + + def test_an_existing_name_raises_naming_the_group(self, writable): + # Spec §4.5: "not an overwrite and not a merge". + writable.create_dimension("x", 3) + writable.create_variable("data", np.dtype("f4"), ["x"]) + with pytest.raises(ValueError) as excinfo: + writable.create_variable("data", np.dtype("f4"), ["x"]) + assert "data" in str(excinfo.value) + assert writable.location in str(excinfo.value) + + def test_a_read_only_dataset_refuses(self, existing): + with ZarrDataset(existing) as dataset: + with pytest.raises(ValueError, match="mode='r'"): + dataset.create_variable("data", np.dtype("f4")) + + +class TestCreateVariableFillValue: + def test_an_undeclared_fill_is_netcdfs_default(self, writable): + # F11: zarr's own default is 0, which would make a masked point of a + # float array read back as a real zero. + writable.create_dimension("x", 3) + variable = writable.create_variable("data", np.dtype("f4"), ["x"]) + assert variable.fill_value == np.float32(9.9692099683868690e36) + + def test_an_undeclared_fill_writes_no_attribute(self, writable): + # Nothing is missing yet. The saver declares _FillValue when it finds + # the source is masked; see Task 11. + writable.create_dimension("x", 3) + variable = writable.create_variable("data", np.dtype("f4"), ["x"]) + assert "_FillValue" not in variable.attributes + + def test_a_declared_fill_reaches_the_array_and_the_attribute(self, writable): + import base64 + import struct + + writable.create_dimension("x", 3) + variable = writable.create_variable( + "data", np.dtype("f4"), ["x"], fill_value=np.float32(-99.0) + ) + assert variable.fill_value == np.float32(-99.0) + packed = base64.b64decode(variable.attributes["_FillValue"]) + assert struct.unpack(" None: + """Declare a dimension of the given length. + + Zarr has no dimension object: a dimension is a name an array carries + in its own metadata. So this records the name and size, and + :meth:`create_variable` writes the names onto each array. The record + is what lets a size conflict be caught here rather than becoming two + arrays that disagree. + + Parameters + ---------- + name : str + The dimension's name. + size : int or None + Its length. ``None`` requests an unlimited dimension, which Zarr + has no form for. + + Raises + ------ + NotImplementedError + For ``size=None``. + ValueError + If `name` is already declared at a different size, or if this + dataset was opened read-only. + + """ + self._check_writable("create_dimension") + if size is None: + message = ( + f"Cannot create the unlimited dimension {name!r}: a Zarr array " + "has a fixed shape, so there is no unlimited dimension to " + "create." + ) + raise NotImplementedError(message) + + declared = self._declared_dimensions.get(name) + if declared is not None and declared != size: + message = ( + f"Cannot declare dimension {name!r} with size {size}: it is " + f"already declared with size {declared}." + ) + raise ValueError(message) + self._declared_dimensions[name] = size + self._dimensions = None +``` + +and the guard it uses, beside it: + +```python + def _check_writable(self, action: str) -> None: + """Raise unless this dataset was opened in a mode that writes.""" + if self._mode not in _WRITE_MODES: + message = ( + f"Cannot {action} on a Zarr store opened with mode={self._mode!r}. " + f"Open it with one of {_WRITE_MODES} to write." + ) + raise ValueError(message) +``` + +- [ ] **Step 5: Report declared dimensions** + +`dimensions` assembles from the arrays present. A dimension declared but not +yet written to any array has to appear too — `_get_dim_names` and +`_create_string_variable` both ask this mapping what exists before choosing a +name. Replace the property with: + +```python + @property + def dimensions(self) -> Mapping: + """The dimension sizes this group's arrays agree on, and any declared. + + A dimension declared by :meth:`create_dimension` appears here before + any array carries it, because that is what a caller choosing a new + name needs to see. + + """ + if self._dimensions is None: + self._dimensions = { + **_assemble_dimensions(self.variables), + **self._declared_dimensions, + } + return self._dimensions +``` + +- [ ] **Step 6: Implement `create_variable`** + +```python + def create_variable( + self, name: str, dtype, dimensions=(), *, fill_value=None, **encoding + ) -> "ZarrDatasetVariable": + """Create and return a new array. + + Parameters + ---------- + name : str + The array's name within this group. + dtype : numpy.dtype or str or type + The array's dtype. ``str`` itself asks for a variable-length + string array, which is how this store holds text. + dimensions : iterable of str, optional + The names of its dimensions, each already declared by + :meth:`create_dimension`. Empty for a scalar. + fill_value : optional + The CF missing-value sentinel. When given it is written as a + ``_FillValue`` attribute as well as the array's own fill, because + on version 3 the attribute is the masking declaration and the + storage fill is not. When not given, the array takes the netCDF + default fill for its dtype, and declares nothing: the saver + declares one if it finds the data is masked. + **encoding + Zarr's own storage keywords: ``chunks``, ``shards``, + ``compressors``, ``filters``, ``serializer``, ``order``. + + Returns + ------- + :class:`ZarrDatasetVariable` + + Raises + ------ + ValueError + If the dataset is read-only, a dimension is undeclared, or `name` + already exists in this group. + TypeError + For a storage keyword this store does not have. + + """ + self._check_writable("create_variable") + self._check_encoding(encoding) + + dtype = dtype if dtype is str else np.dtype(dtype) + dimensions = tuple(dimensions) + shape = tuple(self._dimension_size(dimension) for dimension in dimensions) + + if name in self._group: + message = ( + f"Cannot create {name!r} in the Zarr group {self._location!r}: " + "it already holds something of that name, and Iris neither " + "overwrites nor merges into one." + ) + raise ValueError(message) + + if fill_value is None: + stored_fill = _encode.default_fill_value(np.dtype(dtype)) + else: + stored_fill = np.dtype(dtype).type(fill_value) + + array = self._group.create_array( + name, + shape=shape, + dtype=dtype, + fill_value=stored_fill, + dimension_names=dimensions or None, + **encoding, + ) + variable = ZarrDatasetVariable(array, self._location, self._format) + if fill_value is not None: + variable.attributes["_FillValue"] = fill_value + + # Both caches describe a group that has just changed. + self._variables = None + self._dimensions = None + return variable +``` + +with the two helpers beside it: + +```python + def _dimension_size(self, name: str) -> int: + """Return a declared dimension's size, or raise naming it.""" + size = self.dimensions.get(name) + if size is None: + message = ( + f"Cannot create an array over dimension {name!r}: no dimension " + "of that name has been declared in this group." + ) + raise ValueError(message) + return size + + def _check_encoding(self, encoding: dict) -> None: + """Raise for any storage keyword this store does not have.""" + unknown = set(encoding) - _ZARR_ENCODING_KEYWORDS + if not unknown: + return + lines = [] + for keyword in sorted(unknown): + replacement = _NETCDF_KEYWORDS.get(keyword) + if replacement is None: + lines.append(f" {keyword}: not a Zarr storage keyword") + else: + lines.append(f" {keyword}: use {replacement}") + message = ( + "Unknown Zarr storage keyword(s). A Zarr store takes zarr-python's " + "own keywords, which Iris does not translate netCDF's onto:\n" + + "\n".join(lines) + ) + raise TypeError(message) +``` + +Add `from . import _encode` to the imports. + +**Note on `name in self._group`.** zarr-python raises `ContainsArrayError` +itself, but only for an array; a *group* of that name takes a different path, +and neither message names the store. One check, before anything is created, is +both more uniform and the only form that can say "Iris neither overwrites nor +merges", which is the rule being enforced. + +- [ ] **Step 7: Run the tests** + +```bash +pytest lib/iris/tests/unit/fileformats/zarr/dataset/test_ZarrDataset__write.py -v +``` + +Expected: all pass. + +- [ ] **Step 8: Commit** + +```bash +git add -A +git commit -m "Declare Zarr dimensions, and create Zarr arrays" +``` + +--- From 308d4af4716e00cbfaa245ce406550480293bc42 Mon Sep 17 00:00:00 2001 From: Martin Yeo Date: Thu, 15 Oct 2026 13:47:07 +0000 Subject: [PATCH 05/78] Plan PR 6, stage 5: writing values, and the saver itself Task 6 puts each kind of write at the one place it passes through: attributes are converted before zarr sees them, because a value its encoder rejects stays in the mapping; masked values are filled on the way into an array; and finalise consolidates the whole store from the root, with zarr's "not part of the version 3 specification" warning suppressed and explained. Task 7 is the saver. It is thin, which is the point of PR 5: opening a store, one hook saying Zarr holds text natively and needs no trailing character dimension, and the save() users call. Storage keywords are validated before the store is created, so a mistyped one does not leave an empty store behind. Deferred writes, the alignment invariant and the masked _FillValue declaration are Task 8, on the same class. Co-Authored-By: Claude Opus 5 Session-Id: b8474d78-fdfa-43e5-b907-4bc3fe833fb3 --- .../plans/2026-10-15-zarr-io-pr6.md | 971 +++++++++++++++++- 1 file changed, 952 insertions(+), 19 deletions(-) diff --git a/docs/superpowers/plans/2026-10-15-zarr-io-pr6.md b/docs/superpowers/plans/2026-10-15-zarr-io-pr6.md index eb3f617950..89e2712915 100644 --- a/docs/superpowers/plans/2026-10-15-zarr-io-pr6.md +++ b/docs/superpowers/plans/2026-10-15-zarr-io-pr6.md @@ -1992,7 +1992,7 @@ name. Replace the property with: """ self._check_writable("create_variable") - self._check_encoding(encoding) + check_encoding_keywords(encoding) dtype = dtype if dtype is str else np.dtype(dtype) dimensions = tuple(dimensions) @@ -2043,24 +2043,42 @@ with the two helpers beside it: raise ValueError(message) return size - def _check_encoding(self, encoding: dict) -> None: - """Raise for any storage keyword this store does not have.""" - unknown = set(encoding) - _ZARR_ENCODING_KEYWORDS - if not unknown: - return - lines = [] - for keyword in sorted(unknown): - replacement = _NETCDF_KEYWORDS.get(keyword) - if replacement is None: - lines.append(f" {keyword}: not a Zarr storage keyword") - else: - lines.append(f" {keyword}: use {replacement}") - message = ( - "Unknown Zarr storage keyword(s). A Zarr store takes zarr-python's " - "own keywords, which Iris does not translate netCDF's onto:\n" - + "\n".join(lines) - ) - raise TypeError(message) +and, at module level beside the keyword tables rather than on the class, +because :func:`iris.fileformats.zarr.saver.save` calls it too - before it +creates a store, so that a mistyped keyword does not leave an empty one behind: + +```python +def check_encoding_keywords(encoding) -> None: + """Raise for any storage keyword a Zarr store does not have. + + Parameters + ---------- + encoding : dict + The storage keywords a caller passed. + + Raises + ------ + TypeError + Naming each unknown keyword, and the Zarr replacement where there + is one. + + """ + unknown = set(encoding) - _ZARR_ENCODING_KEYWORDS + if not unknown: + return + lines = [] + for keyword in sorted(unknown): + replacement = _NETCDF_KEYWORDS.get(keyword) + if replacement is None: + lines.append(f" {keyword}: not a Zarr storage keyword") + else: + lines.append(f" {keyword}: use {replacement}") + message = ( + "Unknown Zarr storage keyword(s). A Zarr store takes zarr-python's " + "own keywords, which Iris does not translate netCDF's onto:\n" + + "\n".join(lines) + ) + raise TypeError(message) ``` Add `from . import _encode` to the imports. @@ -2087,3 +2105,918 @@ git commit -m "Declare Zarr dimensions, and create Zarr arrays" ``` --- + +## Task 6: Write values and attributes, and consolidate + +Spec §4.5, "Attributes", "Masked data" and "Consolidation is a separate, +one-shot step". Three small changes in `_dataset.py`, each at the one place +every write of its kind passes through. + +**Files:** +- Modify: `lib/iris/fileformats/zarr/_dataset.py` — `_ZarrAttributes`, + `ZarrDatasetVariable.__setitem__`, `ZarrDataset.finalise` +- Modify: `lib/iris/tests/unit/fileformats/zarr/dataset/test_ZarrDataset__write.py` + +**Interfaces:** +- Consumes: Task 3's `json_native`, `encode_fill_value`, + `encode_missing_values` and `filled`; Task 5's `create_variable`. +- Produces: attribute writes that convert before zarr sees them; + `ZarrDatasetVariable.__setitem__` that fills masked values; + `ZarrDataset.finalise()` that consolidates the whole store once. + +- [ ] **Step 1: Write the failing tests** + +Append to `test_ZarrDataset__write.py`: + +```python +class TestAttributeWriting: + def test_a_numpy_scalar_is_converted(self, writable): + # F6: zarr's encoder rejects numpy types, and a rejected value stays + # in the mapping - so converting afterwards is already too late. + writable.create_dimension("x", 3) + variable = writable.create_variable("data", np.dtype("f4"), ["x"]) + variable.attributes["scale_factor"] = np.float32(0.5) + assert variable.attributes["scale_factor"] == 0.5 + assert type(variable.attributes["scale_factor"]) is float + + def test_a_numpy_array_becomes_a_list(self, writable): + writable.create_dimension("x", 3) + variable = writable.create_variable("data", np.dtype("f4"), ["x"]) + variable.attributes["valid_range"] = np.array([0.0, 10.0], dtype="f4") + assert variable.attributes["valid_range"] == [0.0, 10.0] + + def test_bytes_become_text(self, writable): + writable.create_dimension("x", 3) + variable = writable.create_variable("data", np.dtype("f4"), ["x"]) + variable.attributes["units"] = b"K" + assert variable.attributes["units"] == "K" + + def test_a_global_attribute_is_converted_too(self, writable): + writable.attributes["history"] = np.str_("made by a test") + assert type(writable.attributes["history"]) is str + + def test_an_unserialisable_attribute_raises_and_writes_nothing(self, writable): + class Opaque: + pass + + writable.create_dimension("x", 3) + variable = writable.create_variable("data", np.dtype("f4"), ["x"]) + with pytest.raises(TypeError, match="odd"): + variable.attributes["odd"] = Opaque() + assert "odd" not in variable.attributes + # And the store itself is clean, not merely the mapping. + reopened = zarr.open_group(store=writable.location, mode="r") + assert "odd" not in dict(reopened["data"].attrs) + + def test_missing_value_is_a_plain_number(self, writable): + # F10: base64 here opens in xarray and masks nothing. + writable.create_dimension("x", 3) + variable = writable.create_variable("data", np.dtype("f4"), ["x"]) + variable.attributes["missing_value"] = np.float32(-99.0) + assert variable.attributes["missing_value"] == -99.0 + + def test_a_fill_value_attribute_is_packed(self, writable): + import base64 + import struct + + writable.create_dimension("x", 3) + variable = writable.create_variable("data", np.dtype("f4"), ["x"]) + variable.attributes["_FillValue"] = np.float32(-99.0) + packed = base64.b64decode(variable.attributes["_FillValue"]) + assert struct.unpack("