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"} diff --git a/lib/iris/AGENTS.md b/lib/iris/AGENTS.md index 422d487ff7..03e09a3416 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, 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. @@ -154,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 @@ -197,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). @@ -205,16 +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 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. - -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. +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. -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; @@ -222,14 +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. - -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. +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 @@ -247,9 +244,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 +279,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). diff --git a/lib/iris/fileformats/_nc_load_rules/actions.py b/lib/iris/fileformats/_nc_load_rules/actions.py index a6e75744f0..1b1b686187 100644 --- a/lib/iris/fileformats/_nc_load_rules/actions.py +++ b/lib/iris/fileformats/_nc_load_rules/actions.py @@ -4,38 +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. +.. 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 31b4747a75..06d148f16d 100644 --- a/lib/iris/fileformats/cf/__init__.py +++ b/lib/iris/fileformats/cf/__init__.py @@ -12,24 +12,28 @@ 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/ +.. 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. + """ from ._group import CFGroup diff --git a/lib/iris/fileformats/cf/_dask_locks.py b/lib/iris/fileformats/cf/_dask_locks.py index df04c95698..e74a434b0f 100644 --- a/lib/iris/fileformats/cf/_dask_locks.py +++ b/lib/iris/fileformats/cf/_dask_locks.py @@ -9,31 +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. +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 f38586a3a8..96af8eb2b3 100644 --- a/lib/iris/fileformats/cf/_group.py +++ b/lib/iris/fileformats/cf/_group.py @@ -12,23 +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. +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 2b9caceb43..848c720036 100644 --- a/lib/iris/fileformats/cf/_reader.py +++ b/lib/iris/fileformats/cf/_reader.py @@ -4,49 +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. +: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 91f229bd21..5ebaf4013a 100644 --- a/lib/iris/fileformats/cf/_variables.py +++ b/lib/iris/fileformats/cf/_variables.py @@ -10,37 +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)``. +.. 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 2982708a2e..85b4e73141 100644 --- a/lib/iris/fileformats/cf/dataset.py +++ b/lib/iris/fileformats/cf/dataset.py @@ -14,23 +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``. +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 7133f10503..7145f3b24e 100644 --- a/lib/iris/fileformats/cf/loader.py +++ b/lib/iris/fileformats/cf/loader.py @@ -9,25 +9,29 @@ 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 `_. +.. 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. + """ from collections.abc import Iterable, Iterator, Mapping diff --git a/lib/iris/fileformats/cf/saver.py b/lib/iris/fileformats/cf/saver.py index eaf88255e6..54bad548ff 100644 --- a/lib/iris/fileformats/cf/saver.py +++ b/lib/iris/fileformats/cf/saver.py @@ -11,45 +11,51 @@ 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`, for example, is where a user of +the netCDF format meets all of this. See : `CF Conventions `_. +.. 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. + """ import collections diff --git a/lib/iris/fileformats/netcdf/_dataset.py b/lib/iris/fileformats/netcdf/_dataset.py index cf0cddb255..2a93bde6bd 100644 --- a/lib/iris/fileformats/netcdf/_dataset.py +++ b/lib/iris/fileformats/netcdf/_dataset.py @@ -11,14 +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. - -See section 4.5 of ``docs/superpowers/specs/2026-09-21-zarr-io-design.md``. +.. maintainer-notes:: + + 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. + + 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 2eb780d7ec..bdf147b5c0 100644 --- a/lib/iris/fileformats/netcdf/loader.py +++ b/lib/iris/fileformats/netcdf/loader.py @@ -15,15 +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. - See : `NetCDF User's Guide `_ and `netCDF4 python module `_. Also : `CF Conventions `_. +.. maintainer-notes:: + + 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. + """ from collections.abc import Iterable diff --git a/lib/iris/fileformats/zarr/__init__.py b/lib/iris/fileformats/zarr/__init__.py index dd5fccd38d..f4dec9707e 100644 --- a/lib/iris/fileformats/zarr/__init__.py +++ b/lib/iris/fileformats/zarr/__init__.py @@ -18,13 +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. + + 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 6397c0212f..70694f55e0 100644 --- a/lib/iris/fileformats/zarr/_dataset.py +++ b/lib/iris/fileformats/zarr/_dataset.py @@ -9,46 +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``. +.. 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 e2064a430a..a6497b9b36 100644 --- a/lib/iris/fileformats/zarr/_decode.py +++ b/lib/iris/fileformats/zarr/_decode.py @@ -8,44 +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``. + +.. 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 e6178e96dc..71a1b7270c 100644 --- a/lib/iris/fileformats/zarr/loader.py +++ b/lib/iris/fileformats/zarr/loader.py @@ -9,27 +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. + + 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..d119eabaad 100644 --- a/lib/iris/fileformats/zarr/saver.py +++ b/lib/iris/fileformats/zarr/saver.py @@ -17,15 +17,19 @@ **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. +.. 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``. + """ import dask diff --git a/lib/iris/tests/AGENTS.md b/lib/iris/tests/AGENTS.md index f776960912..9730c96717 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 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 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/`. 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