From fd59309fcdcf71d31915e7dd2ef09a9f5ad233b9 Mon Sep 17 00:00:00 2001 From: Bill Little Date: Thu, 10 Sep 2026 15:20:47 +0000 Subject: [PATCH 1/8] Add design spec for merge/concatenate improvement programme Specifies a twelve-PR programme to make cube merge and concatenate more robust, efficient, configurable, extensible, lazy and lenient, without a rewrite. Each PR is either a behaviour-preserving refactor or a standalone user-visible improvement; together they grow a shared substrate beneath both engines. Includes a confirmed diagnosis and fix for the separability bug open since 2017 (SciTools/iris#2761), verified end-to-end against the reporter's case. Co-Authored-By: Claude Opus 5 --- .../2026-09-10-merge-concatenate-design.md | 458 ++++++++++++++++++ 1 file changed, 458 insertions(+) create mode 100644 docs/superpowers/specs/2026-09-10-merge-concatenate-design.md diff --git a/docs/superpowers/specs/2026-09-10-merge-concatenate-design.md b/docs/superpowers/specs/2026-09-10-merge-concatenate-design.md new file mode 100644 index 0000000000..90d670afa9 --- /dev/null +++ b/docs/superpowers/specs/2026-09-10-merge-concatenate-design.md @@ -0,0 +1,458 @@ +# Making Iris merge and concatenate robust, efficient and configurable + +**Status:** design agreed, not yet implemented +**Date:** 2026-09-10 +**Baseline:** `main` at `c508ac800`, Iris `3.17.0.dev4` + +Line references in this document are accurate as of the baseline commit. + +--- + +## 1. Summary + +`iris.cube.CubeList.merge` and `CubeList.concatenate` are the core of Iris's +input workflow and the source of a long tail of user complaints. They are two +independent engines that share no code, carry an open correctness bug from 2017, +realise lazy arrays repeatedly, and expose almost no configuration. + +This document specifies a programme of twelve small, independently reviewable +pull requests. Each is either a behaviour-preserving refactor or a standalone +user-visible improvement. Together they build a shared substrate beneath both +engines, without requiring anyone to agree to a rewrite up front. + +The six goals, in the requester's words, are that merging datasets into a hyper +cube should be **more robust, efficient, configurable at the API level, +extensible, lazy and lenient**. + +--- + +## 2. Why the current design is at its limit + +### 2.1 Two engines that share nothing + +`lib/iris/_merge.py` (1863 lines) and `lib/iris/_concatenate.py` (1702 lines) +each define their own private `_CubeSignature`, `_CoordSignature`, `_ProtoCube`, +`_CoordAndDims` and `_CoordMetaData`. They share no comparison routine, no +signature type and no error type. + +`lib/iris/_combine.py` (added 2025) sequences them but unifies nothing — its +`_combine_cubes` loop calls `.merge()` and `.concatenate()` in an order given by +the `merge_concat_sequence` option. + +A third comparison implementation, `iris.common.resolve.Resolve` (2607 lines), +was built to be the single source of truth for cube comparison and combination +and is used only by cube maths. Iris therefore carries three independent answers +to "are these two cubes compatible, and how do I combine them?". + +### 2.2 Merge is not lazy + +Merge compares live `Coord` objects and cell measures / ancillary variables with +`==`: + +- `_merge.py:192` — `_CoordPayload._coords_msgs`, reached from + `match_signature` at lines 262 and 267 with tuples of actual `Coord` objects. +- `_merge.py:450-453` — `_CubeSignature.match`, on `cell_measures_and_dims` and + `ancillary_variables_and_dims`. + +`_DimensionalMetadata.__eq__` (`coords.py:659-690`) ends in +`iris.util.array_equal(..., withnans=True)`, so every one of these comparisons +realises lazy arrays. They run once per candidate-cube × ProtoCube pair, with no +caching. + +Concatenate solved exactly this problem in PR #5926 using `xxhash` array hashing +and a single batched `dask.compute` (`_compute_hashes`, `_concatenate.py:479`). +Merge never received it. Reported impact on ECMWF hybrid-pressure loading is +roughly 5×, and more than 10× worse again under `_LAZY_DERIVED_LOADING`, with +over 90% of time in `cubes.combine → merge → coord equality → dask compute` +(SciTools/iris#7063, #7241). + +### 2.3 Merge is barely configurable + +`concatenate()` has four escape hatches — `check_aux_coords`, +`check_cell_measures`, `check_ancils`, `check_derived_coords`. `merge()` has one +keyword, `unique`. This asymmetry is directly user-visible in +SciTools/iris#6790, where cubes refuse to merge but concatenate happily after +`new_axis`. + +### 2.4 Merge can be silently wrong + +SciTools/iris#2761, open since 2017. `build_indexes` (`_merge.py:585-604`) +records, for each scalar value, the *set* of values it co-occurs with. Set +membership discards the structure needed to decide separability, so unrelated +candidate dimensions are judged separable. See §5.6–5.7 for the confirmed +diagnosis and fix. + +### 2.5 Leniency has been requested for a decade + +SciTools/iris#1987 (2016) → #4446 (merge, 2021) → #5392 (concatenate, 2023). +Never delivered, in part because retrofitting it onto two unrelated engines means +doing the work twice in two idioms. + +--- + +## 3. Constraints + +1. **No large pull requests.** Iris core developers are sceptical of agentic + contributions, and the AUX-Coord minutes of 2026-01-07 record "Easy to raise + a PR, but hard to get reviewed". Review capacity is the scarce resource being + optimised for. Every item below must stand alone. +2. **Backwards compatible with v3.x.** No breaking changes. The one deliberate + behaviour change (§5.7) is a bug fix and is treated as such. +3. **Honest labelling.** Agentic work is labelled as agentic; precedent exists in + SciTools/iris PR #7161. + +--- + +## 4. Decisions taken + +### 4.1 Where configurability is exposed + +Each capability is implemented once in the shared substrate, exposed as explicit +keywords on `merge()` / `concatenate()`, and reaches load time through +`merge_kwargs` / `concatenate_kwargs` forwarding dicts on `CombineOptions`. + +This mirrors the existing `equalise_cubes_kwargs` rather than inventing a new +mechanism. `CombineOptions.OPTION_KEYS` is already a list carrying the comment +"so we can update it in an inheriting class" (`_combine.py:182`), and dict-valued +options are already special-cased at `_combine.py:232`. + +Rejected: `CombineOptions`-only, which leaves #6790 unfixed for direct callers +and buries configuration in ambient thread-local state; and method-keywords-only, +which leaves load-time users unable to reach the new behaviour. + +### 4.2 How far leniency goes + +Leniency covers **metadata** (via the existing `metadata.equal(lenient=...)` +semantics) and **structure** (tolerating missing or extra aux coords, cell +measures and ancillary variables). + +Coercion of dtypes, units and calendars stays out of the engine, in +`iris.util.equalise_cubes`, which is the existing extension point for it. + +### 4.3 Substrate built bottom-up, not designed top-down + +The substrate is grown one PR at a time starting from the hashing layer, rather +than specified in advance. The alternative — designing a unified v4.0 engine +first — was considered and parked at . + +Adopting `Resolve` wholesale was also rejected as a *starting* point: it is +pairwise and merge is N-ary, so whether it scales is unresolved research risk, +and nothing user-visible would ship until it was answered. + +--- + +## 5. Design + +### 5.1 The substrate module + +A single private module, `lib/iris/_combine_common.py`, sibling to `_merge.py`, +`_concatenate.py`, `_combine.py` and `_lazy_data.py`. Promoted to a package only +if it exceeds roughly 800 lines. + +Rejected placements: + +- **`iris/common/`** — that subpackage is documented and public-facing; putting + engine internals there implies an API commitment. +- **Nesting under a `_combine/` package** — this inverts the dependency. + `_merge.py` would import `iris._combine.hashing`, executing + `iris/_combine/__init__.py`, which reaches `iris.cube`. The substrate sits + *below* merge and concatenate; `_combine.py` sits *above* them. + +The substrate must import nothing from Iris. The hashing code's only Iris +dependency is `iris.coords` for type hints in `array_id`, which goes behind +`TYPE_CHECKING`. This is what makes the merge-side import risk-free. + +### 5.2 What is shared, and what is not + +The two cube-level signatures are **not** unified. They encode different +questions: merge's asks "same kind of cube, identical shape?", because merge adds +a dimension and all sources have the same shape; concatenate's asks "joinable +along an axis?" and carries extents, dim order and dim mapping to answer it. + +The genuinely shared kernel sits one level down: + +1. **Element signature** — a hashed, comparable representation of a single + dimensional-metadata object (coord, cell measure, ancillary variable): + its `metadata`, its dims, `has_bounds()`, and hashes of points/bounds/data. +2. **Array-collection walk** — the traversal of a cube yielding arrays to hash. + Concatenate's `add_coords` (`_concatenate.py:601-611`) and merge's equivalent + are the same function written twice. +3. **Comparison result** — a value carrying *why* two elements differ, which + feeds diagnostics. + +### 5.3 Merge needs a driver function + +Concatenate's loop lives in `iris._concatenate.concatenate()`, which is why it +can hash everything up front. Merge's loop lives in `CubeList.merge()` at +`cube.py:443-467`; there is no `iris._merge.merge()`. + +A new `iris._merge.merge(cubes, unique=True)` takes that loop, and +`CubeList.merge()` delegates — mirroring `CubeList.concatenate()`. This is also +the landing site for every keyword added later, which is why it comes early. + +`merge_cube()` does not share this loop: it builds a single ProtoCube from +`self[0]` with `error_on_mismatch=True` and no name grouping +(`cube.py:364-373`). It is left alone initially and revisited when its error path +becomes relevant. + +### 5.4 Hash-based comparison in merge + +Arrays are collected up front in `_merge.merge()`, mirroring concatenate, and +`hashes` is threaded through `ProtoCube.register()` to the two consumption sites +in §2.2. `self.scalar.defns` (`_merge.py:256`) is left alone — it is already +metadata-only and cheap. + +**Exactly one `compute_hashes()` call per invocation.** This is not stylistic: +`ArrayHash.__eq__` raises `ValueError` on same-shape/different-chunks, and only a +single batched call guarantees chunk unification across every input cube. + +**This is semantics-preserving.** `_DimensionalMetadata.__eq__` +(`coords.py:659-690`) is metadata equality, then `has_bounds()` parity, then +`array_equal(..., withnans=True)` on values and bounds. And +`lib/iris/tests/unit/concatenate/test_hashing.py:88` already asserts that hash +equality is equivalent to `array_equal(a, b, withnans=True)` across a shared +corpus. The metadata and `has_bounds()` checks stay exactly where they are; only +the two `array_equal` calls become hash lookups. + +**The `id()` hazard.** `array_id` is `f"{id(coord)}{bound}"`, and merge calls +`_extract_coord_payload` once per *(cube, ProtoCube)* pair, so ids must be stable +across calls. They are: `Cube.dim_coords` and `Cube.aux_coords` +(`cube.py:3158,3182`) return a fresh tuple of the *same* objects. +`Cube.derived_coords` (`cube.py:3195`) does not — it regenerates from factories +on every access. Merge never array-compares derived coords, going through +`factory_defns` (metadata only, `_merge.py:271`), so it is unaffected. The driver +must nonetheless hold a reference to every input cube for the call's duration, +and this needs a code comment. + +**Risks to state in the PR:** metadata comparison must remain ordered first, +because hashes unify numerical dtypes (`float32([1])` and `bool([1])` hash equal +— consistent with `array_equal`, but only if ordering is unchanged); and the +chunk-mismatch `ValueError` becomes reachable from merge for the first time. + +### 5.5 Measurement + +`benchmarks/benchmarks/merge_concat.py::Merge` merges two cubes and explicitly +strips cell measures and ancillary variables. It cannot demonstrate this +improvement. The benchmark is extended in a separate PR *before* the change, so +the baseline is committed to `main` and the speedup is reproducible. + +### 5.6 Separability: the diagnosis + +Confirmed by direct probe against the reporter's case in SciTools/iris#2761: + +```text +A = [1, 2, 1, 2, 1, 2] +B = [3, 4, 4, 4, 4, 3] +C = [5, 5, 6, 6, 7, 7] +``` + +Current Iris derives: + +```text +a: separable=['b', 'c'] inseparable=[] +b: separable=['a'] inseparable=['c'] +c: separable=['a'] inseparable=['b'] +``` + +and `CubeList.merge()` raises +`ValueError: You must specify the meta or dtype of the array` — a raw Dask error +with no connection to the actual problem. That is the user experience today. + +The current predicate is `_separable_pair` (`_merge.py:607-631`): X and Y are +separable iff every value of X co-occurs with the same *set* of Y values. + +### 5.7 Separability: the fix + +**Replacing the set with a multiset does not work.** The multiset +co-occurrences are also equal in this case (`a=1 → {3:1, 4:2}`, +`a=2 → {3:1, 4:2}`). The correct criterion is about the pair grid, not per-value +co-occurrence: + +> X and Y are separable if and only if the observed (X, Y) pairs form the +> complete Cartesian product `values(X) × values(Y)` with **uniform +> multiplicity**. + +Uniform, not one: in a full three-way product each (X, Y) pair recurs once per +value of the third dimension, and requiring multiplicity 1 would wrongly reject +it. + +Verified against five topologies: + +| topology | expected | result | +|---|---|---| +| 2×2 product | separable | ✓ | +| #2761 case | only `a`–`c` separable | ✓ | +| 2×2×2 full product | all pairs separable (multiplicity 2, uniform) | ✓ | +| `b = f(a)` | inseparable | ✓ | +| incomplete 2×2 (3 cells) | inseparable | ✓ | + +Verified end-to-end by simulating the predicate through a real +`CubeList.merge()`: six cubes become one, `(a: 2; c: 3; y: 2; x: 2)` with `b` as +a 2-D auxiliary coordinate spanning `a` and `c` — precisely what the reporter +said the answer should be. The rest of the algebra (`derive_groups`, +`_derive_separable_group`, `_derive_consistent_groups`, `_is_dependent`) already +handles it correctly once the predicate is right. + +**Blast radius is one call site**: `_merge.py:1215-1220` in `ProtoCube.merge()`, +where `positions` is already in scope — it is passed to `derive_space` on the +next line. Only `derive_relation_matrix` changes shape. `build_indexes` stays, +because `_define_space` still consumes it (`_merge.py:1446`). + +**This algebra has essentially no unit tests.** Grepping `lib/iris/tests` finds +exactly one reference: `integration/merge/test_merge.py:329` +`test_separable_combination`. That is why a 2017 bug survived to 2026. +Characterisation tests therefore land *before* the fix, so the fix's diff shows +exactly which expectations change. + +**Known edge case to pin with a test:** duplicated source cubes make a pair grid +non-uniform, so a case that was "separable, then duplicate-detected" becomes +"inseparable, then duplicate-detected". Probably the same user-visible outcome, +but it must be verified rather than assumed. + +### 5.8 Diagnostics + +`merge()` explains nothing today — it returns N cubes with no indication why they +did not become one. The `msgs` lists built in `_CubeSignature.match` and +`_CoordPayload.match_signature` are constructed and discarded whenever +`error_on_mismatch=False`. + +The work is therefore mostly plumbing: capture and surface what is already +computed, using the comparison-result type from §5.2. `merge()`'s return type is +unchanged; a separate diagnostic entry point is added alongside +`iris.util.describe_diff` (`util.py:205`), which already does pairwise cube +diffs. What is missing is the group-level account: these three formed one cube, +this fourth stayed out because X. + +### 5.9 Leniency + +The primitive already exists: every metadata class has +`equal(other, lenient=None)` (`metadata.py:660`), decorated `@lenient_service`. +Merge hand-compares metadata field by field in `_CubeSignature._defn_msgs` +(`_merge.py:350-406`); concatenate has its own equivalent. Both become +`equal(lenient=...)`. + +`lenient=True` is passed **explicitly**. The `LENIENT` thread-local +(`iris/common/lenient.py:667`) is not used, so no ambient state enters the +engine's hot path — consistent with §4.1. + +On SciTools/iris#5394 and #5395 the claim must be precise: this adopts the +lenient metadata *semantics* that `Resolve` is built on, not `Resolve` itself. +They are partially addressed, not closed. + +Structural leniency — reconciling elements present in some cubes but not others, +by intersection or union — is separated from the `check_*` flags, which only say +"do not compare". It lands last among the behaviour changes because it is the one +item with genuine semantic debate in it. + +--- + +## 6. The programme + +| # | PR | Type | Closes | +|---|---|---|---| +| 1 | Hashing machinery → `_combine_common.py` | internal | — | +| 2 | `iris._merge.merge()` driver function | internal | — | +| 3 | Extend merge benchmark (baseline) | internal | — | +| 4 | **Hash-based comparison in merge** | performance | #7063, #7241 | +| 5 | Shared element signature + collection walk | internal | — | +| 6 | Separability characterisation tests + doctest cleanup | internal | — | +| 7 | **Separability fix** | bugfix | #2761, likely #5768 | +| 8 | Structured diagnostics | feature | #5375 | +| 9 | Metadata leniency | feature | #4446, #5392; part of #5394/#5395 | +| 10 | `check_*` parity for merge | feature | #6790 | +| 11 | Structural leniency | feature | — | +| 12 | `CombineOptions` forwarding dicts | feature | — | + +**PRs 1–4 are the proving tranche.** They end with a measured speedup against a +committed baseline and no behaviour change whatsoever. If core developers do not +engage with those, the remainder should not be attempted. + +Notes on individual items: + +- **PR 1** moves seven names (`_concatenate.py:305-541`, ~237 lines) and the test + file `tests/unit/concatenate/test_hashing.py`. Names lose their underscore + prefix on the way out, following `_lazy_data.py`; this makes it a move *and* a + rename, which is a reasonable thing for a reviewer to object to and a cheap + thing to concede. +- **PR 1** must state plainly that it is the first step of a sequence and link + the performance issues. Its justification is PR 4. +- **PR 7** will change CML fixtures under `tests/results/cube_merge/`. Cases + previously called separable become inseparable; most raised or produced wrong + cubes, but some may have been accidentally right. Every changed fixture needs + an individual justification. This is the review-cost centre of the programme, + which is why it follows the cheap wins. +- **PR 10** requires a documentation change in the same PR: the note at + `cube.py:427-433` states that aux coords, cell measures and ancillaries "must + be identical in every input cube". `check_*=False` is exactly the relaxation of + that sentence. +- **PR 12** must add each new option to all four `SETTINGS` dicts + (`_combine.py:190-219`), defaulting to `None` in all four so load-time + behaviour is unchanged. + +--- + +## 7. Testing and validation + +Per `lib/iris/tests/AGENTS.md`: pytest style, plain assertions, +`pytest.raises(..., match=...)`, `pytest.warns`, explicit laziness assertions, no +network. + +- **Refactor PRs (1, 2, 5)** must change no test expectations. Test files move + and imports update; assertions do not. +- **PR 4** needs explicit tests for masked arrays, NaN, and dtype edge cases at + the hash boundary, plus a test that the chunk-mismatch `ValueError` does not + fire in normal use. The existing equivalence test + (`tests/unit/concatenate/test_hashing.py:88`) is the anchor. +- **PR 6** introduces the topology table of §5.7 as characterisation tests + against *current* behaviour, and fixes the dead Python-2 doctests in + `build_indexes` and `derive_relation_matrix` (`_merge.py:691` uses + `matrix.iteritems()`, which cannot ever have run). +- **PR 7** flips those expectations, with each change justified. +- **Benchmarks**: `benchmarks/benchmarks/merge_concat.py`, extended in PR 3, with + before/after numbers quoted in PR 4. +- Every PR carries a `changelog/..rst` fragment per + `changelog/AGENTS.md`. + +--- + +## 8. Risks + +| Risk | Mitigation | +|---|---| +| Core devs reject the shared module in PR 1 | PR 1 is a pure move with no behaviour change; if the module name or the rename is contested, concede both — the location matters, not the name | +| PR 7 changes more CML fixtures than expected | Characterisation tests land first (PR 6) so the delta is visible before the fix; each fixture justified individually | +| Hash equality diverges from `Coord.__eq__` in an untested corner | Metadata and `has_bounds()` checks unchanged; only `array_equal` calls replaced; existing equivalence test is the anchor | +| Programme stalls after the proving tranche | PRs 1–4 are independently valuable; nothing later depends on the programme continuing | +| Structural leniency semantics prove contentious | It is last, and nothing else depends on it | + +--- + +## 9. Out of scope + +- Unifying `_CubeSignature` across the two engines (§5.2). +- Adopting `Resolve` as the N-ary comparison engine (§4.3). +- Reconciling `iris.common.metadata.hexdigest` with the array-hashing layer. + `_concatenate.py` uses both — `hexdigest` at line 807, but only in the + diagnostic path that names which coords differ, not in the equality path. + Worth doing eventually; bundling it would turn a mechanical move into a + semantic argument. +- Coercion of dtypes, units and calendars (belongs in + `iris.util.equalise_cubes`). +- A v4.0 unified engine — parked at + . + +--- + +## 10. References + +- SciTools/iris#2761 — Merge Problems (separability) +- SciTools/iris#3234 — Unify merge and concatenate +- SciTools/iris#4446, #5392 — `LENIENT` merge / concatenate +- SciTools/iris#5375 — Various cube merge/concatenate issues +- SciTools/iris#5394, #5395 — trial `resolve` in merge / concatenate +- SciTools/iris#5768 — dimensions mashed together on merge +- SciTools/iris#5926 — array hashing in concatenate +- SciTools/iris#6383 — additional `equalise_cubes` functionality +- SciTools/iris#6790 — merge/concatenate asymmetry +- SciTools/iris#7063, #7241 — merge performance +- SciTools/iris discussion #6881 — AUX-Coord minutes, 2026-01-07 From c37d82f9ef803eb435fd46cc0b09ad11d62ad872 Mon Sep 17 00:00:00 2001 From: Bill Little Date: Thu, 10 Sep 2026 15:21:16 +0000 Subject: [PATCH 2/8] Record design-spec location in AGENTS.md Co-Authored-By: Claude Opus 5 --- AGENTS.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 5ad21e3d45..a772d1021e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -168,6 +168,10 @@ sphinx. See [`changelog/AGENTS.md`](changelog/AGENTS.md) for full rules. Documentation lives under `docs/` and is built with Sphinx. See [`docs/AGENTS.md`](docs/AGENTS.md) for full rules. +Agreed design specs live in `docs/superpowers/specs/` as +`YYYY-MM-DD--design.md`. These are working documents outside the Sphinx +source tree, so RST conventions do not apply to them. + ## Lock-file Maintenance From f9219cf3d2d5241f39d41e44c7e3ecd2dda56420 Mon Sep 17 00:00:00 2001 From: Bill Little Date: Thu, 10 Sep 2026 15:56:46 +0000 Subject: [PATCH 3/8] docs: publish design specs as MyST Markdown Adopt the spec/plan convention used in bjlittle/tephpy. Agreed design specs now live in `docs/src/developers_guide/specs/` and are published as part of the documentation. They are written in MyST Markdown, are living documents revised as a design evolves, and each declares a citation prefix so its sections can be cited from issues and pull requests. Implementation plans live in the sibling `docs/src/developers_guide/plans/`. A plan is point-in-time and frozen once its pull request merges, so plans are tracked in the repository but excluded from the Sphinx build. This requires `myst-parser`, added to `requirements/py*.yml` and inserted into the three lock files following the precedent of #7095 and #7100. Markdown is used only for specs; all other documentation remains reStructuredText. The merge/concatenate design spec moves here from `docs/superpowers/specs/` and is reworked to the new conventions. Co-Authored-By: Claude Opus 5 --- AGENTS.md | 196 +++++++----------- docs/src/conf.py | 13 ++ .../contributing_getting_involved.rst | 1 + docs/src/developers_guide/plans/README.md | 10 + .../2026-09-10-merge-concatenate-design.md | 157 +++++++++----- docs/src/developers_guide/specs/index.rst | 43 ++++ requirements/locks/py312-linux-64.lock | 4 +- requirements/locks/py313-linux-64.lock | 4 +- requirements/locks/py314-linux-64.lock | 6 +- requirements/py312.yml | 1 + requirements/py313.yml | 1 + requirements/py314.yml | 1 + 12 files changed, 256 insertions(+), 181 deletions(-) create mode 100644 docs/src/developers_guide/plans/README.md rename docs/{superpowers => src/developers_guide}/specs/2026-09-10-merge-concatenate-design.md (82%) create mode 100644 docs/src/developers_guide/specs/index.rst diff --git a/AGENTS.md b/AGENTS.md index a772d1021e..965f917ecf 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -7,10 +7,10 @@ Iris is a Python package for analysing and visualising Earth science data, built around CF-compliant multi-dimensional arrays ("Cubes"). Subdirectory AGENTS.md files take precedence for their subtrees: -- [`changelog/AGENTS.md`](changelog/AGENTS.md) — documentation on changelog +- [`changelog/AGENTS.md`](changelog/AGENTS.md) — changelog fragments - [`docs/AGENTS.md`](docs/AGENTS.md) — documentation-specific rules -- [`lib/iris/tests/AGENTS.md`](lib/iris/tests/AGENTS.md) — test-specific rules - +- [`lib/iris/tests/AGENTS.md`](lib/iris/tests/AGENTS.md) — testing rules; read + this before writing or running any test ## Project Overview @@ -24,87 +24,51 @@ Subdirectory AGENTS.md files take precedence for their subtrees: | **Test runner** | pytest + pytest-xdist (`-n auto`) | | **Env management** | nox + conda | - ### Main source layout ``` -lib/iris/ - cube.py # Core Cube / CubeList data structures - coords.py # DimCoord, AuxCoord, CellMeasure, AncillaryVariable - loading.py # File-loading entry points - analysis/ # Collapse, regrid, statistics, calculus - fileformats/ # NetCDF, PP, GRIB, NIMROD format handlers - io/ # I/O registry and URI handling - common/ # Shared metadata, mixins, resolvers - mesh/ # Unstructured grid (UGRID) support - experimental/ # Unstable / in-progress features - tests/ # All tests (unit/, integration/, graphics/) -changelog/ # changelog fragments -docs/src/ # Sphinx documentation source -benchmarks/ # ASV performance benchmarks -requirements/ # Conda environment specs and lock files +lib/iris/ cube.py, coords.py, loading.py; analysis/ (collapse, regrid, + statistics), fileformats/ (NetCDF, PP, GRIB, NIMROD), io/, + common/ (metadata, mixins, resolvers), mesh/ (UGRID), + experimental/, tests/ (unit/, integration/, graphics/) +changelog/ changelog fragments +docs/src/ Sphinx documentation source +benchmarks/ ASV performance benchmarks +requirements/ conda environment specs and lock files ``` - ## Setup -### Conda environment (recommended) - -Always use a conda environment, reuse the iris-dev conda environment if it -already exists but confirm with the user before installing or removing packages. - -If a package cannot be installed via conda then you can use pip that is in the -conda environment. - -```bash -# Create and activate a development environment -conda env create -f requirements/iris.yml -conda activate iris-dev -pip install --no-build-isolation -e . -``` - -Alternatively, use lock files for exact reproducibility: +Always work in a conda environment. Reuse `iris-dev` if it exists, but confirm +with the user before installing or removing packages; if a package is not on +conda, use the `pip` inside the conda environment. ```bash +# Preferred - exact reproducibility from a lock file. conda create -n iris-dev --file requirements/locks/py314-linux-64.lock +# Alternative - solve fresh: conda env create -f requirements/iris.yml conda activate iris-dev pip install --no-build-isolation -e . ``` - ### Environment variables ```bash # Disable CPU features that can cause SIGILL in some CI environments export NPY_DISABLE_CPU_FEATURES="AVX512F,AVX512CD,AVX512_SKX" - # Point to iris-test-data for tests that need external data files export OVERRIDE_TEST_DATA_REPOSITORY=/path/to/iris-test-data/test_data - # Override Cartopy cache directory if needed export CARTOPY_CACHE_DIR=~/.local/share/cartopy ``` - -## Testing - -- [`lib/iris/tests/AGENTS.md`](lib/iris/tests/AGENTS.md) — test-specific - - ## Code Style ```bash -# Lint -ruff check lib/iris - -# Auto-fix safe lint issues -ruff check --fix lib/iris - -# Format -ruff format lib/iris - -# Check formatting without writing -ruff format --check lib/iris +ruff check lib/iris # lint +ruff check --fix lib/iris # auto-fix safe lint issues +ruff format lib/iris # format +ruff format --check lib/iris # check formatting without writing ``` - **Line length**: 88 characters (Ruff default). @@ -121,7 +85,6 @@ ruff format --check lib/iris - **Imports**: Ruff-managed ordering. No direct `import netCDF4` — always use `iris.fileformats.netcdf._thread_safe_nc` for thread safety. - ## Development Conventions ### Core data model @@ -130,112 +93,99 @@ ruff format --check lib/iris - Coordinates: `DimCoord` (regular), `AuxCoord` (auxiliary), `CellMeasure`, `AncillaryVariable`. - Data may be **lazy** (Dask array). Always preserve laziness; never call - `.data` - unnecessarily inside library code. + `.data` unnecessarily inside library code. - Operations return **new** Cubes (functional style); do not mutate in place. - All metadata must be **CF-convention** compliant. - ### Deprecation - Use `iris._deprecation.warn_deprecated()` or issue a custom warning class. - Warning classes live in `iris.warnings` (e.g., `IrisUserWarning`, - `IrisCfWarning`). + `IrisCfWarning`). All `UserWarning` subclasses must ultimately inherit from + `IrisUserWarning`. - Follow the NEP29 deprecation schedule (same as NumPy). -- All `UserWarning` subclasses must ultimately inherit from `IrisUserWarning`. - - -### Exception hierarchy - -Base class: `iris.exceptions.IrisError`. Common subclasses: -`CoordinateNotFoundError`, `CoordinateCollapseError`, `IgnoreCubeException`. - - -### Versioning -Version is derived from git tags via `setuptools_scm`. Do not hard-code version -strings. +### Exceptions and versioning +- Base class: `iris.exceptions.IrisError`. Common subclasses: + `CoordinateNotFoundError`, `CoordinateCollapseError`, `IgnoreCubeException`. +- Version is derived from git tags via `setuptools_scm`. Do not hard-code + version strings. ## Changelog -Changelog fragments lives under `chngelog/` and is built with towncrier via +Changelog fragments live under `changelog/` and are built with towncrier via sphinx. See [`changelog/AGENTS.md`](changelog/AGENTS.md) for full rules. - ## Documentation Documentation lives under `docs/` and is built with Sphinx. See [`docs/AGENTS.md`](docs/AGENTS.md) for full rules. -Agreed design specs live in `docs/superpowers/specs/` as -`YYYY-MM-DD--design.md`. These are working documents outside the Sphinx -source tree, so RST conventions do not apply to them. +Agreed design specs live in `docs/src/developers_guide/specs/` as +`YYYY-MM-DD--design.md`. They are **published** MyST Markdown and are +**living documents**, revised as a design evolves. Each declares a citation +prefix (e.g. `merge spec §5.7`) so its sections can be cited from issues and +pull requests, and carries explicit `(prefix-N-N)=` anchors. Markdown is used +*only* for specs; all other documentation is RST. +Implementation plans live in the sibling `docs/src/developers_guide/plans/` as +`YYYY-MM-DD-.md`. A plan is point-in-time and frozen once its pull +request merges, so plans are tracked in the repository but excluded from the +Sphinx build via `exclude_patterns` in `docs/src/conf.py`. ## Lock-file Maintenance +Adding a dependency means editing `requirements/py*.yml` **and** the matching +`requirements/locks/*.lock`. Repository precedent (PRs #7095, #7100) is a +*minimal* lock edit — insert only the new package URLs and update `input_hash` +— rather than a full re-solve, which bundles dozens of unrelated version bumps +into an otherwise focused PR. Full re-solves are left to the weekly +`refresh-lockfiles` workflow, or run locally with: + ```bash -# Regenerate lock files for all supported Python versions -python tools/update_lockfiles.py -o requirements/locks requirements/py*.yml -# Shortcut via Makefile -make lockfiles +make lockfiles # python tools/update_lockfiles.py -o requirements/locks requirements/py*.yml ``` - ## Pull Request Guidelines - When creating a pull request a template is provided to ensure all checks are considered. -- This project is configured to use pre-commit tht will ensure some checks are +- This project is configured to use pre-commit that will ensure some checks are performed automatically. - Keep changes focused; avoid unrelated refactors in the same PR. - Add or update tests for every change to production code. -- Ensure a whatsnew fragment is added, see -[`changelog/AGENTS.md`](changelog/AGENTS.md) - +- Ensure a changelog fragment is added, see + [`changelog/AGENTS.md`](changelog/AGENTS.md). ## Critical Development Gotchas -1. **xfail_strict Behavior**: Tests marked `@pytest.mark.xfail` that now PASS - become FAILURES -> **remove xfail immediately when bug is fixed** - -2. **Pre-commit Auto-fixes**: Hooks may auto-fix ISC001/COM812 conflicts -> - re-stage files: `git add . && git commit` - -3. **Lockfile Rebuilds**: Updating `requirements/locks/*.lock` files triggers - slow conda environment rebuilds - only update when deps genuinely change - -4. **Lazy Data Pitfalls**: Check `cube.has_lazy_data()` before operations; use - `cube.lazy_data()` when appropriate; `cube.data` materializes arrays - -5. **Coordinate Scope**: Not all coordinates span all dimensions -> use - `cube.coords(dimensions=dim_indices)` to query specific axes - -6. **Dask/NumPy Compatibility**: Some operations fail on lazy (Dask) arrays - -> materialize with `cube.compute()` or check dask support before use - -7. **Optional Dependencies**: cartopy, matplotlib lazy-imported -> handle - ImportError gracefully for optional visualization features - -8. **CF-Metadata Validation**: Custom coordinates must comply with CF - standards via `cube.metadata` - -9. **Cube Immutability**: Operations return new cubes; don't modify in-place - (`cube.data[...] = x` won't affect the cube) - -10. **Environment Reproducibility**: Set CPU features for consistent results: - `export NPY_DISABLE_CPU_FEATURES="AVX512F,AVX512CD,AVX512_SKX"` - +1. **xfail_strict**: `@pytest.mark.xfail` tests that now PASS become FAILURES — + remove the xfail immediately when the bug is fixed. +2. **Pre-commit auto-fixes**: hooks may auto-fix ISC001/COM812 conflicts — + re-stage with `git add . && git commit`. +3. **Lock-file rebuilds**: lock changes trigger slow conda environment rebuilds + — only update when deps genuinely change. +4. **Lazy data**: check `cube.has_lazy_data()`; use `cube.lazy_data()` where + appropriate; `cube.data` materialises arrays. +5. **Coordinate scope**: not all coords span all dims — use + `cube.coords(dimensions=dim_indices)` to query specific axes. +6. **Dask/NumPy compatibility**: some operations fail on lazy arrays — + materialise with `cube.compute()` or check dask support first. +7. **Optional dependencies**: cartopy and matplotlib are lazy-imported — handle + `ImportError` gracefully. +8. **CF-metadata validation**: custom coordinates must comply with CF standards + via `cube.metadata`. +9. **Cube immutability**: operations return new cubes; `cube.data[...] = x` + will not affect the cube. +10. **Environment reproducibility**: set `NPY_DISABLE_CPU_FEATURES` as above for + consistent results. ## Getting Help -- **Discussions**: - [GitHub Discussions](https://github.com/SciTools/iris/discussions) -- **Contributing Guide**: [.github/CONTRIBUTING.md](.github/CONTRIBUTING.md) -- **Developer Guide**: [docs/src/developers_guide/](docs/src/developers_guide/) -- **Code of Conduct**: [CODE_OF_CONDUCT.md](CODE_OF_CONDUCT.md) - +[GitHub Discussions](https://github.com/SciTools/iris/discussions) · +[Contributing Guide](.github/CONTRIBUTING.md) · +[Developer Guide](docs/src/developers_guide/) · [Code of Conduct](CODE_OF_CONDUCT.md) ## ⚠️ Meta-Instruction: Auto-Update Rule - **Trigger**: You MUST automatically update this `AGENTS.md` file if a new diff --git a/docs/src/conf.py b/docs/src/conf.py index 0e491f3ddc..71268a787c 100644 --- a/docs/src/conf.py +++ b/docs/src/conf.py @@ -154,6 +154,7 @@ def _dotv(version): extensions = [ "matplotlib.sphinxext.mathmpl", "matplotlib.sphinxext.plot_directive", + "myst_parser", "readingtime", "readingtime_validator", "sphinx.ext.autodoc", @@ -222,6 +223,15 @@ def _dotv(version): ) ) +# -- myst-parser extension ---------------------------------------------------- +# See https://myst-parser.readthedocs.io/en/latest/configuration.html +# Markdown is reserved for developer design specs, see +# developers_guide/specs/index. All other documentation remains +# reStructuredText. +myst_enable_extensions = [ + "colon_fence", +] + # -- Napoleon extension ------------------------------------------------------- # See https://sphinxcontrib-napoleon.readthedocs.io/en/latest/sphinxcontrib.napoleon.html napoleon_google_docstring = True @@ -458,6 +468,9 @@ def _dotv(version): # list of sources to exclude from the build. exclude_patterns = [ "**/highlights.rst", + # Implementation plans are tracked in the repository but not published; + # see developers_guide/specs/index. + "developers_guide/plans/**", ] # -- sphinx-gallery config ---------------------------------------------------- diff --git a/docs/src/developers_guide/contributing_getting_involved.rst b/docs/src/developers_guide/contributing_getting_involved.rst index 94383cd3bf..41a2ce674c 100644 --- a/docs/src/developers_guide/contributing_getting_involved.rst +++ b/docs/src/developers_guide/contributing_getting_involved.rst @@ -52,6 +52,7 @@ If you are new to using GitHub we recommend reading the contributing_changes github_app release + specs/index .. toctree:: diff --git a/docs/src/developers_guide/plans/README.md b/docs/src/developers_guide/plans/README.md new file mode 100644 index 0000000000..bcc33d550e --- /dev/null +++ b/docs/src/developers_guide/plans/README.md @@ -0,0 +1,10 @@ +# Implementation plans + +Implementation plans for the design specs in [`../specs/`](../specs). + +Unlike a spec, a plan is **not** a living document. It records the sequence of +steps agreed at the time it was written, and is frozen once the work it +describes has merged. Plans are tracked in the repository for provenance, but +are excluded from the Sphinx build via `exclude_patterns` in `docs/src/conf.py`. + +Naming: `YYYY-MM-DD-.md`. diff --git a/docs/superpowers/specs/2026-09-10-merge-concatenate-design.md b/docs/src/developers_guide/specs/2026-09-10-merge-concatenate-design.md similarity index 82% rename from docs/superpowers/specs/2026-09-10-merge-concatenate-design.md rename to docs/src/developers_guide/specs/2026-09-10-merge-concatenate-design.md index 90d670afa9..277a7bd7fd 100644 --- a/docs/superpowers/specs/2026-09-10-merge-concatenate-design.md +++ b/docs/src/developers_guide/specs/2026-09-10-merge-concatenate-design.md @@ -1,12 +1,23 @@ # Making Iris merge and concatenate robust, efficient and configurable -**Status:** design agreed, not yet implemented -**Date:** 2026-09-10 -**Baseline:** `main` at `c508ac800`, Iris `3.17.0.dev4` +```{readingtime} +``` + +> **Living document.** This spec is updated as the design evolves; it is not a +> point-in-time record of what was agreed on any particular day. Cite sections +> of it as `merge spec §N.N`. A bare `§N.N` inside this document refers to this +> document. -Line references in this document are accurate as of the baseline commit. +- **Date:** 2026-09-10 +- **Status:** design agreed, not yet implemented +- **Issues:** {issue}`2761`, {issue}`5375`, {issue}`6790`, {issue}`7063`, + {issue}`7241` +- **Applies to:** `lib/iris/_merge.py`, `lib/iris/_concatenate.py`, + `lib/iris/_combine.py` +- **Baseline:** `main` at `c508ac800`, Iris `3.17.0.dev4`. Line references in + this document are accurate as of that commit. ---- +(merge-spec-1)= ## 1. Summary @@ -24,10 +35,12 @@ The six goals, in the requester's words, are that merging datasets into a hyper cube should be **more robust, efficient, configurable at the API level, extensible, lazy and lenient**. ---- +(merge-spec-2)= ## 2. Why the current design is at its limit +(merge-spec-2-1)= + ### 2.1 Two engines that share nothing `lib/iris/_merge.py` (1863 lines) and `lib/iris/_concatenate.py` (1702 lines) @@ -44,6 +57,8 @@ was built to be the single source of truth for cube comparison and combination and is used only by cube maths. Iris therefore carries three independent answers to "are these two cubes compatible, and how do I combine them?". +(merge-spec-2-2)= + ### 2.2 Merge is not lazy Merge compares live `Coord` objects and cell measures / ancillary variables with @@ -59,36 +74,42 @@ Merge compares live `Coord` objects and cell measures / ancillary variables with realises lazy arrays. They run once per candidate-cube × ProtoCube pair, with no caching. -Concatenate solved exactly this problem in PR #5926 using `xxhash` array hashing -and a single batched `dask.compute` (`_compute_hashes`, `_concatenate.py:479`). -Merge never received it. Reported impact on ECMWF hybrid-pressure loading is -roughly 5×, and more than 10× worse again under `_LAZY_DERIVED_LOADING`, with -over 90% of time in `cubes.combine → merge → coord equality → dask compute` -(SciTools/iris#7063, #7241). +Concatenate solved exactly this problem in {pull}`5926` using `xxhash` array +hashing and a single batched `dask.compute` (`_compute_hashes`, +`_concatenate.py:479`). Merge never received it. Reported impact on ECMWF +hybrid-pressure loading is roughly 5×, and more than 10× worse again under +`_LAZY_DERIVED_LOADING`, with over 90% of time in +`cubes.combine → merge → coord equality → dask compute` ({issue}`7063`, +{issue}`7241`). + +(merge-spec-2-3)= ### 2.3 Merge is barely configurable `concatenate()` has four escape hatches — `check_aux_coords`, `check_cell_measures`, `check_ancils`, `check_derived_coords`. `merge()` has one -keyword, `unique`. This asymmetry is directly user-visible in -SciTools/iris#6790, where cubes refuse to merge but concatenate happily after -`new_axis`. +keyword, `unique`. This asymmetry is directly user-visible in {issue}`6790`, +where cubes refuse to merge but concatenate happily after `new_axis`. + +(merge-spec-2-4)= ### 2.4 Merge can be silently wrong -SciTools/iris#2761, open since 2017. `build_indexes` (`_merge.py:585-604`) -records, for each scalar value, the *set* of values it co-occurs with. Set -membership discards the structure needed to decide separability, so unrelated -candidate dimensions are judged separable. See §5.6–5.7 for the confirmed -diagnosis and fix. +{issue}`2761`, open since 2017. `build_indexes` (`_merge.py:585-604`) records, +for each scalar value, the *set* of values it co-occurs with. Set membership +discards the structure needed to decide separability, so unrelated candidate +dimensions are judged separable. See §5.6–5.7 for the confirmed diagnosis and +fix. + +(merge-spec-2-5)= ### 2.5 Leniency has been requested for a decade -SciTools/iris#1987 (2016) → #4446 (merge, 2021) → #5392 (concatenate, 2023). -Never delivered, in part because retrofitting it onto two unrelated engines means -doing the work twice in two idioms. +{issue}`1987` (2016) → {issue}`4446` (merge, 2021) → {issue}`5392` +(concatenate, 2023). Never delivered, in part because retrofitting it onto two +unrelated engines means doing the work twice in two idioms. ---- +(merge-spec-3)= ## 3. Constraints @@ -99,12 +120,14 @@ doing the work twice in two idioms. 2. **Backwards compatible with v3.x.** No breaking changes. The one deliberate behaviour change (§5.7) is a bug fix and is treated as such. 3. **Honest labelling.** Agentic work is labelled as agentic; precedent exists in - SciTools/iris PR #7161. + {pull}`7161`. ---- +(merge-spec-4)= ## 4. Decisions taken +(merge-spec-4-1)= + ### 4.1 Where configurability is exposed Each capability is implemented once in the shared substrate, exposed as explicit @@ -116,9 +139,12 @@ mechanism. `CombineOptions.OPTION_KEYS` is already a list carrying the comment "so we can update it in an inheriting class" (`_combine.py:182`), and dict-valued options are already special-cased at `_combine.py:232`. -Rejected: `CombineOptions`-only, which leaves #6790 unfixed for direct callers -and buries configuration in ambient thread-local state; and method-keywords-only, -which leaves load-time users unable to reach the new behaviour. +Rejected: `CombineOptions`-only, which leaves {issue}`6790` unfixed for direct +callers and buries configuration in ambient thread-local state; and +method-keywords-only, which leaves load-time users unable to reach the new +behaviour. + +(merge-spec-4-2)= ### 4.2 How far leniency goes @@ -129,20 +155,25 @@ measures and ancillary variables). Coercion of dtypes, units and calendars stays out of the engine, in `iris.util.equalise_cubes`, which is the existing extension point for it. +(merge-spec-4-3)= + ### 4.3 Substrate built bottom-up, not designed top-down The substrate is grown one PR at a time starting from the hashing layer, rather than specified in advance. The alternative — designing a unified v4.0 engine -first — was considered and parked at . +first — was considered and parked at +. Adopting `Resolve` wholesale was also rejected as a *starting* point: it is pairwise and merge is N-ary, so whether it scales is unresolved research risk, and nothing user-visible would ship until it was answered. ---- +(merge-spec-5)= ## 5. Design +(merge-spec-5-1)= + ### 5.1 The substrate module A single private module, `lib/iris/_combine_common.py`, sibling to `_merge.py`, @@ -162,6 +193,8 @@ The substrate must import nothing from Iris. The hashing code's only Iris dependency is `iris.coords` for type hints in `array_id`, which goes behind `TYPE_CHECKING`. This is what makes the merge-side import risk-free. +(merge-spec-5-2)= + ### 5.2 What is shared, and what is not The two cube-level signatures are **not** unified. They encode different @@ -180,6 +213,8 @@ The genuinely shared kernel sits one level down: 3. **Comparison result** — a value carrying *why* two elements differ, which feeds diagnostics. +(merge-spec-5-3)= + ### 5.3 Merge needs a driver function Concatenate's loop lives in `iris._concatenate.concatenate()`, which is why it @@ -195,6 +230,8 @@ the landing site for every keyword added later, which is why it comes early. (`cube.py:364-373`). It is left alone initially and revisited when its error path becomes relevant. +(merge-spec-5-4)= + ### 5.4 Hash-based comparison in merge Arrays are collected up front in `_merge.merge()`, mirroring concatenate, and @@ -229,6 +266,8 @@ because hashes unify numerical dtypes (`float32([1])` and `bool([1])` hash equal — consistent with `array_equal`, but only if ordering is unchanged); and the chunk-mismatch `ValueError` becomes reachable from merge for the first time. +(merge-spec-5-5)= + ### 5.5 Measurement `benchmarks/benchmarks/merge_concat.py::Merge` merges two cubes and explicitly @@ -236,9 +275,11 @@ strips cell measures and ancillary variables. It cannot demonstrate this improvement. The benchmark is extended in a separate PR *before* the change, so the baseline is committed to `main` and the speedup is reproducible. +(merge-spec-5-6)= + ### 5.6 Separability: the diagnosis -Confirmed by direct probe against the reporter's case in SciTools/iris#2761: +Confirmed by direct probe against the reporter's case in {issue}`2761`: ```text A = [1, 2, 1, 2, 1, 2] @@ -261,6 +302,8 @@ with no connection to the actual problem. That is the user experience today. The current predicate is `_separable_pair` (`_merge.py:607-631`): X and Y are separable iff every value of X co-occurs with the same *set* of Y values. +(merge-spec-5-7)= + ### 5.7 Separability: the fix **Replacing the set with a multiset does not work.** The multiset @@ -281,7 +324,7 @@ Verified against five topologies: | topology | expected | result | |---|---|---| | 2×2 product | separable | ✓ | -| #2761 case | only `a`–`c` separable | ✓ | +| {issue}`2761` case | only `a`–`c` separable | ✓ | | 2×2×2 full product | all pairs separable (multiplicity 2, uniform) | ✓ | | `b = f(a)` | inseparable | ✓ | | incomplete 2×2 (3 cells) | inseparable | ✓ | @@ -309,6 +352,8 @@ non-uniform, so a case that was "separable, then duplicate-detected" becomes "inseparable, then duplicate-detected". Probably the same user-visible outcome, but it must be verified rather than assumed. +(merge-spec-5-8)= + ### 5.8 Diagnostics `merge()` explains nothing today — it returns N cubes with no indication why they @@ -323,6 +368,8 @@ unchanged; a separate diagnostic entry point is added alongside diffs. What is missing is the group-level account: these three formed one cube, this fourth stayed out because X. +(merge-spec-5-9)= + ### 5.9 Leniency The primitive already exists: every metadata class has @@ -335,7 +382,7 @@ Merge hand-compares metadata field by field in `_CubeSignature._defn_msgs` (`iris/common/lenient.py:667`) is not used, so no ambient state enters the engine's hot path — consistent with §4.1. -On SciTools/iris#5394 and #5395 the claim must be precise: this adopts the +On {issue}`5394` and {issue}`5395` the claim must be precise: this adopts the lenient metadata *semantics* that `Resolve` is built on, not `Resolve` itself. They are partially addressed, not closed. @@ -344,7 +391,7 @@ by intersection or union — is separated from the `check_*` flags, which only s "do not compare". It lands last among the behaviour changes because it is the one item with genuine semantic debate in it. ---- +(merge-spec-6)= ## 6. The programme @@ -353,13 +400,13 @@ item with genuine semantic debate in it. | 1 | Hashing machinery → `_combine_common.py` | internal | — | | 2 | `iris._merge.merge()` driver function | internal | — | | 3 | Extend merge benchmark (baseline) | internal | — | -| 4 | **Hash-based comparison in merge** | performance | #7063, #7241 | +| 4 | **Hash-based comparison in merge** | performance | {issue}`7063`, {issue}`7241` | | 5 | Shared element signature + collection walk | internal | — | | 6 | Separability characterisation tests + doctest cleanup | internal | — | -| 7 | **Separability fix** | bugfix | #2761, likely #5768 | -| 8 | Structured diagnostics | feature | #5375 | -| 9 | Metadata leniency | feature | #4446, #5392; part of #5394/#5395 | -| 10 | `check_*` parity for merge | feature | #6790 | +| 7 | **Separability fix** | bugfix | {issue}`2761`, likely {issue}`5768` | +| 8 | Structured diagnostics | feature | {issue}`5375` | +| 9 | Metadata leniency | feature | {issue}`4446`, {issue}`5392`; part of {issue}`5394` / {issue}`5395` | +| 10 | `check_*` parity for merge | feature | {issue}`6790` | | 11 | Structural leniency | feature | — | | 12 | `CombineOptions` forwarding dicts | feature | — | @@ -389,7 +436,7 @@ Notes on individual items: (`_combine.py:190-219`), defaulting to `None` in all four so load-time behaviour is unchanged. ---- +(merge-spec-7)= ## 7. Testing and validation @@ -413,7 +460,7 @@ network. - Every PR carries a `changelog/..rst` fragment per `changelog/AGENTS.md`. ---- +(merge-spec-8)= ## 8. Risks @@ -425,7 +472,7 @@ network. | Programme stalls after the proving tranche | PRs 1–4 are independently valuable; nothing later depends on the programme continuing | | Structural leniency semantics prove contentious | It is last, and nothing else depends on it | ---- +(merge-spec-9)= ## 9. Out of scope @@ -441,18 +488,18 @@ network. - A v4.0 unified engine — parked at . ---- +(merge-spec-10)= ## 10. References -- SciTools/iris#2761 — Merge Problems (separability) -- SciTools/iris#3234 — Unify merge and concatenate -- SciTools/iris#4446, #5392 — `LENIENT` merge / concatenate -- SciTools/iris#5375 — Various cube merge/concatenate issues -- SciTools/iris#5394, #5395 — trial `resolve` in merge / concatenate -- SciTools/iris#5768 — dimensions mashed together on merge -- SciTools/iris#5926 — array hashing in concatenate -- SciTools/iris#6383 — additional `equalise_cubes` functionality -- SciTools/iris#6790 — merge/concatenate asymmetry -- SciTools/iris#7063, #7241 — merge performance -- SciTools/iris discussion #6881 — AUX-Coord minutes, 2026-01-07 +- {issue}`2761` — Merge Problems (separability) +- {issue}`3234` — Unify merge and concatenate +- {issue}`4446`, {issue}`5392` — `LENIENT` merge / concatenate +- {issue}`5375` — Various cube merge/concatenate issues +- {issue}`5394`, {issue}`5395` — trial `resolve` in merge / concatenate +- {issue}`5768` — dimensions mashed together on merge +- {pull}`5926` — array hashing in concatenate +- {issue}`6383` — additional `equalise_cubes` functionality +- {issue}`6790` — merge/concatenate asymmetry +- {issue}`7063`, {issue}`7241` — merge performance +- {discussion}`6881` — AUX-Coord minutes, 2026-01-07 diff --git a/docs/src/developers_guide/specs/index.rst b/docs/src/developers_guide/specs/index.rst new file mode 100644 index 0000000000..86d4d8286e --- /dev/null +++ b/docs/src/developers_guide/specs/index.rst @@ -0,0 +1,43 @@ +.. include:: ../../common_links.inc + +.. _developers_guide_specs: + +Design Specs +============ + +A **design spec** records what a substantial piece of Iris work is building and +why, and is agreed before any of the implementation lands. Specs are written +in `MyST`_ Markdown rather than reStructuredText; they are the only part of the +documentation that is. + +Specs are **living documents**. They are revised as the design evolves, so a +spec always describes the current intent rather than a snapshot of a past +discussion. + +Each spec declares a **citation prefix** so that its sections can be referred to +unambiguously from issues, pull requests and other specs. Within a spec, a bare +``§N.N`` always means that same document. + +.. list-table:: + :header-rows: 1 + :widths: 25 75 + + * - Citation + - Spec + * - ``merge spec §…`` + - :doc:`2026-09-10-merge-concatenate-design` + +.. note:: + + Implementation plans live alongside these specs, in + ``docs/src/developers_guide/plans/``. Unlike a spec, a plan is a + point-in-time record that is frozen once the work it describes has merged, + so plans are tracked in the repository but are **not** published here. + +.. _MyST: https://myst-parser.readthedocs.io/ + +.. toctree:: + :maxdepth: 1 + :hidden: + + 2026-09-10-merge-concatenate-design diff --git a/requirements/locks/py312-linux-64.lock b/requirements/locks/py312-linux-64.lock index 541a4e0716..9680f4af73 100644 --- a/requirements/locks/py312-linux-64.lock +++ b/requirements/locks/py312-linux-64.lock @@ -1,6 +1,6 @@ # Generated by conda-lock. # platform: linux-64 -# input_hash: ec70083cb9ed06616b8d4368678123e62003bbdc710c1a3214f19caf1ef913ce +# input_hash: dd58c28bc578adba3973a7540a455cdceb68d17e8ac1e4f8704e1dbc4912929c @EXPLICIT https://conda.anaconda.org/conda-forge/linux-64/eigen-abi-5.0.1.80-hf414acd_0.conda#a85c1768af7780d67c6446c17848b76f https://conda.anaconda.org/conda-forge/noarch/font-ttf-dejavu-sans-mono-2.37-hab24e00_0.tar.bz2#0c96522c6bdaed4b1566d11387caaf45 @@ -297,6 +297,7 @@ https://conda.anaconda.org/conda-forge/linux-64/libegl-devel-1.7.0-ha4b6fd6_5.co https://conda.anaconda.org/conda-forge/linux-64/libgdal-core-3.13.3-h5fdb907_0.conda#c0723084e1315a5247d5a033d9eb5522 https://conda.anaconda.org/conda-forge/linux-64/libharfbuzz-devel-14.4.0-h23af247_0.conda#26e37e05324d7bb723350d50b33057fc https://conda.anaconda.org/conda-forge/linux-64/libraqm-0.11.0-h6406941_0.conda#3ac89a48d224409739dbf6200e524373 +https://conda.anaconda.org/conda-forge/noarch/mdit-py-plugins-0.6.1-pyhd8ed1ab_0.conda#ad6821df7a98510117db06e9a833281f https://conda.anaconda.org/conda-forge/linux-64/mo_pack-0.3.1-py312h4f23490_2.conda#cec5bc5f7d374f8f8095f8e28e31f6cb https://conda.anaconda.org/conda-forge/linux-64/pandas-3.0.5-py312h8ecdadd_1.conda#85eb29ade84d1bd97be5ec55607efb00 https://conda.anaconda.org/conda-forge/linux-64/pango-1.58.2-hda50119_0.conda#6a2822aaf9a34ac3708904a47ff3dd7e @@ -353,6 +354,7 @@ https://conda.anaconda.org/conda-forge/noarch/pyvista-0.48.4-pyhd8ed1ab_1.conda# https://conda.anaconda.org/conda-forge/noarch/esmpy-8.9.1-pyhdfbf58e_0.conda#0155d4a55415cda7f7f09bc0b7760f51 https://conda.anaconda.org/conda-forge/noarch/geovista-0.5.3-pyhd8ed1ab_1.conda#64348d05eedb1b1b5676f63101d004f2 https://conda.anaconda.org/conda-forge/linux-64/graphviz-14.1.2-h8b86629_0.conda#341fc61cfe8efa5c72d24db56c776f44 +https://conda.anaconda.org/conda-forge/noarch/myst-parser-5.1.0-pyhd8ed1ab_0.conda#580615e59fc7c07741e4d2ab052cfc8b https://conda.anaconda.org/conda-forge/noarch/pydata-sphinx-theme-0.19.0-pyhcf101f3_0.conda#87921f66a4dc56ce92e4ff13be5f63dc https://conda.anaconda.org/conda-forge/noarch/sphinx-copybutton-0.5.2-pyhd8ed1ab_1.conda#bf22cb9c439572760316ce0748af3713 https://conda.anaconda.org/conda-forge/noarch/sphinx-data-viewer-0.1.5-pyhd8ed1ab_1.conda#fbe6437c6899c54e52f9258518035aca diff --git a/requirements/locks/py313-linux-64.lock b/requirements/locks/py313-linux-64.lock index 464c78499a..ba4a363155 100644 --- a/requirements/locks/py313-linux-64.lock +++ b/requirements/locks/py313-linux-64.lock @@ -1,6 +1,6 @@ # Generated by conda-lock. # platform: linux-64 -# input_hash: 744ae3daa77a848be733ec9f02b1ea2eaa61374e7ccd2b0443f3814db0f06f9a +# input_hash: eee198cca4941f1c0b249463a8292b901c06a2608e5f787123cf985f818b3940 @EXPLICIT https://conda.anaconda.org/conda-forge/linux-64/eigen-abi-5.0.1.80-hf414acd_0.conda#a85c1768af7780d67c6446c17848b76f https://conda.anaconda.org/conda-forge/noarch/font-ttf-dejavu-sans-mono-2.37-hab24e00_0.tar.bz2#0c96522c6bdaed4b1566d11387caaf45 @@ -298,6 +298,7 @@ https://conda.anaconda.org/conda-forge/linux-64/libegl-devel-1.7.0-ha4b6fd6_5.co https://conda.anaconda.org/conda-forge/linux-64/libgdal-core-3.13.3-h5fdb907_0.conda#c0723084e1315a5247d5a033d9eb5522 https://conda.anaconda.org/conda-forge/linux-64/libharfbuzz-devel-14.4.0-h23af247_0.conda#26e37e05324d7bb723350d50b33057fc https://conda.anaconda.org/conda-forge/linux-64/libraqm-0.11.0-h6406941_0.conda#3ac89a48d224409739dbf6200e524373 +https://conda.anaconda.org/conda-forge/noarch/mdit-py-plugins-0.6.1-pyhd8ed1ab_0.conda#ad6821df7a98510117db06e9a833281f https://conda.anaconda.org/conda-forge/linux-64/mo_pack-0.3.1-py313h29aa505_2.conda#ad53894d278895bf15c8fc324727d224 https://conda.anaconda.org/conda-forge/linux-64/pandas-3.0.5-py313hbfd7664_1.conda#a78866632a871f274cd2015bccdd2ca0 https://conda.anaconda.org/conda-forge/linux-64/pango-1.58.2-hda50119_0.conda#6a2822aaf9a34ac3708904a47ff3dd7e @@ -352,6 +353,7 @@ https://conda.anaconda.org/conda-forge/noarch/pyvista-0.48.4-pyhd8ed1ab_1.conda# https://conda.anaconda.org/conda-forge/noarch/esmpy-8.9.1-pyhdfbf58e_0.conda#0155d4a55415cda7f7f09bc0b7760f51 https://conda.anaconda.org/conda-forge/noarch/geovista-0.5.3-pyhd8ed1ab_1.conda#64348d05eedb1b1b5676f63101d004f2 https://conda.anaconda.org/conda-forge/linux-64/graphviz-14.1.2-h8b86629_0.conda#341fc61cfe8efa5c72d24db56c776f44 +https://conda.anaconda.org/conda-forge/noarch/myst-parser-5.1.0-pyhd8ed1ab_0.conda#580615e59fc7c07741e4d2ab052cfc8b https://conda.anaconda.org/conda-forge/noarch/pydata-sphinx-theme-0.19.0-pyhcf101f3_0.conda#87921f66a4dc56ce92e4ff13be5f63dc https://conda.anaconda.org/conda-forge/noarch/sphinx-copybutton-0.5.2-pyhd8ed1ab_1.conda#bf22cb9c439572760316ce0748af3713 https://conda.anaconda.org/conda-forge/noarch/sphinx-data-viewer-0.1.5-pyhd8ed1ab_1.conda#fbe6437c6899c54e52f9258518035aca diff --git a/requirements/locks/py314-linux-64.lock b/requirements/locks/py314-linux-64.lock index bf0fca9583..de80c91abd 100644 --- a/requirements/locks/py314-linux-64.lock +++ b/requirements/locks/py314-linux-64.lock @@ -1,6 +1,6 @@ # Generated by conda-lock. # platform: linux-64 -# input_hash: 44e0f500cd52311fec1d8114cd6dcb464ded7a96eedaaf6a9daaf7fbb548c949 +# input_hash: 397a587135f480eca53832a6158013d747d96ee14eec2d30e16fb85606052a4a @EXPLICIT https://conda.anaconda.org/conda-forge/noarch/font-ttf-dejavu-sans-mono-2.37-hab24e00_0.tar.bz2#0c96522c6bdaed4b1566d11387caaf45 https://conda.anaconda.org/conda-forge/noarch/font-ttf-inconsolata-3.000-h77eed37_0.tar.bz2#34893075a5c9e55cdafac56607368fc6 @@ -174,6 +174,7 @@ https://conda.anaconda.org/conda-forge/linux-64/libxkbcommon-1.13.2-h51789e4_1.c https://conda.anaconda.org/conda-forge/linux-64/libxml2-devel-2.15.3-h49c6c72_1.conda#f986d9c9de80813d076d235f753889ff https://conda.anaconda.org/conda-forge/noarch/locket-1.0.0-pyhd8ed1ab_0.tar.bz2#91e27ef3d05cc772ce627e51cff111c4 https://conda.anaconda.org/conda-forge/linux-64/markupsafe-3.0.3-py314h67df5f8_1.conda#9a17c4307d23318476d7fbf0fedc0cde +https://conda.anaconda.org/conda-forge/noarch/mdurl-0.1.2-pyhd8ed1ab_1.conda#592132998493b3ff25fd7479396e8351 https://conda.anaconda.org/conda-forge/linux-64/msgpack-python-1.2.2-py314h5383ef5_0.conda#66e5d65a13ac23cd29ca87eb858c7d27 https://conda.anaconda.org/conda-forge/noarch/munkres-1.1.4-pyhd8ed1ab_1.conda#37293a85a0f4f77bbd9cf7aaefc62609 https://conda.anaconda.org/conda-forge/noarch/packaging-26.3-pyhc364b38_0.conda#936687ed80f295a1f5dbcf8bd34c252c @@ -228,6 +229,7 @@ https://conda.anaconda.org/conda-forge/noarch/jinja2-3.1.6-pyhcf101f3_1.conda#04 https://conda.anaconda.org/conda-forge/linux-64/libgl-devel-1.7.0-ha4b6fd6_5.conda#f5f7fc038c611a25b22758c0a40d25c9 https://conda.anaconda.org/conda-forge/linux-64/libharfbuzz-14.4.0-h23af247_0.conda#4463210c2a16ff5d3bf7f502af23f1f4 https://conda.anaconda.org/conda-forge/linux-64/libspatialite-5.1.0-gpl_hab3fe16_120.conda#5947bdda89ced07ab0d8a5d6503082c3 +https://conda.anaconda.org/conda-forge/noarch/markdown-it-py-4.2.0-pyhd8ed1ab_0.conda#6d03368f2b2b0a5fb6839df53b2eb5e0 https://conda.anaconda.org/conda-forge/noarch/nodeenv-1.10.0-pyhd8ed1ab_0.conda#eb52d14a901e23c39e9e7b4a1a5c015f https://conda.anaconda.org/conda-forge/linux-64/numpy-2.5.2-py314h2b28147_0.conda#d77631addad93399a90157d2c597e7f3 https://conda.anaconda.org/conda-forge/noarch/partd-1.4.2-pyhd8ed1ab_0.conda#0badf9c54e24cecfb0ad2f99d680c163 @@ -252,6 +254,7 @@ https://conda.anaconda.org/conda-forge/linux-64/libegl-devel-1.7.0-ha4b6fd6_5.co https://conda.anaconda.org/conda-forge/linux-64/libgdal-core-3.13.3-h5fdb907_0.conda#c0723084e1315a5247d5a033d9eb5522 https://conda.anaconda.org/conda-forge/linux-64/libharfbuzz-devel-14.4.0-h23af247_0.conda#26e37e05324d7bb723350d50b33057fc https://conda.anaconda.org/conda-forge/linux-64/libraqm-0.11.0-h6406941_0.conda#3ac89a48d224409739dbf6200e524373 +https://conda.anaconda.org/conda-forge/noarch/mdit-py-plugins-0.6.1-pyhd8ed1ab_0.conda#ad6821df7a98510117db06e9a833281f https://conda.anaconda.org/conda-forge/linux-64/mo_pack-0.3.1-py314hc02f841_2.conda#55ac6d85f5dd8ec5e9919e7762fcb31a https://conda.anaconda.org/conda-forge/linux-64/pandas-3.0.5-py314hb4ffadd_1.conda#15cd0f375b79b47c4048e44396e6e6f1 https://conda.anaconda.org/conda-forge/linux-64/pango-1.58.2-hda50119_0.conda#6a2822aaf9a34ac3708904a47ff3dd7e @@ -294,6 +297,7 @@ https://conda.anaconda.org/conda-forge/noarch/adwaita-icon-theme-49.0-unix_0.con https://conda.anaconda.org/conda-forge/linux-64/esmf-8.9.1-nompi_h39d0119_4.conda#074ca9356f9e916ec51ad6ab47fd55ba https://conda.anaconda.org/conda-forge/noarch/esmpy-8.9.1-pyhdfbf58e_0.conda#0155d4a55415cda7f7f09bc0b7760f51 https://conda.anaconda.org/conda-forge/linux-64/graphviz-14.1.2-h8b86629_0.conda#341fc61cfe8efa5c72d24db56c776f44 +https://conda.anaconda.org/conda-forge/noarch/myst-parser-5.1.0-pyhd8ed1ab_0.conda#580615e59fc7c07741e4d2ab052cfc8b https://conda.anaconda.org/conda-forge/noarch/pydata-sphinx-theme-0.19.0-pyhcf101f3_0.conda#87921f66a4dc56ce92e4ff13be5f63dc https://conda.anaconda.org/conda-forge/noarch/sphinx-copybutton-0.5.2-pyhd8ed1ab_1.conda#bf22cb9c439572760316ce0748af3713 https://conda.anaconda.org/conda-forge/noarch/sphinx-data-viewer-0.1.5-pyhd8ed1ab_1.conda#fbe6437c6899c54e52f9258518035aca diff --git a/requirements/py312.yml b/requirements/py312.yml index 2683ed107a..dc249dcbbf 100644 --- a/requirements/py312.yml +++ b/requirements/py312.yml @@ -52,6 +52,7 @@ dependencies: - requests # Documentation dependencies. + - myst-parser - sphinx <8.2 - sphinxcontrib-apidoc - sphinx-copybutton diff --git a/requirements/py313.yml b/requirements/py313.yml index 1bfbeb3ba7..f5fd30452d 100644 --- a/requirements/py313.yml +++ b/requirements/py313.yml @@ -52,6 +52,7 @@ dependencies: - requests # Documentation dependencies. + - myst-parser - sphinx <8.2 - sphinxcontrib-apidoc - sphinx-copybutton diff --git a/requirements/py314.yml b/requirements/py314.yml index 3f8e3d81ee..f0fba8146e 100644 --- a/requirements/py314.yml +++ b/requirements/py314.yml @@ -52,6 +52,7 @@ dependencies: - requests # Documentation dependencies. + - myst-parser - sphinx <8.2 - sphinxcontrib-apidoc - sphinx-copybutton From 65d05b8ccd367edf79c18472727c3973f3622d00 Mon Sep 17 00:00:00 2001 From: Bill Little Date: Thu, 10 Sep 2026 16:46:32 +0000 Subject: [PATCH 4/8] docs: add a plan roadmap and decision register to the merge spec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The spec described a twelve-PR programme but had nowhere to record how far it had got, which assumptions it rested on, or what it had left open. Restructure §6 from a static table of work into the programme's live progress record: dependencies as a partial order rather than a chain, a status vocabulary, an "Outside the roadmap" list, and a numbered register of assumptions and open decisions - each item owned by the pull request that must resolve it. Add §10 for the questions the programme raises but does not answer, renumbering References to §11. Also exclude "_build" and "_static" from the Sphinx build. myst-parser makes ".md" a source suffix, so the README of each was being read as a document. Co-Authored-By: Claude Opus 5 --- docs/src/conf.py | 4 + .../2026-09-10-merge-concatenate-design.md | 207 ++++++++++++++++-- docs/src/developers_guide/specs/index.rst | 5 +- 3 files changed, 197 insertions(+), 19 deletions(-) diff --git a/docs/src/conf.py b/docs/src/conf.py index 71268a787c..af2d7189ff 100644 --- a/docs/src/conf.py +++ b/docs/src/conf.py @@ -471,6 +471,10 @@ def _dotv(version): # Implementation plans are tracked in the repository but not published; # see developers_guide/specs/index. "developers_guide/plans/**", + # myst-parser makes ".md" a source suffix, which would otherwise pull the + # README of each of these non-document directories into the build. + "_build/**", + "_static/**", ] # -- sphinx-gallery config ---------------------------------------------------- diff --git a/docs/src/developers_guide/specs/2026-09-10-merge-concatenate-design.md b/docs/src/developers_guide/specs/2026-09-10-merge-concatenate-design.md index 277a7bd7fd..cdf2c95af1 100644 --- a/docs/src/developers_guide/specs/2026-09-10-merge-concatenate-design.md +++ b/docs/src/developers_guide/specs/2026-09-10-merge-concatenate-design.md @@ -9,7 +9,8 @@ > document. - **Date:** 2026-09-10 -- **Status:** design agreed, not yet implemented +- **Status:** design agreed; no pull request opened yet. §6 is the live progress + record, and is updated as each pull request lands. - **Issues:** {issue}`2761`, {issue}`5375`, {issue}`6790`, {issue}`7063`, {issue}`7241` - **Applies to:** `lib/iris/_merge.py`, `lib/iris/_concatenate.py`, @@ -393,27 +394,52 @@ item with genuine semantic debate in it. (merge-spec-6)= -## 6. The programme - -| # | PR | Type | Closes | -|---|---|---|---| -| 1 | Hashing machinery → `_combine_common.py` | internal | — | -| 2 | `iris._merge.merge()` driver function | internal | — | -| 3 | Extend merge benchmark (baseline) | internal | — | -| 4 | **Hash-based comparison in merge** | performance | {issue}`7063`, {issue}`7241` | -| 5 | Shared element signature + collection walk | internal | — | -| 6 | Separability characterisation tests + doctest cleanup | internal | — | -| 7 | **Separability fix** | bugfix | {issue}`2761`, likely {issue}`5768` | -| 8 | Structured diagnostics | feature | {issue}`5375` | -| 9 | Metadata leniency | feature | {issue}`4446`, {issue}`5392`; part of {issue}`5394` / {issue}`5395` | -| 10 | `check_*` parity for merge | feature | {issue}`6790` | -| 11 | Structural leniency | feature | — | -| 12 | `CombineOptions` forwarding dicts | feature | — | +## 6. Plan roadmap + +Twelve pull requests deliver the design. Each gets its own implementation plan in +`docs/src/developers_guide/plans/`, and a PR is merged before any PR that +*depends on it* is written. The dependencies form a partial order, not a chain: +PRs 1, 2, 3 and 6 have no predecessors and may proceed in parallel. The ordering +follows §5's layering — the substrate first (PR 1), then the seam that lets merge +use it (PRs 2–3), then the payoff (PR 4), then the correctness and capability +work that the substrate makes cheap. + +This table is the programme's live progress record and is updated as each PR +lands. Status vocabulary: **not started** · **in progress** · **✅ complete** · +**blocked** · **abandoned**, each carrying its {pull} citation once one exists. + +| # | PR | Scope (spec §) | Type | Closes | Depends on | Status | +|---|---|---|---|---|---|---| +| 1 | Hashing machinery → `_combine_common.py` | §5.1 — move `_concatenate.py:305-541` (~237 lines) and `tests/unit/concatenate/test_hashing.py`; names lose their underscore prefix | internal | — | — | not started | +| 2 | `iris._merge.merge()` driver function | §5.3 — lift the loop out of `CubeList.merge()` (`cube.py:443-467`); `CubeList.merge` delegates, mirroring `CubeList.concatenate` | internal | — | — | not started | +| 3 | Extend merge benchmark | §5.5 — `benchmarks/benchmarks/merge_concat.py::Merge` currently strips cell measures and ancillaries, so it cannot show the PR 4 effect | internal | — | — | not started | +| 4 | **Hash-based comparison in merge** | §5.4 — collect arrays up front, thread `hashes` through `ProtoCube.register()` to the two `==` sites of §2.2 | performance | {issue}`7063`, {issue}`7241` | 1, 2, 3 | not started | +| 5 | Shared element signature + collection walk | §5.2 items 1–3 — one hashed element signature, one array-collection walk, one comparison-result type, for both engines | internal | — | 1, 4 | not started | +| 6 | Separability characterisation tests + doctest cleanup | §5.6, §7 — the five-topology table as tests against *current* behaviour; fix the Python-2 doctests at `_merge.py:691` | internal | — | — | not started | +| 7 | **Separability fix** | §5.7 — replace `_separable_pair`'s set criterion with complete-grid-and-uniform-multiplicity, at the single call site `_merge.py:1215-1220` | bugfix | {issue}`2761`, likely {issue}`5768` | 6 | not started | +| 8 | Structured diagnostics | §5.8 — capture the `msgs` already built and discarded; a group-level account beside `iris.util.describe_diff` | feature | {issue}`5375` | 2, 5 | not started | +| 9 | Metadata leniency | §5.9 — `metadata.equal(lenient=...)` replaces hand-rolled field comparison in both engines | feature | {issue}`4446`, {issue}`5392`; part of {issue}`5394` / {issue}`5395` | 2, 5 | not started | +| 10 | `check_*` parity for merge | §2.3 — the four `check_*` keywords on merge, with the `cube.py:427-433` documentation change in the same PR | feature | {issue}`6790` | 2 | not started | +| 11 | Structural leniency | §5.9 — reconcile elements present in some cubes but not others, by intersection or union | feature | — | 9, 10 | not started | +| 12 | `CombineOptions` forwarding dicts | §4.1 — `merge_kwargs` / `concatenate_kwargs` in all four `SETTINGS` dicts, defaulting to `None` | feature | — | 10, 11 | not started | **PRs 1–4 are the proving tranche.** They end with a measured speedup against a committed baseline and no behaviour change whatsoever. If core developers do not engage with those, the remainder should not be attempted. +Cross-cutting rules (apply to every row rather than one of them): + +- **Each PR cites this spec** and says which row it is, so a reviewer can see the + shape of the programme without reading twelve descriptions, and says plainly + that it is agentic work — {pull}`7161` is the precedent (§3). +- **Changelog fragment ships with its PR**, typed as the Type column says, per + `changelog/AGENTS.md`. +- **Refactor PRs change no test expectations** (rows 1, 2, 5). Test files move and + imports update; assertions do not. Behaviour PRs land their tests with them. +- **Performance claims are quoted, not asserted.** Any row claiming an effect + quotes before/after numbers from `benchmarks/benchmarks/merge_concat.py` + against a baseline already on `main` (§5.5). + Notes on individual items: - **PR 1** moves seven names (`_concatenate.py:305-541`, ~237 lines) and the test @@ -436,6 +462,99 @@ Notes on individual items: (`_combine.py:190-219`), defaulting to `None` in all four so load-time behaviour is unchanged. +Outside the roadmap: + +- A v4.0 unified engine is parked at + (§4.3, §9) and is not a row here. +- The coercion work under {issue}`6383` is adjacent and separately owned: §4.2 + routes dtype, unit and calendar coercion to `iris.util.equalise_cubes` rather + than into either engine. +- Core-developer engagement is the gating condition, not a deliverable. PR 1's + reception decides whether PRs 5–12 are attempted (§3). + +### Assumptions and open decisions + +Enumerated so they are visible decisions rather than silent drift. Items 1–5 are +decisions this design makes; the remainder are questions assigned to the PR that +must answer them, ordered by owning PR. Items carry a tracking issue where one +already exists; the rest name their owning row, and get an issue when that PR is +opened. + +1. **Resolved** (2026-09-10) — **Incremental substrate, not a designed engine.** + The alternative — specify a unified v4.0 engine, then build it — was + considered and rejected as a *starting* point, because nothing user-visible + ships until the whole thing is agreed. It is preserved rather than discarded, + at , and §4.3 carries the + reasoning. Adopting `Resolve` wholesale was rejected on the same ground: it is + pairwise, merge is N-ary, and whether it scales is unresolved research risk. +2. **Resolved** (2026-09-10) — **Configurability is keywords plus forwarding + dicts.** Each capability is implemented once in the substrate, exposed as an + explicit keyword on `merge()` / `concatenate()`, and reaches load time through + `merge_kwargs` / `concatenate_kwargs` on `CombineOptions` (§4.1). The two + rejected alternatives are recorded there: `CombineOptions`-only leaves + {issue}`6790` unfixed for direct callers, and method-keywords-only leaves + load-time users unable to reach the behaviour. +3. **Resolved** (2026-09-10) — **Leniency covers metadata and structure; + coercion stays out.** §4.2. Dtype, unit and calendar coercion belongs in + `iris.util.equalise_cubes`, which is the existing extension point for it. +4. **Resolved** (2026-09-10) — **The substrate is a flat private module.** + `lib/iris/_combine_common.py`, sibling to `_merge.py` and `_concatenate.py`, + promoted to a package only above roughly 800 lines. `iris/common/` was + rejected as public-facing, and a `_combine/` package as inverting the + dependency — the substrate sits *below* the two engines, `_combine.py` sits + above them (§5.1). +5. **Resolved** (2026-09-10) — **The two cube-level signatures are not + unified.** They encode different questions, and forcing one type on both would + be a rewrite wearing a refactor's clothes (§5.2). {issue}`3234` therefore + stays open; this programme narrows it rather than closing it. +6. **Open** (PR 1) — **Whether the moved names drop their underscore prefix.** + Following `_lazy_data.py` makes PR 1 a move *and* a rename, which is a + reasonable thing for a reviewer to object to and a cheap thing to concede. The + location is what matters, not the name (§8). +7. **Open** (PR 1) — **Whether the programme proceeds past the proving tranche.** + §3 names review capacity as the scarce resource. PR 1 is a pure move with no + behaviour change and its justification is PR 4, so its reception is the signal + for everything after it. Nothing in PRs 5–12 is load-bearing for PRs 1–4. +8. **Open** (PR 2) — **Whether `merge_cube()` joins the driver.** §5.3 leaves it + alone: it builds a single ProtoCube from `self[0]` with + `error_on_mismatch=True` and no name grouping (`cube.py:364-373`). Its error + path is exactly where PR 8's diagnostics are most visible, so the question + re-opens there rather than being settled once. +9. **Open** (PR 4) — **What merge does if `ArrayHash.__eq__` raises.** §5.4 + argues that exactly one batched `compute_hashes()` call per invocation makes + the same-shape/different-chunks `ValueError` unreachable. PR 4 must prove that + with a test rather than assert it, and decide the fallback if the argument is + wrong — this exception becomes reachable from merge for the first time. +10. **Open** (PR 5, §9) — **Whether `hexdigest` reconciliation stays out of + scope.** `_concatenate.py` uses both `iris.common.metadata.hexdigest` (line + 807, diagnostic path only) and the array-hashing layer. §9 keeps them apart so + that a mechanical move does not become a semantic argument. If PR 5's shared + element signature makes the duplication glaring, revisit it there. +11. **Open** (PR 6) — **How many CML fixtures PR 7 changes.** Unknown until the + characterisation tests exist. Cases previously called separable become + inseparable; most raised or produced wrong cubes, but some may have been + accidentally right, and every changed fixture needs an individual + justification. §5.7 calls this the programme's review-cost centre; PR 6 + measures it and PR 7 justifies it. +12. **Open** (PR 6, PR 7) — **Duplicated source cubes under the new criterion.** + Duplication makes a pair grid non-uniform, so a case that was "separable, + then duplicate-detected" becomes "inseparable, then duplicate-detected". + Believed to be the same user-visible outcome, but §5.7 requires it be pinned + by a test rather than assumed. +13. **Open** (PR 7, {issue}`5768`) — **Whether the separability fix closes + {issue}`5768`.** The roadmap says "likely", which is not good enough for a PR + description. The claim needs a reproduction against the fixed predicate before + it is made. +14. **Open** (PR 10) — **Whether the new keywords also go on `merge_cube()`.** + Downstream of item 8. If `merge_cube()` never joins the driver, it acquires + the keywords separately or not at all, and {issue}`6790` is only partly + answered. +15. **Open** (PR 11) — **Intersection or union for structural leniency.** + Reconciling elements present in some cubes but not others has two defensible + answers, and the choice is user-visible. §5.9 separates this from the + `check_*` flags, which only say "do not compare". It lands last precisely + because it is the one item with genuine semantic debate in it. + (merge-spec-7)= ## 7. Testing and validation @@ -490,7 +609,59 @@ network. (merge-spec-10)= -## 10. References +## 10. Open questions + +Questions raised by the research behind this spec that the roadmap does not +answer. Unlike §6's assumptions — each of which is owned by a PR that must +resolve it — these are open against the programme as a whole, and some will +outlive it. Every one carries a status and, where it exists, a citation; those +without one say so. + +- **Open** ({issue}`3234`) — **Should the two engines ever become one?** §5.2 + says no *now*: the two cube-level signatures encode different questions, and + the shared substrate captures the real duplication without forcing a common + type. Whether that remains the right answer once PRs 1, 5 and 9 have landed is + worth re-asking against the codebase they leave behind, not against today's. +- **Parked** () — **Does `Resolve` + scale from pairwise to N-ary comparison?** `Resolve` is the modern, tested, + public machinery for reconciling two cubes, and merge is inherently N-ary. + Nobody has established what pairwise-folding an N-ary problem costs, in + performance or in semantics. This is the central unknown behind {issue}`5394` + and {issue}`5395`, and it is research risk, not implementation risk — which is + why §4.3 keeps it off the roadmap rather than pretending to have costed it. +- **Open** ({issue}`5375`) — **Should merge diagnostics be public API?** PR 8 + makes the reasons a group failed to merge *available*; it does not settle + whether they are a documented, stable, user-facing surface or an error-message + improvement. The two have very different maintenance costs, and the answer + determines whether the diagnostics need a deprecation policy. +- **Open** ({issue}`6383`) — **Does `equalise_cubes` absorb enough coercion to + keep engine leniency narrow?** §4.2 draws the line at metadata and structure on + the strength of `equalise_cubes` covering dtype, unit and calendar. If its + sub-issues stall, pressure returns to put coercion in the engines, and the line + has to be re-argued rather than silently redrawn. +- **Open** ({issue}`7063`) — **What is the measured speedup from hash-based + comparison in merge?** {issue}`7063` and {issue}`7241` report the problem; + {pull}`5926` reports roughly 5× for the concatenate case. Merge's profile is + different — many small scalar-coord comparisons rather than a few large array + ones — so the concatenate figure is not transferable. PR 3 exists to make this + answerable before PR 4 claims anything (§5.5). +- **Open** (no tracking issue yet; raised with PR 4) — **Can cube registration + stay lazy end to end?** Hashing still computes, so PR 4 replaces + many realisations with one batched one rather than removing realisation. + Whether merge could instead decide most registrations on metadata alone and + touch array values only for genuine ties is a larger question that PR 4 does + not foreclose but also does not answer. +- **Open** (no tracking issue yet; raised with PR 7) — **Does the separability + fix need a deprecation cycle?** §5.7 treats it as a bugfix: the current + criterion produces silently wrong cubes, and there is no defensible way to + keep producing them. But it changes results for existing data without any + opt-in, which is the kind of change core developers reasonably want staged. + Warn-then-change is available; whether it is warranted is a judgement for the + PR 7 review, and the answer is worth recording here when it comes. + +(merge-spec-11)= + +## 11. References - {issue}`2761` — Merge Problems (separability) - {issue}`3234` — Unify merge and concatenate diff --git a/docs/src/developers_guide/specs/index.rst b/docs/src/developers_guide/specs/index.rst index 86d4d8286e..326358e087 100644 --- a/docs/src/developers_guide/specs/index.rst +++ b/docs/src/developers_guide/specs/index.rst @@ -12,7 +12,10 @@ documentation that is. Specs are **living documents**. They are revised as the design evolves, so a spec always describes the current intent rather than a snapshot of a past -discussion. +discussion. Each carries a **plan roadmap** recording the pull requests that +deliver it and the status of each, together with the assumptions, open decisions +and open questions the work has surfaced. A spec is therefore the place to look +for how far a programme of work has got, not only what it intends. Each spec declares a **citation prefix** so that its sections can be referred to unambiguously from issues, pull requests and other specs. Within a spec, a bare From abcf8b9b35c0161628f21124925dd6d093edd302 Mon Sep 17 00:00:00 2001 From: Bill Little Date: Thu, 10 Sep 2026 21:10:10 +0000 Subject: [PATCH 5/8] docs: record how agentic contributions are pushed and labelled This work is long-running partly because it is also an exercise in earning core-developer buy-in for agentic contributions, so the mechanics of how it reaches them are worth writing down rather than re-deciding each session. Add a "Contribution Workflow" section to AGENTS.md carrying the general rule: branches to the fork, pull requests to a feature branch rather than main, honest "Agentic" labelling, clear attribution, and ask before doing anything outward-facing when unsure. Drop the "Getting Help" link list to stay inside the file's 200-line budget - the links it held are all reachable from CONTRIBUTING.md. The programme-specific parts - base branch "greenfield" and the "Feature: Merge/Concatenate" label - go in the spec's cross-cutting rules instead, where a reviewer can see why these pull requests do not target main. Co-Authored-By: Claude Opus 5 --- AGENTS.md | 32 +++++++++---------- .../2026-09-10-merge-concatenate-design.md | 7 ++++ 2 files changed, 23 insertions(+), 16 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 965f917ecf..9e8ac0b487 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -147,16 +147,22 @@ into an otherwise focused PR. Full re-solves are left to the weekly make lockfiles # python tools/update_lockfiles.py -o requirements/locks requirements/py*.yml ``` -## Pull Request Guidelines - -- When creating a pull request a template is provided to ensure all checks are - considered. -- This project is configured to use pre-commit that will ensure some checks are - performed automatically. -- Keep changes focused; avoid unrelated refactors in the same PR. -- Add or update tests for every change to production code. -- Ensure a changelog fragment is added, see - [`changelog/AGENTS.md`](changelog/AGENTS.md). +## Contribution Workflow + +- Push development branches to `origin` (the `bjlittle/iris` fork). Raise pull + requests from there against the relevant **feature branch** on `upstream` + (`SciTools/iris`) — never against `upstream/main`. The `upstream` remote is + push-disabled, which enforces this. +- **Attribute agentic work clearly.** Say in the body of every pull request and + issue that it is agentic, and end commit messages with the `Co-Authored-By` + trailer. Never let agentic work read as hand-written. +- Label such pull requests and issues `Agentic` and `Type: Feature Branch`, + plus whichever `Feature: …` label fits the subject. +- Use the pull request template; keep changes focused and avoid unrelated + refactors; add or update tests for every production change; ship a changelog + fragment (see [`changelog/AGENTS.md`](changelog/AGENTS.md)). +- **If in doubt about anything outward-facing** — base branch, labels, whether + to post at all — **ask before pushing.** ## Critical Development Gotchas @@ -181,12 +187,6 @@ make lockfiles # python tools/update_lockfiles.py -o requirements/locks require 10. **Environment reproducibility**: set `NPY_DISABLE_CPU_FEATURES` as above for consistent results. -## Getting Help - -[GitHub Discussions](https://github.com/SciTools/iris/discussions) · -[Contributing Guide](.github/CONTRIBUTING.md) · -[Developer Guide](docs/src/developers_guide/) · [Code of Conduct](CODE_OF_CONDUCT.md) - ## ⚠️ Meta-Instruction: Auto-Update Rule - **Trigger**: You MUST automatically update this `AGENTS.md` file if a new structural pattern, critical bug fix, or persistent repository rule is diff --git a/docs/src/developers_guide/specs/2026-09-10-merge-concatenate-design.md b/docs/src/developers_guide/specs/2026-09-10-merge-concatenate-design.md index cdf2c95af1..b6c011a317 100644 --- a/docs/src/developers_guide/specs/2026-09-10-merge-concatenate-design.md +++ b/docs/src/developers_guide/specs/2026-09-10-merge-concatenate-design.md @@ -432,6 +432,13 @@ Cross-cutting rules (apply to every row rather than one of them): - **Each PR cites this spec** and says which row it is, so a reviewer can see the shape of the programme without reading twelve descriptions, and says plainly that it is agentic work — {pull}`7161` is the precedent (§3). +- **Every PR targets the `greenfield` feature branch, not `main`.** The + programme is long-running and deliberately incremental, so its parts land + together rather than trickling into `main` one at a time; `greenfield` is then + merged back once the tranche it carries is coherent. Each PR is labelled + `Feature: Merge/Concatenate`, `Type: Feature Branch` and `Agentic`, so the + agentic ones are filterable — both for reviewers who want them and for those + who would rather not. - **Changelog fragment ships with its PR**, typed as the Type column says, per `changelog/AGENTS.md`. - **Refactor PRs change no test expectations** (rows 1, 2, 5). Test files move and From 62ad6ac7e9a6861db7965327f7f19860033f4e0b Mon Sep 17 00:00:00 2001 From: Bill Little Date: Thu, 10 Sep 2026 21:18:19 +0000 Subject: [PATCH 6/8] docs: add changelog fragments for the design spec Co-Authored-By: Claude Opus 5 --- changelog/7274.dependency.rst | 3 +++ changelog/7274.doc.rst | 6 ++++++ 2 files changed, 9 insertions(+) create mode 100644 changelog/7274.dependency.rst create mode 100644 changelog/7274.doc.rst diff --git a/changelog/7274.dependency.rst b/changelog/7274.dependency.rst new file mode 100644 index 0000000000..812d965ab1 --- /dev/null +++ b/changelog/7274.dependency.rst @@ -0,0 +1,3 @@ +:user:`bjlittle` added `myst-parser `_ as a +documentation dependency, so that developer design specs may be authored in +MyST Markdown. All other documentation remains reStructuredText. diff --git a/changelog/7274.doc.rst b/changelog/7274.doc.rst new file mode 100644 index 0000000000..19000e671b --- /dev/null +++ b/changelog/7274.doc.rst @@ -0,0 +1,6 @@ +:user:`bjlittle` added a design spec for a programme of work to make +:meth:`~iris.cube.CubeList.merge` and :meth:`~iris.cube.CubeList.concatenate` +more robust, efficient, configurable, extensible, lazy and lenient, along with a +new :ref:`developers_guide_specs` section of the Developers Guide to publish it +and any future specs. (:issue:`2761`, :issue:`5375`, :issue:`6790`, +:issue:`7063`, :issue:`7241`) From 61908991b365b21b399dc01555da3009f72e622b Mon Sep 17 00:00:00 2001 From: Bill Little Date: Thu, 10 Sep 2026 21:29:43 +0000 Subject: [PATCH 7/8] docs: rebase the spec baseline onto greenfield The branch was cut from main at c508ac800; greenfield carries #7270 on top of that, which re-solved all three lock files. The lock additions here are re-derived against those fresh locks, and remain minimal - the myst-parser dependency closure and the input_hash, nothing else. The cube.py line references the spec cites are unaffected: #7270's changes are confined to rolling_window, some 4500 lines below the merge and concatenate methods. Co-Authored-By: Claude Opus 5 --- .../specs/2026-09-10-merge-concatenate-design.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/src/developers_guide/specs/2026-09-10-merge-concatenate-design.md b/docs/src/developers_guide/specs/2026-09-10-merge-concatenate-design.md index b6c011a317..fdc430a344 100644 --- a/docs/src/developers_guide/specs/2026-09-10-merge-concatenate-design.md +++ b/docs/src/developers_guide/specs/2026-09-10-merge-concatenate-design.md @@ -15,8 +15,8 @@ {issue}`7241` - **Applies to:** `lib/iris/_merge.py`, `lib/iris/_concatenate.py`, `lib/iris/_combine.py` -- **Baseline:** `main` at `c508ac800`, Iris `3.17.0.dev4`. Line references in - this document are accurate as of that commit. +- **Baseline:** `greenfield` at `253510b37`, Iris `3.17.0.dev4`. Line references + in this document are accurate as of that commit. (merge-spec-1)= From 07c43bb5d2428bae137f8dd8ccf32ed610506394 Mon Sep 17 00:00:00 2001 From: Bill Little Date: Thu, 10 Sep 2026 22:09:20 +0000 Subject: [PATCH 8/8] Spell out spec section citation runs Give every cited section its own section sign, and separate a run of them with a comma or a solidus rather than a dash. A dashed range such as "5.6-5.7" reads as a single citation carrying a stray suffix, so only its first section is unambiguously identified -- by a reader now, and by any cross-reference tooling later. Records the rule on the Design Specs index page, next to the citation prefix table it belongs with, and applies it to the one occurrence in the merge/concatenate spec. Co-Authored-By: Claude Opus 5 --- .../specs/2026-09-10-merge-concatenate-design.md | 4 ++-- docs/src/developers_guide/specs/index.rst | 6 ++++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/docs/src/developers_guide/specs/2026-09-10-merge-concatenate-design.md b/docs/src/developers_guide/specs/2026-09-10-merge-concatenate-design.md index fdc430a344..9359f443fb 100644 --- a/docs/src/developers_guide/specs/2026-09-10-merge-concatenate-design.md +++ b/docs/src/developers_guide/specs/2026-09-10-merge-concatenate-design.md @@ -99,8 +99,8 @@ where cubes refuse to merge but concatenate happily after `new_axis`. {issue}`2761`, open since 2017. `build_indexes` (`_merge.py:585-604`) records, for each scalar value, the *set* of values it co-occurs with. Set membership discards the structure needed to decide separability, so unrelated candidate -dimensions are judged separable. See §5.6–5.7 for the confirmed diagnosis and -fix. +dimensions are judged separable. See §5.6 for the confirmed diagnosis and §5.7 +for the fix. (merge-spec-2-5)= diff --git a/docs/src/developers_guide/specs/index.rst b/docs/src/developers_guide/specs/index.rst index 326358e087..f7470d5cd8 100644 --- a/docs/src/developers_guide/specs/index.rst +++ b/docs/src/developers_guide/specs/index.rst @@ -21,6 +21,12 @@ Each spec declares a **citation prefix** so that its sections can be referred to unambiguously from issues, pull requests and other specs. Within a spec, a bare ``§N.N`` always means that same document. +Give every cited section its own ``§``, and separate a run of them with a comma +or a solidus — ``§5.6, §5.7`` or ``§5.6 / §5.7``, never ``§5.6-5.7``. A dashed +range reads as a single citation with a stray suffix, so only its first section +is unambiguously identified; spelling both out costs one character and leaves +nothing for a reader, or a future cross-reference tool, to guess at. + .. list-table:: :header-rows: 1 :widths: 25 75