From 50c157ea72c6fcf9a4881324e1f9dc4d417630ce Mon Sep 17 00:00:00 2001 From: Martin Yeo Date: Mon, 26 Oct 2026 13:10:44 +0000 Subject: [PATCH 1/9] Revise the library rules on imports, graphs, prose and exits Separate "never import *" from the __all__ rule, and drop the ban on conditional imports in a package __init__.py. The ban sat under Greppability, where it does not belong: a try-wrapped import of a verbatim name greps perfectly well. It also contradicted the tree, where iris_grib and iris.fileformats.zarr both defer an optional dependency's import. Record that indexing does not shrink a Dask graph. A slice still references every chunk of its source, and is culled only at compute. State the purposes of a docstring, a comment and a module docstring at the head of the section that governs all three. Replace the guard-clause advice with a single-exit rule. Nearly every function in lib/iris that contains a return has exactly one, so the old wording argued against the surrounding code. Delete the prose describing the sphinx-needs item a public module docstring carries. The check_docs_page_metadata hook enforces it, and names the expected type, the line limit and the missing tag when it fails. Co-Authored-By: Claude Opus 5 --- lib/iris/AGENTS.md | 79 +++++++++++++++++++++++----------------------- 1 file changed, 40 insertions(+), 39 deletions(-) diff --git a/lib/iris/AGENTS.md b/lib/iris/AGENTS.md index 422d487ff7..916296287d 100644 --- a/lib/iris/AGENTS.md +++ b/lib/iris/AGENTS.md @@ -16,14 +16,13 @@ message here, as well as anything posted to GitHub — read ## Why This File Exists -Iris is maintained by humans and agents together. Agents read through a -narrow context window using text search and cannot run the code; humans read -with intuition but limited patience. Both want explicit names, local -reasoning and small units. This file records that shared style, plus the few -places the two diverge and which way to resolve them. Models are trained to -please diff reviewers rather than to produce code that is cheap to navigate -later, and drift towards long functions, redundant comments and defensive -wrapping; correct for that deliberately. +Iris is maintained by humans and agents together. Agents search text through +a narrow window and cannot run the code; humans read with intuition but +limited patience. Both want explicit names, local reasoning and small units. +This file records that shared style, and the few places the two diverge. +Models are trained to please diff reviewers rather than to leave code cheap +to navigate, and drift towards long functions, redundant comments and +defensive wrapping; correct for that deliberately. ## Fast Rules @@ -43,10 +42,9 @@ wrapping; correct for that deliberately. - Keep behaviour derivable from the function's own body, signature, type hints and docstring. Avoid designs that require opening three other files. - Prefer composition and explicit delegation over deep inheritance and mixin - stacks. Resolving an MRO across modules costs an agent many tool calls and - is opaque to a newcomer. Where Iris already uses mixins (e.g. - `iris.common.mixin`, metadata managers), follow the existing pattern rather - than inventing a parallel one. + stacks: resolving an MRO across modules costs many tool calls and is opaque + to a newcomer. Where Iris already uses mixins (`iris.common.mixin`, + metadata managers), follow that pattern rather than inventing a parallel. - Do not add indirection (registry, dispatch dict, plugin hook, base class) for a single caller. This governs implementation layers, not API surface; a package `__init__.py` that re-exports is not indirection, see below. @@ -66,15 +64,16 @@ does not exist. a greppable literal and is the preferred idiom for optional CF attributes. - Give public names enough distinctiveness to search for. Local `data`, `cube`, `result` are fine; a public helper called `_process` is not. -- Declare `__all__` in modules with a public surface. No `import *`. -- **Re-export freely from a package `__init__.py`.** Surfacing objects at - the level users import from is a real convenience, and it keeps the file - layout free to change later. `iris.mesh` is the model: explicit - `from .components import MeshXY`, gathered into `__all__`. Re-export - verbatim — never rename on the way out, never wrap in `try`/conditional - imports, never put logic in `__init__.py`. Each object stays defined in - one module, so `class MeshXY` still finds it in a single grep. - (`iris.common` uses `import *`; that is legacy, not a pattern to copy.) +- Declare `__all__` in modules with a public surface. +- **Never `import *`.** (`iris.common` does; legacy, not a pattern to copy.) +- **Re-export freely from a package `__init__.py`.** It surfaces objects + where users import them and leaves the file layout free to change later. + `iris.mesh` is the model: explicit `from .components import MeshXY`, + gathered into `__all__`. Re-export verbatim — never rename on the way out, + and otherwise keep `__init__.py` free of logic — so that each object stays + defined in one greppable place. The routine reason to do more is deferring + an optional dependency's import, as `iris_grib` and `iris.fileformats.zarr` + both do; say in the module docstring which dependency that protects. - Make error and warning messages distinctive and mostly static, so a traceback maps to exactly one line. Put interpolated values at the end: `f"Cannot collapse a coordinate with bounds: {coord.name()}"`. @@ -109,6 +108,8 @@ Realising data is the caller's decision, never a library function's. and `has_lazy_data()` / `has_lazy_points()` answer which you have. - Build graphs whole-array with `iris._lazy_data` — `map_complete_blocks`, `lazy_elementwise`, `as_lazy_data`. Per-slice graph building is expensive. +- Indexing does not shrink a graph. Under Dask's multi-level graphs a slice + still references every chunk of its source, and is culled only at compute. ## Conform to the Specification, Not to the Sample File @@ -126,6 +127,11 @@ are evidence that a code path gets exercised, not authority for what it does. ## Comments and Docstrings +Three channels, three readers. A **docstring** states the contract for +whoever calls the thing. A **comment** explains why, at the line that needs +it, for whoever maintains it. A **module docstring** carries what is too big +for either: the subsystem explanation a maintainer needs before touching it. + A stale comment is worse than no comment — an agent treats it as evidence and a human treats it as documentation. @@ -206,13 +212,12 @@ least able to judge how much of it belongs in the file. Assume too much. ## Module Docstrings Carry the Explanation Prose explaining a subsystem belongs in the module docstring. Inline comments -scatter one argument across dozens of sites; a companion Markdown file is -validated by nothing and rots unseen. Only the docstring is read on every -visit, published by Sphinx, checked by numpydoc, and reviewed in the diff. +scatter one argument over dozens of sites; a companion Markdown file is +validated by nothing and rots unseen. Only the docstring is read every visit, +published, checked by numpydoc and reviewed in the diff. -Be generous here, and only here: sixty lines of orientation above a -two-thousand-line module is cheap, read once per visit rather than once per -call site. Not a tutorial. +Be generous here and only here: sixty lines of orientation above a +two-thousand-line module is cheap. Not a tutorial. Cover whichever apply: @@ -226,11 +231,6 @@ Leave out anything a reader or Sphinx can already derive: API listings, signatures, call sequences, per-release history. Explanation scoped to a single class belongs in that class's docstring instead. -A public module's docstring is also published as an API page, so it must open -with a `z_reference` item — a title, `:tags:` carrying at least one `topic_*`, -and a one-line summary — within its first 25 lines. Copy the form from a -neighbouring module; `pre-commit run check-docs-page-metadata` checks it. - **Exemption to "do not document code you did not change":** if you had to work out how a subsystem behaves in order to edit it, write that understanding into the module docstring — the comprehension is otherwise discarded when your @@ -247,9 +247,11 @@ code in a private sibling module instead. - New modules: aim under ~1000 lines, one coherent concept each. - Functions: aim to fit one screen (~50 lines). -- Use guard clauses and early returns instead of nested conditionals. -- Give a function one exit where it must still warn, record or close something - on the way out. A second exit added later silently skips that work. +- **One exit.** Nearly every function here has a single `return`; match them. + A second exit added later silently skips whatever the first does on the way + out — the warning, the record, the close. +- Flatten a deep conditional with a progress flag carried down a flat + sequence of `if`s, rather than with extra returns. - Nesting beyond three levels is a signal to extract a named helper. @@ -280,10 +282,9 @@ code in a private sibling module instead. re-staged and the run repeated until clean. - New and changed public functions have type hints and NumPy docstrings. - No comment or docstring was left stale by the change. -- If you debugged during this change, re-read the diff as someone who was not - there. Comments added after the first working version are where verbosity - collects — and where a comment turns into a reply to a conversation the - reader never had. +- If you debugged here, re-read the diff as someone who was not there. + Comments added after the first working version are where verbosity collects, + and where a comment becomes a reply to a conversation the reader never had. - Laziness behaviour is preserved and documented. - Tests updated per [`tests/AGENTS.md`](tests/AGENTS.md); changelog fragment added per [`../../changelog/AGENTS.md`](../../changelog/AGENTS.md). From f2ce30f066e80ce030c489adf30b8feaab6a59f9 Mon Sep 17 00:00:00 2001 From: Martin Yeo Date: Mon, 26 Oct 2026 13:10:50 +0000 Subject: [PATCH 2/9] Require a doctest run before finishing, not only after editing one A doctest asserts real output, so a change anywhere in the library or the user guide can break one. Nothing in the pytest suite covers them, so the previous rule - run them only if you edited a >>> example - left those breakages to CI. Exempt only changes confined to lib/iris/tests/ or to changelog/, which cannot reach a doctest. Co-Authored-By: Claude Opus 5 --- lib/iris/tests/AGENTS.md | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/iris/tests/AGENTS.md b/lib/iris/tests/AGENTS.md index f776960912..6a658eec5c 100644 --- a/lib/iris/tests/AGENTS.md +++ b/lib/iris/tests/AGENTS.md @@ -99,8 +99,12 @@ does: | `nox -s doctest` | what CI runs: `make clean html`, then `make doctest` | | `cd docs/src && make doctest` | skips the gallery rebuild; faster, not identical to CI | -Both take minutes, so run one once before finishing, and only if you touched -a `>>>` example. Do not substitute `pytest --doctest-modules` or +Both take minutes. Run one once before finishing, whether or not you edited a +`>>>` — a doctest asserts real output, so a change anywhere in the library or +the guide can break one, and nothing else in the suite would tell you. Only a +change confined to `lib/iris/tests/` or to `changelog/` is exempt. + +Do not substitute `pytest --doctest-modules` or `python -m doctest`: 150 `testsetup::` / `testcode::` directives supply context only Sphinx applies, so plain doctest reports failures that are not real. @@ -153,6 +157,9 @@ real. ## Pre-Finish Checklist - The changed tests pass locally. +- **Doctests were run**, unless the change stayed inside `lib/iris/tests/` or + `changelog/`. The pytest suite does not cover them, so a `>>>` you never + looked at is the one that breaks. See [Doctests](#doctests) above. - No unrelated tests were modified. - Any required external data setup is documented in the change notes. - **Read the skip count, not just the failures.** Comparing failures before From 850d90752439638c3fd52e3a0aae49672635f727 Mon Sep 17 00:00:00 2001 From: Martin Yeo Date: Mon, 26 Oct 2026 14:24:26 +0000 Subject: [PATCH 3/9] Add a maintainer-notes directive that publishes nothing A module docstring is the only prose read on every visit to a module and reviewed in the diff, which makes it the right home for subsystem explanation. Sphinx publishes it, so prose written for a maintainer arrives in front of a user. This directive drops its content instead, leaving it in the file and off the page. Built on docutils and sphinx alone, so it adds no dependency. The content is never parsed, which makes broken markup and unbalanced doctest prompts safe to write and keeps the doctest builder away from them. Misspelling the name stops the build, because make html runs with -W, and the content does not render on the way out. A plain reST comment fails the other way: dedent one line and the prose is silently published. Co-Authored-By: Claude Opus 5 --- docs/src/conf.py | 1 + docs/src/sphinxext/maintainer_notes.py | 49 ++++++++++++++++++++++++++ 2 files changed, 50 insertions(+) create mode 100644 docs/src/sphinxext/maintainer_notes.py diff --git a/docs/src/conf.py b/docs/src/conf.py index 8b029498c2..acd8831cf3 100644 --- a/docs/src/conf.py +++ b/docs/src/conf.py @@ -152,6 +152,7 @@ def _dotv(version): # extensions coming with Sphinx (named "sphinx.ext.*") or your custom # ones. extensions = [ + "maintainer_notes", "matplotlib.sphinxext.mathmpl", "matplotlib.sphinxext.plot_directive", "page_metadata_validator", diff --git a/docs/src/sphinxext/maintainer_notes.py b/docs/src/sphinxext/maintainer_notes.py new file mode 100644 index 0000000000..86f87923c3 --- /dev/null +++ b/docs/src/sphinxext/maintainer_notes.py @@ -0,0 +1,49 @@ +# 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. +"""The ``maintainer-notes`` directive: docstring prose that is never published. + +A module docstring is the only prose read on every visit to a module, checked +in the diff, and impossible to lose track of. That makes it the right home for +subsystem explanation - but Sphinx publishes it, so prose written for whoever +maintains the module arrives in front of whoever uses it. This directive is +the way out: docutils collects its content and this returns nothing, so the +prose stays in the file and reaches no page. + +The content is deliberately never parsed. :meth:`MaintainerNotes.run` returns +no nodes and never calls ``nested_parse``, so anything inside is free text. +Broken markup, unbalanced ``>>>`` and stray asterisks are all safe, and the +doctest builder does not see them either. That is the point of the design, not +an oversight: an agent writing notes cannot break a docs build with them. + +Getting the name wrong fails loudly. ``make html`` runs with ``-W``, so an +unknown directive stops the build, and the content does not render on the way +out. A plain reST comment fails the other way: dedent one line and the prose +is silently published. +""" + +from docutils.parsers.rst import Directive # type: ignore[import-untyped] +from sphinx.application import Sphinx + + +class MaintainerNotes(Directive): + """Swallow the directive's content, contributing nothing to the page.""" + + has_content = True + optional_arguments = 1 + final_argument_whitespace = True + + def run(self): + """Return no nodes, leaving the content unparsed and unpublished.""" + return [] + + +def setup(app: Sphinx): + """Set up the Sphinx extension. + + This function is expected by Sphinx to register the extension. + """ + app.add_directive("maintainer-notes", MaintainerNotes) + + return {"version": "0.1"} From add3af6ef9f28144435c9115497f332d7d01aa02 Mon Sep 17 00:00:00 2001 From: Martin Yeo Date: Mon, 26 Oct 2026 14:24:34 +0000 Subject: [PATCH 4/9] Route maintainer prose to a maintainer-notes block Names the block in the three places lib/iris/AGENTS.md already sends maintainer-facing prose: the summary of which channel serves which reader, the brevity rule that sorts a sentence by who it is for, and the home for a surprise no test can pin. Co-Authored-By: Claude Opus 5 --- lib/iris/AGENTS.md | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/lib/iris/AGENTS.md b/lib/iris/AGENTS.md index 916296287d..1b1d583773 100644 --- a/lib/iris/AGENTS.md +++ b/lib/iris/AGENTS.md @@ -130,7 +130,7 @@ are evidence that a code path gets exercised, not authority for what it does. Three channels, three readers. A **docstring** states the contract for whoever calls the thing. A **comment** explains why, at the line that needs it, for whoever maintains it. A **module docstring** carries what is too big -for either: the subsystem explanation a maintainer needs before touching it. +for either, in a `maintainer-notes` block that is never published. A stale comment is worse than no comment — an agent treats it as evidence and a human treats it as documentation. @@ -160,8 +160,8 @@ a human treats it as documentation. sentence, ask what breaks if it is gone; most rationale survives its own deletion. Then ask who each surviving sentence is for — a **user** calling the thing, a **maintainer** not breaking it, or a **reviewer** you are - answering. Only the first belongs here; the second goes to the module - docstring or a test, the third to the pull request. + answering. Only the first belongs here; the second goes to + `maintainer-notes` or a test, the third to the pull request. - Write notes, not an essay. Fragments are correct, articles are optional, shorthand is welcome. Say what holds; do not restate the signature, defend the design against an alternative nobody proposed, or narrate how you found @@ -203,7 +203,7 @@ least able to judge how much of it belongs in the file. Assume too much. in its docstring, the source keeping one line that names it. This is the only home that fails loudly when it stops being true. - A surprise no test can pin — a library quirk, an invariant, a coupling — - goes in the module docstring's traps. + goes in the module docstring's `maintainer-notes`. - A surprise caused by an external bug keeps the URL, not a retelling. - Write the fact, not your side of the argument about it. The tells that you have written a reply are in [`WRITING.md`](../../.github/WRITING.md). @@ -213,8 +213,13 @@ least able to judge how much of it belongs in the file. Assume too much. Prose explaining a subsystem belongs in the module docstring. Inline comments scatter one argument over dozens of sites; a companion Markdown file is -validated by nothing and rots unseen. Only the docstring is read every visit, -published, checked by numpydoc and reviewed in the diff. +validated by nothing and rots unseen. Only the docstring is read every visit +and reviewed in the diff. + +Anything aimed at a maintainer rather than a user goes in a `maintainer-notes` +block at the end, which Sphinx drops: the prose stays in the file and never +reaches the API page. Misspell the directive and the docs build fails, rather +than publishing what you meant to hide. Be generous here and only here: sixty lines of orientation above a two-thousand-line module is cheap. Not a tutorial. From a1bc83a911f918dace5882eacad9d3a4c90df892 Mon Sep 17 00:00:00 2001 From: Martin Yeo Date: Mon, 26 Oct 2026 14:24:40 +0000 Subject: [PATCH 5/9] Mark the module docstrings whose prose should move Sixteen module docstrings carry maintainer-facing prose - design rationale, traps, invariants, coupling - and each gains a one-line todo at the point the maintainer-notes block will go. Converting them is a separate change. The prose has to be read and sorted, and several of these files are under active development, so a wide diff here would conflict with work in flight for no gain. Co-Authored-By: Claude Opus 5 --- lib/iris/fileformats/_nc_load_rules/actions.py | 2 ++ lib/iris/fileformats/cf/__init__.py | 2 ++ lib/iris/fileformats/cf/_dask_locks.py | 2 ++ lib/iris/fileformats/cf/_group.py | 2 ++ lib/iris/fileformats/cf/_reader.py | 2 ++ lib/iris/fileformats/cf/_variables.py | 2 ++ lib/iris/fileformats/cf/dataset.py | 2 ++ lib/iris/fileformats/cf/loader.py | 2 ++ lib/iris/fileformats/cf/saver.py | 2 ++ lib/iris/fileformats/netcdf/_dataset.py | 2 ++ lib/iris/fileformats/netcdf/loader.py | 2 ++ lib/iris/fileformats/zarr/__init__.py | 2 ++ lib/iris/fileformats/zarr/_dataset.py | 2 ++ lib/iris/fileformats/zarr/_decode.py | 2 ++ lib/iris/fileformats/zarr/loader.py | 2 ++ lib/iris/io/format_picker.py | 2 ++ 16 files changed, 32 insertions(+) diff --git a/lib/iris/fileformats/_nc_load_rules/actions.py b/lib/iris/fileformats/_nc_load_rules/actions.py index 4d600c9420..3f599df340 100644 --- a/lib/iris/fileformats/_nc_load_rules/actions.py +++ b/lib/iris/fileformats/_nc_load_rules/actions.py @@ -37,6 +37,8 @@ comparing behaviour with the old Pyke rules (debugging). But once that is no longer useful, this can be considerably simplified. +.. todo:: Move the maintainer prose above into a ``maintainer-notes`` block. + """ from functools import partial, wraps diff --git a/lib/iris/fileformats/cf/__init__.py b/lib/iris/fileformats/cf/__init__.py index 31b4747a75..85e6e3a445 100644 --- a/lib/iris/fileformats/cf/__init__.py +++ b/lib/iris/fileformats/cf/__init__.py @@ -30,6 +30,8 @@ [CF] NetCDF Climate and Forecast (CF) Metadata conventions. [NUG] NetCDF User's Guide, https://docs.unidata.ucar.edu/nug/current/ +.. todo:: Move the maintainer prose above into a ``maintainer-notes`` block. + """ from ._group import CFGroup diff --git a/lib/iris/fileformats/cf/_dask_locks.py b/lib/iris/fileformats/cf/_dask_locks.py index df04c95698..97ff1f6919 100644 --- a/lib/iris/fileformats/cf/_dask_locks.py +++ b/lib/iris/fileformats/cf/_dask_locks.py @@ -35,6 +35,8 @@ already open for writing simply raises an access error. Iris does not support that mode at present. +.. todo:: Move the maintainer prose above into a ``maintainer-notes`` block. + """ import threading diff --git a/lib/iris/fileformats/cf/_group.py b/lib/iris/fileformats/cf/_group.py index f38586a3a8..3cd5e0f215 100644 --- a/lib/iris/fileformats/cf/_group.py +++ b/lib/iris/fileformats/cf/_group.py @@ -30,6 +30,8 @@ from that referrer with a warning, then promoted so that it is not lost altogether. Promotion is how Iris avoids silently discarding either. +.. todo:: Move the maintainer prose above into a ``maintainer-notes`` block. + """ from collections.abc import MutableMapping diff --git a/lib/iris/fileformats/cf/_reader.py b/lib/iris/fileformats/cf/_reader.py index 2b9caceb43..75d4e85278 100644 --- a/lib/iris/fileformats/cf/_reader.py +++ b/lib/iris/fileformats/cf/_reader.py @@ -48,6 +48,8 @@ Only the first case mentions netCDF, and only because something has to be the default. +.. todo:: Move the maintainer prose above into a ``maintainer-notes`` block. + """ from collections.abc import Iterable diff --git a/lib/iris/fileformats/cf/_variables.py b/lib/iris/fileformats/cf/_variables.py index 91f229bd21..b12d58c8e0 100644 --- a/lib/iris/fileformats/cf/_variables.py +++ b/lib/iris/fileformats/cf/_variables.py @@ -42,6 +42,8 @@ path here that touches data, and it is reached only from ``CFReader(..., monotonic=True)``. +.. todo:: Move the maintainer prose above into a ``maintainer-notes`` block. + """ from abc import ABCMeta, abstractmethod diff --git a/lib/iris/fileformats/cf/dataset.py b/lib/iris/fileformats/cf/dataset.py index 2982708a2e..bcbd93be98 100644 --- a/lib/iris/fileformats/cf/dataset.py +++ b/lib/iris/fileformats/cf/dataset.py @@ -32,6 +32,8 @@ See sections 4.2, 4.3 and 4.5 of ``docs/superpowers/specs/2026-09-21-zarr-io-design.md``. +.. todo:: Move the maintainer prose above into a ``maintainer-notes`` block. + """ from abc import ABC, abstractmethod diff --git a/lib/iris/fileformats/cf/loader.py b/lib/iris/fileformats/cf/loader.py index 7133f10503..c4b6f2bbd7 100644 --- a/lib/iris/fileformats/cf/loader.py +++ b/lib/iris/fileformats/cf/loader.py @@ -28,6 +28,8 @@ Also : `CF Conventions `_. +.. todo:: Move the maintainer prose above into a ``maintainer-notes`` block. + """ from collections.abc import Iterable, Iterator, Mapping diff --git a/lib/iris/fileformats/cf/saver.py b/lib/iris/fileformats/cf/saver.py index d738cc5062..fe1400a018 100644 --- a/lib/iris/fileformats/cf/saver.py +++ b/lib/iris/fileformats/cf/saver.py @@ -51,6 +51,8 @@ See : `CF Conventions `_. +.. todo:: Move the maintainer prose above into a ``maintainer-notes`` block. + """ import collections diff --git a/lib/iris/fileformats/netcdf/_dataset.py b/lib/iris/fileformats/netcdf/_dataset.py index 925223aad7..e9b166467f 100644 --- a/lib/iris/fileformats/netcdf/_dataset.py +++ b/lib/iris/fileformats/netcdf/_dataset.py @@ -20,6 +20,8 @@ See section 4.5 of ``docs/superpowers/specs/2026-09-21-zarr-io-design.md``. +.. todo:: Move the maintainer prose above into a ``maintainer-notes`` block. + """ from collections.abc import Iterator, Mapping, MutableMapping diff --git a/lib/iris/fileformats/netcdf/loader.py b/lib/iris/fileformats/netcdf/loader.py index 2eb780d7ec..ba0e433fc3 100644 --- a/lib/iris/fileformats/netcdf/loader.py +++ b/lib/iris/fileformats/netcdf/loader.py @@ -24,6 +24,8 @@ Also : `CF Conventions `_. +.. todo:: Move the maintainer prose above into a ``maintainer-notes`` block. + """ from collections.abc import Iterable diff --git a/lib/iris/fileformats/zarr/__init__.py b/lib/iris/fileformats/zarr/__init__.py index 3b6306653e..070f582e22 100644 --- a/lib/iris/fileformats/zarr/__init__.py +++ b/lib/iris/fileformats/zarr/__init__.py @@ -26,6 +26,8 @@ See sections 4.1 and 4.4 of ``docs/superpowers/specs/2026-09-21-zarr-io-design.md``. +.. todo:: Move the maintainer prose above into a ``maintainer-notes`` block. + """ import importlib.metadata diff --git a/lib/iris/fileformats/zarr/_dataset.py b/lib/iris/fileformats/zarr/_dataset.py index 8c9d5a5683..164a5eb4c1 100644 --- a/lib/iris/fileformats/zarr/_dataset.py +++ b/lib/iris/fileformats/zarr/_dataset.py @@ -39,6 +39,8 @@ See sections 4.2, 4.4 and 4.6 of ``docs/superpowers/specs/2026-09-21-zarr-io-design.md``. +.. todo:: Move the maintainer prose above into a ``maintainer-notes`` block. + """ from collections.abc import Callable, Iterator, Mapping, MutableMapping diff --git a/lib/iris/fileformats/zarr/_decode.py b/lib/iris/fileformats/zarr/_decode.py index e2064a430a..6c664c7d8f 100644 --- a/lib/iris/fileformats/zarr/_decode.py +++ b/lib/iris/fileformats/zarr/_decode.py @@ -47,6 +47,8 @@ See section 4.4 of ``docs/superpowers/specs/2026-09-21-zarr-io-design.md``. +.. todo:: Move the maintainer prose above into a ``maintainer-notes`` block. + """ import base64 diff --git a/lib/iris/fileformats/zarr/loader.py b/lib/iris/fileformats/zarr/loader.py index e6178e96dc..54563c0db1 100644 --- a/lib/iris/fileformats/zarr/loader.py +++ b/lib/iris/fileformats/zarr/loader.py @@ -31,6 +31,8 @@ See sections 4.6 and 4.7 of ``docs/superpowers/specs/2026-09-21-zarr-io-design.md``. +.. todo:: Move the maintainer prose above into a ``maintainer-notes`` block. + """ from collections.abc import Iterable diff --git a/lib/iris/io/format_picker.py b/lib/iris/io/format_picker.py index dc04fd549e..f92c52e1da 100644 --- a/lib/iris/io/format_picker.py +++ b/lib/iris/io/format_picker.py @@ -44,6 +44,8 @@ The calling sequence of handler is dependent on the function given in the original specification and can be customised to your project's needs. +.. todo:: Move the maintainer prose above into a ``maintainer-notes`` block. + """ from collections.abc import Callable From 95d5d85c4bf974631032cd9595babc4e32822faf Mon Sep 17 00:00:00 2001 From: Martin Yeo Date: Mon, 26 Oct 2026 14:32:57 +0000 Subject: [PATCH 6/9] Tighten Module Docstrings Carry the Explanation by eight lines Three of the removed sentences were said twice. That inline comments scatter an argument, and that a companion Markdown file rots unseen, are both Fast Rule 7. That misspelling the directive fails the build is in the directive's own module docstring. lib/iris/AGENTS.md now fits the 300-line cap its meta-instruction sets, which it did not before. Co-Authored-By: Claude Opus 5 --- lib/iris/AGENTS.md | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/lib/iris/AGENTS.md b/lib/iris/AGENTS.md index 1b1d583773..03e09a3416 100644 --- a/lib/iris/AGENTS.md +++ b/lib/iris/AGENTS.md @@ -211,20 +211,13 @@ least able to judge how much of it belongs in the file. Assume too much. ## Module Docstrings Carry the Explanation -Prose explaining a subsystem belongs in the module docstring. Inline comments -scatter one argument over dozens of sites; a companion Markdown file is -validated by nothing and rots unseen. Only the docstring is read every visit -and reviewed in the diff. +Prose explaining a subsystem belongs in the module docstring: it is the only +prose read on every visit and reviewed in the diff. Anything aimed at a +maintainer rather than a user goes in a `maintainer-notes` block at the end, +which Sphinx drops, so it never reaches the API page. -Anything aimed at a maintainer rather than a user goes in a `maintainer-notes` -block at the end, which Sphinx drops: the prose stays in the file and never -reaches the API page. Misspell the directive and the docs build fails, rather -than publishing what you meant to hide. - -Be generous here and only here: sixty lines of orientation above a -two-thousand-line module is cheap. Not a tutorial. - -Cover whichever apply: +Be generous here and only here — sixty lines of orientation above a +two-thousand-line module is cheap, and not a tutorial. Cover whichever apply: - what the module is for, and what it deliberately does not do; - the vocabulary — name the concepts the code assumes you already know; @@ -232,9 +225,8 @@ Cover whichever apply: - why the design is as it is, including approaches tried and rejected; - known traps, and the modules this one is coupled to. -Leave out anything a reader or Sphinx can already derive: API listings, -signatures, call sequences, per-release history. Explanation scoped to a -single class belongs in that class's docstring instead. +Leave out what a reader or Sphinx can derive: API listings, signatures, call +sequences, per-release history. Class-scoped explanation goes in that class. **Exemption to "do not document code you did not change":** if you had to work out how a subsystem behaves in order to edit it, write that understanding into From 504b78530e68247c3987bbb688e2e43631febb97 Mon Sep 17 00:00:00 2001 From: Martin Yeo Date: Tue, 27 Oct 2026 12:26:06 +0000 Subject: [PATCH 7/9] Move maintainer prose into maintainer-notes blocks Seventeen module docstrings now split by reader. What a caller needs stays on the page; design rationale, traps, invariants and coupling move into a maintainer-notes block that Sphinx drops. Two of the seventeen are the saving modules added by #7346, which never carried a marker. iris.io.format_picker loses its marker without gaining a block. Its docstring is a usage tutorial throughout, so it was marked on length rather than on content, and nothing in it is written for a maintainer. Two corrections made in passing, both to text that was being moved anyway. iris.fileformats.zarr._dataset announced three things about Zarr and then listed four. iris.fileformats.cf pointed at "the plan, section 8.4"; section 8 of the design document is Risks and has no subsections, so that resolved to nothing and is dropped. Co-Authored-By: Claude Opus 5 Session-Id: 76a82181-7fe1-4063-9fd7-deb10fb0d658 --- .../fileformats/_nc_load_rules/actions.py | 69 +++++++------- lib/iris/fileformats/cf/__init__.py | 22 +++-- lib/iris/fileformats/cf/_dask_locks.py | 55 +++++------ lib/iris/fileformats/cf/_group.py | 41 +++++---- lib/iris/fileformats/cf/_reader.py | 92 ++++++++++--------- lib/iris/fileformats/cf/_variables.py | 67 +++++++------- lib/iris/fileformats/cf/dataset.py | 40 ++++---- lib/iris/fileformats/cf/loader.py | 28 +++--- lib/iris/fileformats/cf/saver.py | 78 ++++++++-------- lib/iris/fileformats/netcdf/_dataset.py | 16 ++-- lib/iris/fileformats/netcdf/loader.py | 9 +- lib/iris/fileformats/zarr/__init__.py | 14 +-- lib/iris/fileformats/zarr/_dataset.py | 86 ++++++++--------- lib/iris/fileformats/zarr/_decode.py | 83 +++++++++-------- lib/iris/fileformats/zarr/_encode.py | 61 ++++++------ lib/iris/fileformats/zarr/loader.py | 33 +++---- lib/iris/fileformats/zarr/saver.py | 18 ++-- lib/iris/io/format_picker.py | 2 - 18 files changed, 424 insertions(+), 390 deletions(-) diff --git a/lib/iris/fileformats/_nc_load_rules/actions.py b/lib/iris/fileformats/_nc_load_rules/actions.py index ac8a406a7a..1b1b686187 100644 --- a/lib/iris/fileformats/_nc_load_rules/actions.py +++ b/lib/iris/fileformats/_nc_load_rules/actions.py @@ -4,40 +4,41 @@ # See LICENSE in the root of the repository for full licensing details. """Replacement code for the Pyke rules. -For now, we are still emulating various aspects of how our original Pyke-based -code used the Pyke 'engine' to hold translation data, both Pyke-specific and -not : - -1) basic details from the iris.fileformats.cf analysis of the file are - recorded before translating each output cube, using - "engine.assert_case_specific_fact(name, args)". - -2) this is also used to store intermediate info passed between rules, which - used to be done with a "facts_cf.provides" statement in rule actions. - -3) Iris-specific info is (still) stored in additional properties created on - the engine object : - engine.cf_var, .cube, .cube_parts, .requires, .rules_triggered, .filename - -Our "rules" are just action routines. -The top-level 'run_actions' routine decides which actions to call, based on the -info recorded when processing each cube output. It does this in a simple -explicit way, which doesn't use any clever chaining, "trigger conditions" or -other rule-type logic. - -Each 'action' function can replace several similar 'rules'. -E.G. 'action_provides_grid_mapping' replaces all 'fc_provides_grid_mapping_'. -To aid debug, each returns a 'rule_name' string, indicating which original rule -this particular action call is emulating : In some cases, this may include a -textual note that this rule 'failed', aka "did not trigger", which would not be -recorded in the original implementation. - -TODO: remove the use of intermediate "facts" to carry information between -actions. This mimics older behaviour, so is still useful while we are still -comparing behaviour with the old Pyke rules (debugging). But once that is no -longer useful, this can be considerably simplified. - -.. todo:: Move the maintainer prose above into a ``maintainer-notes`` block. +.. maintainer-notes:: + + For now, we are still emulating various aspects of how our original + Pyke-based code used the Pyke 'engine' to hold translation data, both + Pyke-specific and not : + + 1) basic details from the iris.fileformats.cf analysis of the file are + recorded before translating each output cube, using + "engine.assert_case_specific_fact(name, args)". + + 2) this is also used to store intermediate info passed between rules, which + used to be done with a "facts_cf.provides" statement in rule actions. + + 3) Iris-specific info is (still) stored in additional properties created on + the engine object : + engine.cf_var, .cube, .cube_parts, .requires, .rules_triggered, + .filename + + Our "rules" are just action routines. The top-level 'run_actions' routine + decides which actions to call, based on the info recorded when processing + each cube output. It does this in a simple explicit way, which doesn't use + any clever chaining, "trigger conditions" or other rule-type logic. + + Each 'action' function can replace several similar 'rules'. E.G. + 'action_provides_grid_mapping' replaces all + 'fc_provides_grid_mapping_'. To aid debug, each returns a 'rule_name' + string, indicating which original rule this particular action call is + emulating : In some cases, this may include a textual note that this rule + 'failed', aka "did not trigger", which would not be recorded in the + original implementation. + + TODO: remove the use of intermediate "facts" to carry information between + actions. This mimics older behaviour, so is still useful while we are + still comparing behaviour with the old Pyke rules (debugging). But once + that is no longer useful, this can be considerably simplified. """ diff --git a/lib/iris/fileformats/cf/__init__.py b/lib/iris/fileformats/cf/__init__.py index 85e6e3a445..06d148f16d 100644 --- a/lib/iris/fileformats/cf/__init__.py +++ b/lib/iris/fileformats/cf/__init__.py @@ -12,25 +12,27 @@ Provides the capability to load netCDF files and interpret them according to the 'NetCDF Climate and Forecast (CF) Metadata Conventions'. -This package is a re-export surface and holds no logic of its own. Every -public name below is defined in exactly one private module and imported here -verbatim, so ``class CFReader`` still finds its definition in a single grep. Import from :mod:`iris.fileformats.cf`, not from the private modules: the file layout is deliberately free to change, and it will, as the format-agnostic CF machinery grows a dataset abstraction, a loader and a saver. -The re-exported classes keep the ``__module__`` their defining module gives -them, so ``repr(CFReader)`` names ``iris.fileformats.cf._reader`` rather than -this package. That is the same trade-off :mod:`iris.mesh` already makes, and -it is the one that keeps ``inspect.getsource`` and the documentation's -``[source]`` links working. See the plan, section 8.4. - References ---------- [CF] NetCDF Climate and Forecast (CF) Metadata conventions. [NUG] NetCDF User's Guide, https://docs.unidata.ucar.edu/nug/current/ -.. todo:: Move the maintainer prose above into a ``maintainer-notes`` block. +.. maintainer-notes:: + + This package is a re-export surface and holds no logic of its own. Every + public name below is defined in exactly one private module and imported + here verbatim, so ``class CFReader`` still finds its definition in a single + grep. + + The re-exported classes keep the ``__module__`` their defining module gives + them, so ``repr(CFReader)`` names ``iris.fileformats.cf._reader`` rather + than this package. That is the same trade-off :mod:`iris.mesh` already + makes, and it is the one that keeps ``inspect.getsource`` and the + documentation's ``[source]`` links working. """ diff --git a/lib/iris/fileformats/cf/_dask_locks.py b/lib/iris/fileformats/cf/_dask_locks.py index 97ff1f6919..e74a434b0f 100644 --- a/lib/iris/fileformats/cf/_dask_locks.py +++ b/lib/iris/fileformats/cf/_dask_locks.py @@ -9,33 +9,34 @@ so this module works out which scheduler is in play and returns a lock to match. The scheduler must therefore be configured before a lock is asked for. -A dataset implementation asks for one lock per target, identified by its path -or URL, and gives it to every write handle it builds. The netCDF write handle -reopens the file, acquires the lock, writes its chunk, closes the file and -releases the lock, which is the mirror image of what the read path does for a -lazy chunk. A store whose chunks are separate objects needs no such -coordination, and asks for no lock. - -For a threaded scheduler the lock is a plain :class:`threading.Lock`. The -workers are threads sharing one address space, so they all hold the same -object, and the identity string is never used. - -For a distributed scheduler the lock is a ``distributed.Lock`` named for the -target. It reaches the workers by being serialised along with the task, and -every copy carries the same name, so the scheduler can make them exclude one -another. A worker behaves like a process, and may run on another machine. - -The "processes" scheduler is refused by :func:`get_worker_lock`. Supporting it -would need a serialisable shared lock that does not require a distributed -scheduler to function. - -Writing to one target from genuinely separate processes is a further step -again. For netCDF it needs libnetcdf built with the parallel access option, -which is uncommon: with an ordinary build, opening a file for writing that is -already open for writing simply raises an access error. Iris does not support -that mode at present. - -.. todo:: Move the maintainer prose above into a ``maintainer-notes`` block. +The "processes" scheduler is refused by :func:`get_worker_lock`, and writing +to one target from genuinely separate processes is not supported. + +.. maintainer-notes:: + + A dataset implementation asks for one lock per target, identified by its + path or URL, and gives it to every write handle it builds. The netCDF write + handle reopens the file, acquires the lock, writes its chunk, closes the + file and releases the lock, which is the mirror image of what the read path + does for a lazy chunk. A store whose chunks are separate objects needs no + such coordination, and asks for no lock. + + For a threaded scheduler the lock is a plain :class:`threading.Lock`. The + workers are threads sharing one address space, so they all hold the same + object, and the identity string is never used. + + For a distributed scheduler the lock is a ``distributed.Lock`` named for + the target. It reaches the workers by being serialised along with the task, + and every copy carries the same name, so the scheduler can make them + exclude one another. A worker behaves like a process, and may run on + another machine. + + Supporting the "processes" scheduler would need a serialisable shared lock + that does not require a distributed scheduler to function. Writing from + genuinely separate processes is a further step again: for netCDF it needs + libnetcdf built with the parallel access option, which is uncommon, since + with an ordinary build opening a file for writing that is already open for + writing simply raises an access error. """ diff --git a/lib/iris/fileformats/cf/_group.py b/lib/iris/fileformats/cf/_group.py index 3cd5e0f215..96af8eb2b3 100644 --- a/lib/iris/fileformats/cf/_group.py +++ b/lib/iris/fileformats/cf/_group.py @@ -12,25 +12,28 @@ On top of that mapping sit one read-only property per variable type -- :attr:`CFGroup.bounds`, :attr:`CFGroup.grid_mappings` and so on -- each of -which filters the mapping by ``isinstance``. They are computed on every access -rather than cached, so a group stays correct when -:class:`~iris.fileformats.cf.CFReader` mutates it during its second pass. A -variable therefore appears in every property whose type it satisfies, and -:attr:`CFGroup.formula_terms` is the one exception to the ``isinstance`` -rule: it filters on ``has_formula_terms()`` instead, because participating in -a formula term is a property of the variable rather than of its class. - -Two attributes sit alongside the variables and are populated by the reader, -not by this module. ``global_attributes`` holds the file's netCDF global -attributes. ``promoted`` records the variables that the reader re-classified -as :class:`~iris.fileformats.cf.CFDataVariable`, which it does for two -distinct reasons: a formula term that names a reference surface is a data -pay-load in its own right, per [CF] Appendix D; and a variable that was -referenced but whose dimensions are not a subset of the referrer's is dropped -from that referrer with a warning, then promoted so that it is not lost -altogether. Promotion is how Iris avoids silently discarding either. - -.. todo:: Move the maintainer prose above into a ``maintainer-notes`` block. +which filters the mapping by ``isinstance``. + +.. maintainer-notes:: + + Those properties are computed on every access rather than cached, so a + group stays correct when :class:`~iris.fileformats.cf.CFReader` mutates it + during its second pass. A variable therefore appears in every property + whose type it satisfies, and :attr:`CFGroup.formula_terms` is the one + exception to the ``isinstance`` rule: it filters on ``has_formula_terms()`` + instead, because participating in a formula term is a property of the + variable rather than of its class. + + Two attributes sit alongside the variables and are populated by the reader, + not by this module. ``global_attributes`` holds the file's netCDF global + attributes. ``promoted`` records the variables that the reader + re-classified as :class:`~iris.fileformats.cf.CFDataVariable`, which it + does for two distinct reasons: a formula term that names a reference + surface is a data pay-load in its own right, per [CF] Appendix D; and a + variable that was referenced but whose dimensions are not a subset of the + referrer's is dropped from that referrer with a warning, then promoted so + that it is not lost altogether. Promotion is how Iris avoids silently + discarding either. """ diff --git a/lib/iris/fileformats/cf/_reader.py b/lib/iris/fileformats/cf/_reader.py index 75d4e85278..848c720036 100644 --- a/lib/iris/fileformats/cf/_reader.py +++ b/lib/iris/fileformats/cf/_reader.py @@ -4,51 +4,53 @@ # See LICENSE in the root of the repository for full licensing details. """Read a netCDF dataset and interpret it as CF-netCDF variables and groups. -:class:`CFReader` is the entry point to the CF layer: give it a file path or -an already-open dataset and it returns an object whose ``cf_group`` is a fully -populated :class:`~iris.fileformats.cf.CFGroup`. All of the work happens in -:meth:`CFReader.__init__`, in two passes over the dataset's variables. - -The first pass, ``_translate``, is classification. It asks each class in -``_variable_types`` to ``identify`` itself among the netCDF variables, then -handles the three special cases that tuple deliberately excludes: -:class:`~iris.fileformats.cf.CFCoordinateVariable` (identified by the netCDF -dimension/variable name coincidence rather than by an attribute), -``_CFFormulaTermsVariable`` (whose members are also something else) and -:class:`~iris.fileformats.cf.CFDataVariable` (what is left over once every -other classification has had its say). It also caches the parse of every -``grid_mapping`` attribute in ``_coord_system_mappings``, mapping a variable -name to the coordinate-system-to-coordinates relationships it declares, so -that the loading rules do not re-parse the same attribute once per coordinate. - -The second pass, ``_build_cf_groups``, is association: for each variable it -builds the sub-:class:`~iris.fileformats.cf.CFGroup` of the variables that -variable references, and checks that a referenced variable's dimensions span -the referrer's. One that does not is not an error -- it is dropped from the -referrer's group with a warning and, if nothing else claims it, *promoted* to -a :class:`~iris.fileformats.cf.CFDataVariable` of its own. A final -``_reset`` clears the attribute touch history that classification dirtied, so -that the loading rules start from a clean record of which netCDF attributes -have been consumed. - -``CFReader`` may or may not own the file it reads, and ``_own_file`` records -which. Constructed from a path, it opens the dataset and owns it; constructed -from an open dataset, it borrows it. ``_close`` -- reached from ``__exit__`` -and from ``__del__`` -- only closes what it owns, so a borrowed dataset -survives garbage collection of the reader. ``_own_file`` is set ``False`` -before anything that can raise, so the destructor is safe even if -``__init__`` fails part way. - -``CFReader`` takes one of three things. A path opens a netCDF file, because -netCDF is the default a bare filename means. An open netCDF4 dataset, or an -emulator of one such as ncdata passes, is wrapped. And a -:class:`~iris.fileformats.cf.dataset.CFDataset` of any format is used as it -stands, which is how a Zarr store reaches this machinery: its loader opens -the store and hands it over, so nothing here has to know that Zarr exists. -Only the first case mentions netCDF, and only because something has to be -the default. - -.. todo:: Move the maintainer prose above into a ``maintainer-notes`` block. +:class:`CFReader` is the entry point to the CF layer. Give it a file path, an +open netCDF4 dataset, or a :class:`~iris.fileformats.cf.dataset.CFDataset` of +any format, and it returns an object whose ``cf_group`` is a fully populated +:class:`~iris.fileformats.cf.CFGroup`. + +.. maintainer-notes:: + + Only the path case mentions netCDF, and only because something has to be + the default that a bare filename means. An open netCDF4 dataset, or an + emulator of one such as ncdata passes, is wrapped. A ``CFDataset`` is used + as it stands, which is how a Zarr store reaches this machinery: its loader + opens the store and hands it over, so nothing here has to know that Zarr + exists. + + All of the work happens in :meth:`CFReader.__init__`, in two passes over + the dataset's variables. + + The first pass, ``_translate``, is classification. It asks each class in + ``_variable_types`` to ``identify`` itself among the netCDF variables, then + handles the three special cases that tuple deliberately excludes: + :class:`~iris.fileformats.cf.CFCoordinateVariable` (identified by the + netCDF dimension/variable name coincidence rather than by an attribute), + ``_CFFormulaTermsVariable`` (whose members are also something else) and + :class:`~iris.fileformats.cf.CFDataVariable` (what is left over once every + other classification has had its say). It also caches the parse of every + ``grid_mapping`` attribute in ``_coord_system_mappings``, mapping a + variable name to the coordinate-system-to-coordinates relationships it + declares, so that the loading rules do not re-parse the same attribute once + per coordinate. + + The second pass, ``_build_cf_groups``, is association: for each variable it + builds the sub-:class:`~iris.fileformats.cf.CFGroup` of the variables that + variable references, and checks that a referenced variable's dimensions + span the referrer's. One that does not is not an error -- it is dropped + from the referrer's group with a warning and, if nothing else claims it, + *promoted* to a :class:`~iris.fileformats.cf.CFDataVariable` of its own. A + final ``_reset`` clears the attribute touch history that classification + dirtied, so that the loading rules start from a clean record of which + netCDF attributes have been consumed. + + ``CFReader`` may or may not own the file it reads, and ``_own_file`` + records which. Constructed from a path, it opens the dataset and owns it; + constructed from an open dataset, it borrows it. ``_close`` -- reached from + ``__exit__`` and from ``__del__`` -- only closes what it owns, so a + borrowed dataset survives garbage collection of the reader. ``_own_file`` + is set ``False`` before anything that can raise, so the destructor is safe + even if ``__init__`` fails part way. """ diff --git a/lib/iris/fileformats/cf/_variables.py b/lib/iris/fileformats/cf/_variables.py index b12d58c8e0..5ebaf4013a 100644 --- a/lib/iris/fileformats/cf/_variables.py +++ b/lib/iris/fileformats/cf/_variables.py @@ -10,39 +10,40 @@ names a coordinate system variable, and so on. This module is where those attributes are read and each variable is given a class that says what it is. -:class:`CFVariable` is the abstract base and defines the contract. A subclass -declares which netCDF attribute names it -- ``cf_identity``, a single -attribute name, or ``cf_identities``, a list of them for the classes that -answer to several -- and implements ``identify``. ``identify`` is a -classmethod, not an instance method: it is called on the class, is handed the -whole ``{name: CFDatasetVariable}`` mapping of the file, and returns the -subset of it that belongs to that class, as a ``{name: CFVariable instance}`` -mapping. ``ignore`` and ``target`` narrow what it looks at; ``warn`` gates -whether it complains. - -Classification is deliberately forgiving. A variable that names a variable -which is not in the file, or names one whose dimensions make no sense for the -role, produces a warning -- :class:`~iris.warnings.IrisCfMissingVarWarning` -and friends -- and is skipped. It does not raise. Iris is expected to load -what it can from a file that is only mostly conformant, and a hard failure -here would make a single bad attribute cost the user the whole file. - -The CF-UGRID classes for unstructured meshes live here rather than in -:mod:`iris.mesh` because they are classifiers, not data: they answer the same -``identify`` contract as every other class in this module and are driven by -the same pass in :class:`~iris.fileformats.cf.CFReader`. What they identify is -handed to :mod:`iris.mesh` to build the mesh proper. - -This module opens no files. It is given netCDF variables and returns -classifications; :mod:`iris.fileformats.cf._group` collects them and -:mod:`iris.fileformats.cf._reader` drives the whole thing. It does read array -data in exactly one place: ``CFCoordinateVariable.identify`` fetches a -candidate's values when called with ``monotonic=True``, because whether a -coordinate is monotonic cannot be told from its metadata. That is the only -path here that touches data, and it is reached only from -``CFReader(..., monotonic=True)``. - -.. todo:: Move the maintainer prose above into a ``maintainer-notes`` block. +.. maintainer-notes:: + + :class:`CFVariable` is the abstract base and defines the contract. A + subclass declares which netCDF attribute names it -- ``cf_identity``, a + single attribute name, or ``cf_identities``, a list of them for the classes + that answer to several -- and implements ``identify``. ``identify`` is a + classmethod, not an instance method: it is called on the class, is handed + the whole ``{name: CFDatasetVariable}`` mapping of the file, and returns + the subset of it that belongs to that class, as a + ``{name: CFVariable instance}`` mapping. ``ignore`` and ``target`` narrow + what it looks at; ``warn`` gates whether it complains. + + Classification is deliberately forgiving. A variable that names a variable + which is not in the file, or names one whose dimensions make no sense for + the role, produces a warning -- + :class:`~iris.warnings.IrisCfMissingVarWarning` and friends -- and is + skipped. It does not raise. Iris is expected to load what it can from a + file that is only mostly conformant, and a hard failure here would make a + single bad attribute cost the user the whole file. + + The CF-UGRID classes for unstructured meshes live here rather than in + :mod:`iris.mesh` because they are classifiers, not data: they answer the + same ``identify`` contract as every other class in this module and are + driven by the same pass in :class:`~iris.fileformats.cf.CFReader`. What + they identify is handed to :mod:`iris.mesh` to build the mesh proper. + + This module opens no files. It is given netCDF variables and returns + classifications; :mod:`iris.fileformats.cf._group` collects them and + :mod:`iris.fileformats.cf._reader` drives the whole thing. It does read + array data in exactly one place: ``CFCoordinateVariable.identify`` fetches + a candidate's values when called with ``monotonic=True``, because whether a + coordinate is monotonic cannot be told from its metadata. That is the only + path here that touches data, and it is reached only from + ``CFReader(..., monotonic=True)``. """ diff --git a/lib/iris/fileformats/cf/dataset.py b/lib/iris/fileformats/cf/dataset.py index bcbd93be98..85b4e73141 100644 --- a/lib/iris/fileformats/cf/dataset.py +++ b/lib/iris/fileformats/cf/dataset.py @@ -14,25 +14,27 @@ :class:`~iris.fileformats.cf.CFVariable` classes, the CF loader and the CF saver actually need, with one implementation per storage format. -These classes are more restrictive than a netCDF variable about what can be -reached through attribute syntax. netCDF presents ``units``, a CF attribute -read from the file, and ``dimensions``, a property of the storage, the same -way, and a caller cannot tell which is which. Here the property ``attributes`` -is the only route to a CF attribute, and every storage property is a named, -typed member. - -The property ``attributes`` holds an open-ended set of data keys read from a -file, with no schema to enumerate, which is why :class:`TrackedAttributes` -exists and why :meth:`~iris.fileformats.cf.CFVariable.__getattr__` forwards to -it. Tracking is the load-bearing part: Iris decides which file attributes -survive onto a loaded cube by asking which ones the loading rules did *not* -read, so a read that goes unrecorded silently leaks a CF-reserved attribute -onto the cube. - -See sections 4.2, 4.3 and 4.5 of -``docs/superpowers/specs/2026-09-21-zarr-io-design.md``. - -.. todo:: Move the maintainer prose above into a ``maintainer-notes`` block. +The property ``attributes`` is the only route to a CF attribute, and every +storage property is a named, typed member. + +.. maintainer-notes:: + + That split makes these classes more restrictive than a netCDF variable + about what can be reached through attribute syntax. netCDF presents + ``units``, a CF attribute read from the file, and ``dimensions``, a + property of the storage, the same way, and a caller cannot tell which is + which. + + ``attributes`` holds an open-ended set of data keys read from a file, with + no schema to enumerate, which is why :class:`TrackedAttributes` exists and + why :meth:`~iris.fileformats.cf.CFVariable.__getattr__` forwards to it. + Tracking is the load-bearing part: Iris decides which file attributes + survive onto a loaded cube by asking which ones the loading rules did *not* + read, so a read that goes unrecorded silently leaks a CF-reserved attribute + onto the cube. + + See sections 4.2, 4.3 and 4.5 of + ``docs/superpowers/specs/2026-09-21-zarr-io-design.md``. """ diff --git a/lib/iris/fileformats/cf/loader.py b/lib/iris/fileformats/cf/loader.py index c4b6f2bbd7..7145f3b24e 100644 --- a/lib/iris/fileformats/cf/loader.py +++ b/lib/iris/fileformats/cf/loader.py @@ -9,26 +9,28 @@ API reference -This module is the CF half of what used to be -``iris.fileformats.netcdf.loader``. It takes the -:class:`~iris.fileformats.cf.CFVariable` objects a +Takes the :class:`~iris.fileformats.cf.CFVariable` objects a :class:`~iris.fileformats.cf.CFReader` produced, runs the loading rules over -them and returns :class:`~iris.cube.Cube` objects. None of that is specific to -netCDF, and nothing here imports netCDF4 - though one deferred import still -reaches back into :mod:`iris.fileformats.netcdf` for ``loader.DEBUG``, which -never moves (see below). It is commented at its own call site with why. - -Where a storage format *does* show through - reading a variable's array - the -work is delegated to :meth:`~iris.fileformats.cf.dataset.CFDatasetVariable.read_data`, -and this module supplies only the chunking policy to apply if the result comes -back lazy. +them and returns :class:`~iris.cube.Cube` objects. Opening files remains the format's own business: see :func:`iris.fileformats.netcdf.load_cubes`. Also : `CF Conventions `_. -.. todo:: Move the maintainer prose above into a ``maintainer-notes`` block. +.. maintainer-notes:: + + This module is the CF half of what used to be + ``iris.fileformats.netcdf.loader``. None of what it does is specific to + netCDF, and nothing here imports netCDF4 - though one deferred import still + reaches back into :mod:`iris.fileformats.netcdf` for ``loader.DEBUG``, + which never moves. It is commented at its own call site with why. + + Where a storage format *does* show through - reading a variable's array - + the work is delegated to + :meth:`~iris.fileformats.cf.dataset.CFDatasetVariable.read_data`, and this + module supplies only the chunking policy to apply if the result comes back + lazy. """ diff --git a/lib/iris/fileformats/cf/saver.py b/lib/iris/fileformats/cf/saver.py index 5541754002..c8924c2185 100644 --- a/lib/iris/fileformats/cf/saver.py +++ b/lib/iris/fileformats/cf/saver.py @@ -11,46 +11,50 @@ Everything about writing a cube that does not depend on what it is written into. The CF data model is here - dimension naming, coordinate and bounds -variables, cell methods, grid mappings, meshes, attribute compliance. Every -storage call goes through :class:`~iris.fileformats.cf.dataset.CFDataset`, -except where a store's own ``_grid_mapping_writer`` writes somewhere that is -not part of it; netCDF's override does that, and every other store's does not. - -A format supplies a subclass of ``_CFSaver`` holding its own vocabulary of -storage keywords, and whatever of the five hooks it needs: -``_create_string_variable``, ``_backend_write``, ``_flush_before_close``, -``_patch_dataset`` and ``_grid_mapping_writer``. -:class:`iris.fileformats.netcdf.saver.Saver` is one such subclass, and is where -a user of the netCDF format meets all of this. - -``_CFSaver`` is private. What a format must supply is the five hooks, which -are private too, and one subclass is not enough evidence to promise their -shape; everything public on the class is the caller's API, inherited -unchanged by each format's own public saver. Contrast -:class:`~iris.fileformats.cf.dataset.CFDataset`, which is public because every -member of it is something a format must implement, and because a user can -reach one through :attr:`iris.fileformats.cf.CFVariable.cf_data`. - -Nothing here is netCDF-specific any longer: the last thing that was is now a -hook like the rest. - -``_add_grid_mapping_to_dataset`` writes each CF grid-mapping parameter through -the function ``_grid_mapping_writer`` hands back, so that where a parameter -lands is visible at the point it is written. netCDF's function writes onto 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 -writes into the ``attributes`` mapping. - -``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. +variables, cell methods, grid mappings, meshes, attribute compliance. + +Each storage format supplies its own public saver on top: +:class:`iris.fileformats.netcdf.saver.Saver` is where a user of the netCDF +format meets all of this. See : `CF Conventions `_. -.. todo:: Move the maintainer prose above into a ``maintainer-notes`` block. +.. maintainer-notes:: + + Every storage call goes through + :class:`~iris.fileformats.cf.dataset.CFDataset`, except where a store's own + ``_grid_mapping_writer`` writes somewhere that is not part of it; netCDF's + override does that, and every other store's does not. + + A format supplies a subclass of ``_CFSaver`` holding its own vocabulary of + storage keywords, and whatever of the five hooks it needs: + ``_create_string_variable``, ``_backend_write``, ``_flush_before_close``, + ``_patch_dataset`` and ``_grid_mapping_writer``. + + ``_CFSaver`` is private. What a format must supply is the five hooks, which + are private too, and one subclass is not enough evidence to promise their + shape; everything public on the class is the caller's API, inherited + unchanged by each format's own public saver. Contrast + :class:`~iris.fileformats.cf.dataset.CFDataset`, which is public because + every member of it is something a format must implement, and because a user + can reach one through :attr:`iris.fileformats.cf.CFVariable.cf_data`. + + Nothing here is netCDF-specific any longer: the last thing that was is now + a hook like the rest. + + ``_add_grid_mapping_to_dataset`` writes each CF grid-mapping parameter + through the function ``_grid_mapping_writer`` hands back, so that where a + parameter lands is visible at the point it is written. netCDF's function + writes onto 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 + writes into the ``attributes`` mapping. + + ``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. """ diff --git a/lib/iris/fileformats/netcdf/_dataset.py b/lib/iris/fileformats/netcdf/_dataset.py index 5a6e8a066c..2a93bde6bd 100644 --- a/lib/iris/fileformats/netcdf/_dataset.py +++ b/lib/iris/fileformats/netcdf/_dataset.py @@ -11,16 +11,16 @@ ``createDimension``, ``filepath()``, ``isopen()`` - either has a named member on the interface or lives here as a netCDF-only member. -Reading and writing share one class, and the read path must not touch -:attr:`NetCDFDataset.write_lock`: making the lock raises for Dask schedulers -that only the saver cares about, so it is made on first use rather than at -construction. :meth:`NetCDFDataset.from_existing` serves both paths and -cannot tell them apart, so it stipulates an open mode rather than observing -one, and always wraps for byte encoding. +.. maintainer-notes:: -See section 4.5 of ``docs/superpowers/specs/2026-09-21-zarr-io-design.md``. + Reading and writing share one class, and the read path must not touch + :attr:`NetCDFDataset.write_lock`: making the lock raises for Dask + schedulers that only the saver cares about, so it is made on first use + rather than at construction. :meth:`NetCDFDataset.from_existing` serves + both paths and cannot tell them apart, so it stipulates an open mode rather + than observing one, and always wraps for byte encoding. -.. todo:: Move the maintainer prose above into a ``maintainer-notes`` block. + See section 4.5 of ``docs/superpowers/specs/2026-09-21-zarr-io-design.md``. """ diff --git a/lib/iris/fileformats/netcdf/loader.py b/lib/iris/fileformats/netcdf/loader.py index ba0e433fc3..01efa4d351 100644 --- a/lib/iris/fileformats/netcdf/loader.py +++ b/lib/iris/fileformats/netcdf/loader.py @@ -15,16 +15,17 @@ API reference -What is left here is what is genuinely netCDF's: opening the file sources, and -the proxy class that reads from them. Interpreting what is inside is -:mod:`iris.fileformats.cf.loader`'s job. +Interpreting what is inside a file is :mod:`iris.fileformats.cf.loader`'s job. See : `NetCDF User's Guide `_ and `netCDF4 python module `_. Also : `CF Conventions `_. -.. todo:: Move the maintainer prose above into a ``maintainer-notes`` block. +.. maintainer-notes:: + + What is left here is what is genuinely netCDF's: opening the file sources, + and the proxy class that reads from them. """ diff --git a/lib/iris/fileformats/zarr/__init__.py b/lib/iris/fileformats/zarr/__init__.py index 1a4c398509..f4dec9707e 100644 --- a/lib/iris/fileformats/zarr/__init__.py +++ b/lib/iris/fileformats/zarr/__init__.py @@ -18,15 +18,15 @@ this package until a Zarr target is actually loaded, so Iris installs and imports without it. -This package shadows the third-party ``zarr`` distribution by name. Python 3 -resolves ``import zarr`` absolutely, so inside these modules it still finds the -third-party package; xarray's ``xarray.backends.zarr`` has the same -arrangement. +.. maintainer-notes:: -See sections 4.1 and 4.4 of -``docs/superpowers/specs/2026-09-21-zarr-io-design.md``. + This package shadows the third-party ``zarr`` distribution by name. Python + 3 resolves ``import zarr`` absolutely, so inside these modules it still + finds the third-party package; xarray's ``xarray.backends.zarr`` has the + same arrangement. -.. todo:: Move the maintainer prose above into a ``maintainer-notes`` block. + See sections 4.1 and 4.4 of + ``docs/superpowers/specs/2026-09-21-zarr-io-design.md``. """ diff --git a/lib/iris/fileformats/zarr/_dataset.py b/lib/iris/fileformats/zarr/_dataset.py index 4070d08f9e..70694f55e0 100644 --- a/lib/iris/fileformats/zarr/_dataset.py +++ b/lib/iris/fileformats/zarr/_dataset.py @@ -9,48 +9,50 @@ are recorded, what a chunk is, what the store's own fill value is - either has a named member on the interface or lives here as a Zarr-only member. -Three things about Zarr shape this module, and each is a trap if forgotten. - -**A dimension name guarantees less than a netCDF one.** The version 3 core -specification makes ``dimension_names`` optional, allows a null entry, only -*recommends* that names are distinct within an array, and places no -restriction at all on reusing a name across arrays. All of that is conforming -Zarr, so none of it may raise. A dimension with no name is **anonymous**, which -Iris has always had: a cube dimension carrying no coordinate. - -**``Array.fill_value`` is not ``_FillValue``.** It says what unwritten storage -reads back as; it does not say the value is missing. zarr-python defaults it to -zero, so reading it as a missing-value sentinel masks every valid zero in an -array that never declared one. :attr:`ZarrDatasetVariable.fill_value` reports -it as the storage fact it is, and the masking decision is made in -:mod:`iris.fileformats.zarr._decode` from the CF attributes. - -**The read unit is the inner chunk, even when the array is sharded.** Zarr's -sharding codec is indexed, so a reader fetches only the inner chunks it needs. -:attr:`ZarrDatasetVariable.chunking` therefore returns ``Array.chunks``, never -``Array.shards``. - -**An array is created in the platform's own byte order**, whatever order it -was asked for, and NumPy converts the values on assignment. Zarr version 3 -has no big-endian data type: endianness lives in the ``bytes`` codec, which -zarr-python leaves at ``little``, so asking for ``>f4`` writes the same store -as asking for ``f4`` -cube as ``float32``, and a PP or GRIB load hands the saver big-endian arrays -routinely. - -Chunking is sized by :func:`iris._lazy_data._optimum_chunksize`, the same way -the netCDF path sizes it, and is not then adjusted to the store's own -chunking. Iris warns when a stored chunk is larger than Dask's target, from -:func:`iris.fileformats.cf.loader._chunks_from_chunk_control`, and leaves the -choice to ``CHUNK_CONTROL``. - -See sections 4.2, 4.4 and 4.6 of -``docs/superpowers/specs/2026-09-21-zarr-io-design.md``. - -.. todo:: Move the maintainer prose above into a ``maintainer-notes`` block. +.. maintainer-notes:: + + Four things about Zarr shape this module, and each is a trap if forgotten. + + **A dimension name guarantees less than a netCDF one.** The version 3 core + specification makes ``dimension_names`` optional, allows a null entry, only + *recommends* that names are distinct within an array, and places no + restriction at all on reusing a name across arrays. All of that is + conforming Zarr, so none of it may raise. A dimension with no name is + **anonymous**, which Iris has always had: a cube dimension carrying no + coordinate. + + **``Array.fill_value`` is not ``_FillValue``.** It says what unwritten + storage reads back as; it does not say the value is missing. zarr-python + defaults it to zero, so reading it as a missing-value sentinel masks every + valid zero in an array that never declared one. + :attr:`ZarrDatasetVariable.fill_value` reports it as the storage fact it + is, and the masking decision is made in + :mod:`iris.fileformats.zarr._decode` from the CF attributes. + + **The read unit is the inner chunk, even when the array is sharded.** + Zarr's sharding codec is indexed, so a reader fetches only the inner chunks + it needs. :attr:`ZarrDatasetVariable.chunking` therefore returns + ``Array.chunks``, never ``Array.shards``. + + **An array is created in the platform's own byte order**, whatever order it + was asked for, and NumPy converts the values on assignment. Zarr version 3 + has no big-endian data type: endianness lives in the ``bytes`` codec, which + zarr-python leaves at ``little``, so asking for ``>f4`` writes the same + store as asking for ``f4`` cube as ``float32``, and a PP or GRIB load hands the + saver big-endian arrays routinely. + + Chunking is sized by :func:`iris._lazy_data._optimum_chunksize`, the same + way the netCDF path sizes it, and is not then adjusted to the store's own + chunking. Iris warns when a stored chunk is larger than Dask's target, from + :func:`iris.fileformats.cf.loader._chunks_from_chunk_control`, and leaves + the choice to ``CHUNK_CONTROL``. + + See sections 4.2, 4.4 and 4.6 of + ``docs/superpowers/specs/2026-09-21-zarr-io-design.md``. """ diff --git a/lib/iris/fileformats/zarr/_decode.py b/lib/iris/fileformats/zarr/_decode.py index 6c664c7d8f..a6497b9b36 100644 --- a/lib/iris/fileformats/zarr/_decode.py +++ b/lib/iris/fileformats/zarr/_decode.py @@ -8,46 +8,49 @@ ``valid_min``/``valid_max``/``valid_range`` masking and ``scale_factor``/``add_offset`` unpacking on every read. Zarr returns a plain ``numpy.ndarray`` and applies none of it, so these rules are implemented here. -They live on the Zarr side rather than in the shared CF layer because the -netCDF path gets the behaviour from the C library and has nothing to reuse. - -Everything here is a pure function of values, with no store and no Dask in -sight. :func:`mask_and_scale` is what the loader lays over a raw array as a -``map_blocks`` layer - visible in the graph, fusable with its neighbours, and -testable on a plain ndarray. - -Three names are this module's surface: :func:`decode_parameters`, -:func:`decoding_required` and :func:`mask_and_scale`. Everything else is a -step of one of those, and carries a leading underscore to say so - the module -being private already does not make the distinction worthless, since -``_dataset.py`` should not be reaching for a decode step on its own. - -Two traps, both from the Zarr specification rather than from CF. - -**A floating ``_FillValue`` is base64 on version 3.** Released xarray encodes -it as base64 of a little-endian ``float64`` for *every* floating dtype, -including ``float32``, and its reader requires that form. It is the de-facto -encoding, not a producer's mistake, so Iris reads both it and a plain JSON -number. CF section 2.5.1 asks for "the scalar attribute with the name -``_FillValue`` and of the same type as its variable"; this is a deliberate, -narrow deviation, taken because interoperability with the other Zarr readers -is the reason for native Zarr support. - -**``Array.fill_value`` is not a missing-value declaration.** It says what -unwritten storage reads back as, and zarr-python defaults it to zero. On -version 3 it is never a mask source: an ``int16`` array of ``[0, 1, 2]`` with -no CF attributes would otherwise load with its valid zero masked. On version 2 -it is one, but only when no CF attribute is present, because version 2 stores -predate the convention - which is what released xarray's -``use_zarr_fill_value_as_mask`` does. zarr-python 3 defaults the field to zero -for a version 2 store too, so a store written today can lose a genuine zero -this way; :func:`decode_parameters` warns for exactly that case - the storage -fill value is zero - and not for a non-zero one, which is the writer's own -deliberate, pre-convention sentinel and not something to warn about. - -See section 4.4 of ``docs/superpowers/specs/2026-09-21-zarr-io-design.md``. - -.. todo:: Move the maintainer prose above into a ``maintainer-notes`` block. + +.. maintainer-notes:: + + The rules live on the Zarr side rather than in the shared CF layer because + the netCDF path gets the behaviour from the C library and has nothing to + reuse. + + Everything here is a pure function of values, with no store and no Dask in + sight. :func:`mask_and_scale` is what the loader lays over a raw array as a + ``map_blocks`` layer - visible in the graph, fusable with its neighbours, + and testable on a plain ndarray. + + Three names are this module's surface: :func:`decode_parameters`, + :func:`decoding_required` and :func:`mask_and_scale`. Everything else is a + step of one of those, and carries a leading underscore to say so - the + module being private already does not make the distinction worthless, + since ``_dataset.py`` should not be reaching for a decode step on its own. + + Two traps, both from the Zarr specification rather than from CF. + + **A floating ``_FillValue`` is base64 on version 3.** Released xarray + encodes it as base64 of a little-endian ``float64`` for *every* floating + dtype, including ``float32``, and its reader requires that form. It is the + de-facto encoding, not a producer's mistake, so Iris reads both it and a + plain JSON number. CF section 2.5.1 asks for "the scalar attribute with the + name ``_FillValue`` and of the same type as its variable"; this is a + deliberate, narrow deviation, taken because interoperability with the other + Zarr readers is the reason for native Zarr support. + + **``Array.fill_value`` is not a missing-value declaration.** It says what + unwritten storage reads back as, and zarr-python defaults it to zero. On + version 3 it is never a mask source: an ``int16`` array of ``[0, 1, 2]`` + with no CF attributes would otherwise load with its valid zero masked. On + version 2 it is one, but only when no CF attribute is present, because + version 2 stores predate the convention - which is what released xarray's + ``use_zarr_fill_value_as_mask`` does. zarr-python 3 defaults the field to + zero for a version 2 store too, so a store written today can lose a genuine + zero this way; :func:`decode_parameters` warns for exactly that case - the + storage fill value is zero - and not for a non-zero one, which is the + writer's own deliberate, pre-convention sentinel and not something to warn + about. + + See section 4.4 of ``docs/superpowers/specs/2026-09-21-zarr-io-design.md``. """ diff --git a/lib/iris/fileformats/zarr/_encode.py b/lib/iris/fileformats/zarr/_encode.py index 426c392d53..565518d482 100644 --- a/lib/iris/fileformats/zarr/_encode.py +++ b/lib/iris/fileformats/zarr/_encode.py @@ -4,34 +4,39 @@ # See LICENSE in the root of the repository for full licensing details. """Value conversions a Zarr store needs on the way in. -The mirror of :mod:`iris.fileformats.zarr._decode`: pure functions of their -arguments, with no zarr-python call and no Iris object between them, so that -every rule can be tested on a plain value. - -**Attribute values are converted before zarr sees them, never after.** A value -zarr's JSON encoder rejects raises, *and stays in the attributes mapping*, so a -caller that converts on failure has already corrupted the store's metadata. - -**``_FillValue`` and ``missing_value`` are encoded differently, and the -difference is not Iris's.** On a floating array, released xarray reads -``_FillValue`` through zarr's own fill-value parser, which wants the base64 of -a little-endian ``float64`` and fails the *whole dataset open* on a plain JSON -number. It reads ``missing_value`` as an ordinary attribute, where base64 is a -string that masks nothing. So the first is packed and the second is not. See -§4.5 of the native Zarr I/O design, -``docs/superpowers/specs/2026-09-21-zarr-io-design.md``. - -**Packing is applied here, not assumed from the backend.** netCDF4-python -applies ``scale_factor``/``add_offset`` packing itself, on assignment, once -those attributes are set on the variable - which is why the generic CF saver -only ever writes the attributes and streams the unpacked values through -unchanged. Zarr has no such behaviour, so :func:`pack` does the arithmetic -that would otherwise silently not happen. It is the mirror of -:func:`iris.fileformats.zarr._decode.unpack`, called from -:meth:`iris.fileformats.zarr._dataset.ZarrDatasetVariable.__setitem__` before -the fill described above, because the fill value lives in *packed* space and -the data arrives in *unpacked* space - packing an already-filled sentinel -would put it through the same transform as real data and lose it. +The mirror of :mod:`iris.fileformats.zarr._decode`. + +.. maintainer-notes:: + + Every rule here is a pure function of its arguments, with no zarr-python + call and no Iris object between them, so that it can be tested on a plain + value. + + **Attribute values are converted before zarr sees them, never after.** A + value zarr's JSON encoder rejects raises, *and stays in the attributes + mapping*, so a caller that converts on failure has already corrupted the + store's metadata. + + **``_FillValue`` and ``missing_value`` are encoded differently, and the + difference is not Iris's.** On a floating array, released xarray reads + ``_FillValue`` through zarr's own fill-value parser, which wants the base64 + of a little-endian ``float64`` and fails the *whole dataset open* on a plain + JSON number. It reads ``missing_value`` as an ordinary attribute, where + base64 is a string that masks nothing. So the first is packed and the second + is not. See §4.5 of the native Zarr I/O design, + ``docs/superpowers/specs/2026-09-21-zarr-io-design.md``. + + **Packing is applied here, not assumed from the backend.** netCDF4-python + applies ``scale_factor``/``add_offset`` packing itself, on assignment, once + those attributes are set on the variable - which is why the generic CF saver + only ever writes the attributes and streams the unpacked values through + unchanged. Zarr has no such behaviour, so :func:`pack` does the arithmetic + that would otherwise silently not happen. It is the mirror of + :func:`iris.fileformats.zarr._decode.unpack`, called from + :meth:`iris.fileformats.zarr._dataset.ZarrDatasetVariable.__setitem__` + before the fill described above, because the fill value lives in *packed* + space and the data arrives in *unpacked* space - packing an already-filled + sentinel would put it through the same transform as real data and lose it. """ diff --git a/lib/iris/fileformats/zarr/loader.py b/lib/iris/fileformats/zarr/loader.py index 54563c0db1..71a1b7270c 100644 --- a/lib/iris/fileformats/zarr/loader.py +++ b/lib/iris/fileformats/zarr/loader.py @@ -9,29 +9,30 @@ API reference -A thin loop: open the store as a -:class:`~iris.fileformats.zarr._dataset.ZarrDataset`, hand it to -:class:`~iris.fileformats.cf.CFReader`, and build one cube per CF data -variable with :mod:`iris.fileformats.cf.loader` - the same machinery the -netCDF loader runs. Everything Zarr-specific happens below this module, in -:mod:`iris.fileformats.zarr._dataset`. - Iris loads **one** group per call. A Zarr store can legitimately hold unrelated datasets, and merging them into one ``CubeList`` would be wrong, so nested groups are not walked and ``group=`` selects which one to read. UGRID meshes load from a Zarr store exactly as they do from a netCDF file. -Nothing in the mesh machinery is netCDF-specific - it reads a -:class:`~iris.fileformats.cf.CFReader`, and a `CFReader` over a `ZarrDataset` -answers the same questions - so this module calls into -:mod:`iris.fileformats.netcdf.ugrid_load` rather than growing a second copy. -That module's name is the historical home of the code, not a statement about -what it can read. -See sections 4.6 and 4.7 of -``docs/superpowers/specs/2026-09-21-zarr-io-design.md``. +.. maintainer-notes:: + + A thin loop: open the store as a + :class:`~iris.fileformats.zarr._dataset.ZarrDataset`, hand it to + :class:`~iris.fileformats.cf.CFReader`, and build one cube per CF data + variable with :mod:`iris.fileformats.cf.loader` - the same machinery the + netCDF loader runs. Everything Zarr-specific happens below this module, in + :mod:`iris.fileformats.zarr._dataset`. + + Nothing in the mesh machinery is netCDF-specific - it reads a + :class:`~iris.fileformats.cf.CFReader`, and a ``CFReader`` over a + ``ZarrDataset`` answers the same questions - so this module calls into + :mod:`iris.fileformats.netcdf.ugrid_load` rather than growing a second + copy. That module's name is the historical home of the code, not a + statement about what it can read. -.. todo:: Move the maintainer prose above into a ``maintainer-notes`` block. + See sections 4.6 and 4.7 of + ``docs/superpowers/specs/2026-09-21-zarr-io-design.md``. """ diff --git a/lib/iris/fileformats/zarr/saver.py b/lib/iris/fileformats/zarr/saver.py index 73cd4798a3..cf83b23058 100644 --- a/lib/iris/fileformats/zarr/saver.py +++ b/lib/iris/fileformats/zarr/saver.py @@ -17,14 +17,20 @@ **The storage keywords are zarr-python's own**: ``chunks``, ``shards``, ``compressors``, ``filters``, ``serializer``. A netCDF keyword raises and names -the Zarr replacement. Translating ``zlib=True`` onto a Blosc codec would sell a -false equivalence, and would hand this new API the shape the netCDF saver is -stuck with. See §4.5 of the native Zarr I/O design, -``docs/superpowers/specs/2026-09-21-zarr-io-design.md``. +the Zarr replacement. **The default mode is ``"w-"``**, which refuses an existing target, where the -netCDF saver clobbers one. A Zarr clobber is many non-atomic deletes, so an -interrupted one destroys the old store without completing the new. +netCDF saver clobbers one. + +.. maintainer-notes:: + + Translating ``zlib=True`` onto a Blosc codec would sell a false + equivalence, and would hand this new API the shape the netCDF saver is + stuck with. See §4.5 of the native Zarr I/O design, + ``docs/superpowers/specs/2026-09-21-zarr-io-design.md``. + + ``"w-"`` is the default because a Zarr clobber is many non-atomic deletes, + so an interrupted one destroys the old store without completing the new. """ diff --git a/lib/iris/io/format_picker.py b/lib/iris/io/format_picker.py index f92c52e1da..dc04fd549e 100644 --- a/lib/iris/io/format_picker.py +++ b/lib/iris/io/format_picker.py @@ -44,8 +44,6 @@ The calling sequence of handler is dependent on the function given in the original specification and can be customised to your project's needs. -.. todo:: Move the maintainer prose above into a ``maintainer-notes`` block. - """ from collections.abc import Callable From d8ce36b5ccc0dd4141c78c57407d1c41c704e59c Mon Sep 17 00:00:00 2001 From: Martin Yeo Date: Tue, 27 Oct 2026 14:27:25 +0000 Subject: [PATCH 8/9] Return three docstring passages to the reader who needs them The non-atomic clobber behind the "w-" default is a data-loss warning, not a rationale: a user choosing mode="w" needs to know an interrupted Zarr clobber destroys the old store without completing the new. iris.fileformats.netcdf.loader had one thought split across the boundary. What is left in the module and what moved to the CF loader is one statement, so it goes back whole, into the block. Naming only the netCDF saver implied it was the only one; there are two public _CFSaver subclasses. Co-Authored-By: Claude Opus 5 Session-Id: ee18e927-ba60-4705-aee1-029036b34706 --- lib/iris/fileformats/cf/saver.py | 4 ++-- lib/iris/fileformats/netcdf/loader.py | 5 ++--- lib/iris/fileformats/zarr/saver.py | 6 ++---- 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/lib/iris/fileformats/cf/saver.py b/lib/iris/fileformats/cf/saver.py index c8924c2185..54bad548ff 100644 --- a/lib/iris/fileformats/cf/saver.py +++ b/lib/iris/fileformats/cf/saver.py @@ -14,8 +14,8 @@ variables, cell methods, grid mappings, meshes, attribute compliance. Each storage format supplies its own public saver on top: -:class:`iris.fileformats.netcdf.saver.Saver` is where a user of the netCDF -format meets all of this. +:class:`iris.fileformats.netcdf.saver.Saver`, for example, is where a user of +the netCDF format meets all of this. See : `CF Conventions `_. diff --git a/lib/iris/fileformats/netcdf/loader.py b/lib/iris/fileformats/netcdf/loader.py index 01efa4d351..bdf147b5c0 100644 --- a/lib/iris/fileformats/netcdf/loader.py +++ b/lib/iris/fileformats/netcdf/loader.py @@ -15,8 +15,6 @@ API reference -Interpreting what is inside a file is :mod:`iris.fileformats.cf.loader`'s job. - See : `NetCDF User's Guide `_ and `netCDF4 python module `_. @@ -25,7 +23,8 @@ .. maintainer-notes:: What is left here is what is genuinely netCDF's: opening the file sources, - and the proxy class that reads from them. + and the proxy class that reads from them. Interpreting what is inside is + :mod:`iris.fileformats.cf.loader`'s job. """ diff --git a/lib/iris/fileformats/zarr/saver.py b/lib/iris/fileformats/zarr/saver.py index cf83b23058..d119eabaad 100644 --- a/lib/iris/fileformats/zarr/saver.py +++ b/lib/iris/fileformats/zarr/saver.py @@ -20,7 +20,8 @@ the Zarr replacement. **The default mode is ``"w-"``**, which refuses an existing target, where the -netCDF saver clobbers one. +netCDF saver clobbers one. A Zarr clobber is many non-atomic deletes, so an +interrupted one destroys the old store without completing the new. .. maintainer-notes:: @@ -29,9 +30,6 @@ stuck with. See §4.5 of the native Zarr I/O design, ``docs/superpowers/specs/2026-09-21-zarr-io-design.md``. - ``"w-"`` is the default because a Zarr clobber is many non-atomic deletes, - so an interrupted one destroys the old store without completing the new. - """ import dask From 2226b28b941a2df4ccc7d9bd1ad685be4354c1d1 Mon Sep 17 00:00:00 2001 From: Martin Yeo Date: Tue, 27 Oct 2026 14:31:50 +0000 Subject: [PATCH 9/9] Trim the doctest guidance, and stop it over-claiming The run-before-finishing rule keeps one clause of why and drops the rest. The clause earns its place: without it an agent reasons that its change cannot reach a doctest and skips the run, which is the rule this replaced. The checklist item said a doctest you never looked at "is the one that breaks", which implies one you have looked at is already safe. It now states what is true instead: nothing in the pytest suite covers them. Co-Authored-By: Claude Opus 5 Session-Id: ee18e927-ba60-4705-aee1-029036b34706 --- lib/iris/tests/AGENTS.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/iris/tests/AGENTS.md b/lib/iris/tests/AGENTS.md index 6a658eec5c..9730c96717 100644 --- a/lib/iris/tests/AGENTS.md +++ b/lib/iris/tests/AGENTS.md @@ -100,9 +100,9 @@ does: | `cd docs/src && make doctest` | skips the gallery rebuild; faster, not identical to CI | Both take minutes. Run one once before finishing, whether or not you edited a -`>>>` — a doctest asserts real output, so a change anywhere in the library or -the guide can break one, and nothing else in the suite would tell you. Only a -change confined to `lib/iris/tests/` or to `changelog/` is exempt. +`>>>`: a change anywhere can break one, and nothing else in the suite would +tell you. Only a change confined to `lib/iris/tests/` or `changelog/` is +exempt. Do not substitute `pytest --doctest-modules` or `python -m doctest`: 150 `testsetup::` / `testcode::` directives supply @@ -158,8 +158,8 @@ real. - The changed tests pass locally. - **Doctests were run**, unless the change stayed inside `lib/iris/tests/` or - `changelog/`. The pytest suite does not cover them, so a `>>>` you never - looked at is the one that breaks. See [Doctests](#doctests) above. + `changelog/`. Nothing in the pytest suite covers them. See + [Doctests](#doctests) above. - No unrelated tests were modified. - Any required external data setup is documented in the change notes. - **Read the skip count, not just the failures.** Comparing failures before