fix(groupby): group by non-dimension coordinate names; fast multi-key grouping by names (#750, #753)#751
Conversation
`LinearExpression.groupby` could not group by an attached non-dimension
coordinate. `expr.groupby("period").sum()` raised `ValueError: period
already exists as coordinate or variable name`, and passing the coordinate
`DataArray` (`groupby(period)`) raised because the fast path dropped only
the dimension index, then renamed the group dim onto a name still held by
the attached coordinate.
Fix both paths:
- `sum()` resolves a string group naming an existing coordinate to that
coordinate so it takes the fast path, and drops every coordinate aligned
to the grouped dimension (index, MultiIndex levels, auxiliary coords)
before reshaping, since collapsing the dimension invalidates them all.
- The `groupby` property detaches an attached non-dimension coordinate used
as the group before handing it to xarray, so xarray does not try to
re-expand it when recombining groups (the `use_fallback=True` path).
`expr.groupby("period").sum()` now mirrors `xarray.Dataset.groupby`.
Closes #750
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Reorganize the #750 coverage into TestGroupbyByAttachedCoordinate, a parametrized matrix that asserts the grouped expression against hard-coded `vars`/`coeffs` literals (on a deterministic 4- and 8-variable model) instead of comparing to a sibling computation that could share the same bug. Covers single-key (name / DataArray x use_fallback), multi-key (list / tuple x use_fallback), an extra auxiliary coord on the grouped dimension, and a 2-D variable that must keep its other dimension. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tract A single-element key list (`groupby(["period"])`) now groups like the scalar key, matching xarray -- it is unwrapped in both `sum()` and the `groupby` property. Multi-key grouping must be spelled with names (`["period", "season"]`); a list of `DataArray`s is unhashable and raises in xarray itself, so linopy mirrors that (covered by an explicit `pytest.raises` row in the matrix). Also shorten the test class docstring to house style. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The GH #750 detach must only drop *free* (non-indexed) coordinates. The earlier change also dropped a MultiIndex level when grouping by it via `use_fallback=True`, leaving the dimension without an index (`('snapshot',) are not coordinates with an index`). Guard the detach with `group.name not in data.xindexes` so MultiIndex levels are left intact. Grouping by a MultiIndex level now works on both paths (the pydata/xarray 6836 case, fixed upstream). Add a parametrized regression test over both levels and both paths. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Extract the single-element-list unwrap + coordinate-name resolution shared by `sum()` and the `groupby` property into one `_resolve_group` helper, removing the duplication (and the drift between the two that caused the earlier MultiIndex-level regression). - Drop the now-stale `(GH #750)` references from code comments; the link lives in the release notes. - Add a test for grouping by a dimension coordinate name (the fast-path broadening), and note it in the release note. - Simplify `test_multi_key`: a multi-key group always uses the xarray fallback, so drop the redundant `use_fallback` parametrization. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Grouping by a dimension coordinate or a MultiIndex level by name already worked; only the non-dimension (free) coordinate case was broken. Correct the release note, which had overclaimed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Drop the verbose comment blocks; the rationale lives in the _resolve_group docstring and the regression tests enforce the invariants (e.g. test_multiindex_level guards the xindexes detach guard). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
63e82ef to
ae23a3c
Compare
63e82ef to
de067dc
Compare
`groupby(["a","b"]).sum()` previously dropped to the slow xarray fallback. Resolve a list of coordinate names (1-D, same dim) to a value frame so it rides the existing reindex fast path, then unstack the stacked result back into one dimension per key -- byte-identical to the fallback, sparse fill cells included. The DataFrame grouper is untouched and stays compact (stacked MultiIndex over observed combinations only), so this is non-breaking. One dimension per key is a dense cartesian grid, so a sparse key crossing materialises mostly-fill cells. Warn (pointing at the DataFrame grouper) when the grid is much larger than the observed combinations; the check reads the collapsed MultiIndex levels, so it is O(observed) and fires before unstack allocates. See #753; sparse-representation follow-ups tracked against #740. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
de067dc to
3cc774f
Compare
|
Note The following content was generated by AI (Claude Code), posted on behalf of @FabianHofmann. A follow-up commit added an expr.groupby(["period", "season"]).sum(observed=True)keeps the result stacked over only the observed key combinations (a The multikey gather and unstack epilogue were also extracted into helpers ( |
|
Amazing! |
Prove a first-class pd.MultiIndex snapshot can be replaced by a flat dim + level aux coords without changing the model, by explicit equality checks: reset_index normalization, where/groupby selection, per-period roll (#751), group-by-level (#751), an equivalent solved LP, and the output re-stack round-trip (flat solution -> MI via the caller's own mapping). Pins that Variable.sel does not forward MI tuple-select (#752 §2). Both semantics. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Prove a first-class pd.MultiIndex snapshot can be replaced by a flat dim + level aux coords without changing the model, by explicit equality checks: reset_index normalization, where/groupby selection, per-period roll (#751), group-by-level (#751), an equivalent solved LP, and the output re-stack round-trip (flat solution -> MI via the caller's own mapping). Pins that Variable.sel does not forward MI tuple-select (#752 §2). Both semantics. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…744) (#803) * test: MultiIndex -> flat+aux feasibility checks (#744) Prove a first-class pd.MultiIndex snapshot can be replaced by a flat dim + level aux coords without changing the model, by explicit equality checks: reset_index normalization, where/groupby selection, per-period roll (#751), group-by-level (#751), an equivalent solved LP, and the output re-stack round-trip (flat solution -> MI via the caller's own mapping). Pins that Variable.sel does not forward MI tuple-select (#752 §2). Both semantics. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): MultiIndex -> flat+aux feasibility matrix (#744) Discussion artifact for the v1 MultiIndex decision: feasible (verified) vs desirable (opinion) matrix per op, with PyPSA call sites pinned at v1.2.4; the snapshot dim-coordinate sub-decision; the conclusion that linopy drops MultiIndex from its mental model (flat in, flat out, output re-stack owned by PyPSA); and the open PyPSA-side rows (stochastic 2nd MI, n.snapshots). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(mi): storage-SOC LP-file equivalence; refine matrix (#744) - add test_storage_soc_lp_equivalent: the cyclic-per-period SOC "previous" built via groupby("period").roll vs an explicit positional roll produces a byte-identical LP file; a period-unaware global roll is the negative control (diverges at the period-start rows), proving the check has teeth - matrix: drop the non-operation "solve LP" row (the end-to-end compose proof is noted inline on the tracked-tests line instead); mark storage SOC 🟢 now that it is pinned by a test Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(mi): cover non-cyclic SOC boundary (ramps too) (#744) Parametrize test_storage_soc_lp_equivalent over boundary in {cyclic, non-cyclic}. Non-cyclic (period start carries nothing) is the boundary ramps and non-cyclic storage use -- groupby("period").roll + a period-start where-mask vs an explicit positional roll, byte-identical LP either way. Teeth: the two boundaries yield different LPs (mask/wrap is not a no-op); the period-unaware global-roll control stays cyclic-only since non-cyclic masks away the rows it diverges on. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(mi): separate ramp (row-drop) from non-cyclic SOC (where) (#744) The prior "ramps too" conflated two distinct period-start boundaries, now both covered as a third parametrize value: - non-cyclic storage: drop the previous *term*, keep the row (initial SOC -> rhs). This is exactly PyPSA's `previous_soc...roll(1).where(include_previous_soc)` (constraints.py L1691) -- a `where` on the term, verified byte-faithful. - ramp: drop the whole *row* at the period start via a constraint `mask=` (PyPSA's `_period_start_mask`, L838) -- no previous to ramp from. flat (groupby roll) == oracle (positional roll) byte-for-byte in all three; each boundary differs from the cyclic baseline (teeth); global-roll control stays cyclic-only since the other two mask away the rows it diverges on. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(mi): rename to test_period_boundary_lp_identical; show 3 boundaries (#744) The test's scope is the per-period roll's period-start boundary (cyclic / non-cyclic / ramp), not storage SOC specifically -- and it proves a byte-IDENTICAL LP file, distinct from the solve-equivalence test_per_period_lp_equivalent. Rename accordingly. Surface all three boundary spellings in the storage-SOC matrix row's flat+aux cell (wrap / .where term-drop / mask= row-drop). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): stochastic is N-D, not a second MI (#744) PyPSA's stochastic path keeps `scenario` and `name` as separate xarray dims (components/array.py: expand_dims(scenario=...), isel(scenario=0)); the `(scenario, name)` MultiIndex is manufactured only by `.stack(combined=...)` at the pandas output boundary, and common.py's MI is the input DataFrame's columns. So it is not an in-model MI like the snapshot `(period, timestep)` index -- it is already the flat+aux N-D form, nothing to decompose. Correct the stochastic row's verdict and the conclusion/decision-record prose: the one real open item is the public `n.snapshots` choice; stochastic drops from "genuine open risk / second MI" to a low-risk watch on whether #1484 ever introduces an in-model stacked index (v1.2.4 does not). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): tag boundary / non-linopy-MI rows with ⊥ (#744) Six rows are the genuine in-linopy MultiIndex decision (snapshot (period, timestep) sitting inside linopy variables). The other three are not, and now carry a ⊥ tag + legend: output (PyPSA-side boundary re-stack), n.snapshots (PyPSA public API, never a linopy model index), stochastic (not an MI at all — N-D dims). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): lead with the MI-scope premise; add reset_index root cause (#744) Open with a bold, falsifiable claim: inside the linopy model PyPSA uses a MultiIndex in exactly one place -- the snapshot dim (period, timestep). The lookalikes are ruled out inline (stochastic = N-D dims, n.snapshots = public API, output = pandas .stack), so the whole linopy-side question collapses to one axis; other axes are assumed N-D, not audited -- corrections welcome. Add a "What reset_index changes (and doesn't)" section stating the root cause: snapshot is one dim whose index is a MultiIndex, period/timestep are levels (never dims, surfacing only via .sel-collapse), and reset_index flips only the index type -- not the dim count -- turning collapse/loop/.data into direct groupby/where. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): state the snapshot-only MI scope plainly (#744) Drop the "working assumption / corrections welcome" framing -- just state that inside the linopy model MultiIndex is used only on the snapshot dim, with the three lookalikes ruled out inline. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): output is in-linopy MI (snapshot out), not ⊥ boundary (#744) solution/dual carry the snapshot dim, which today is the MultiIndex inherited from the MI-indexed variables -- so output holds the same in-model snapshot MI as entry, on the way out. Drop the ⊥ tag; show the snapshot MI flowing in (entry) -> ops -> out (output) in the lead and legend. Only stochastic (N-D) and n.snapshots (public API) remain ⊥. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(mi): pin snapshots-param flat rebuild; matrix 🔵→🟢 (#744) PyPSA parks the snapshot index on model.parameters (assign(snapshots=sns), optimize.py L689) and reads it back via .snapshots.to_index() (L905) -- today that lands a real MultiIndex *inside* the linopy object. test_snapshots_param_flat_rebuild shows the flat+aux form parks only a flat snapshot + period/timestep aux vars (no MI index inside parameters) and rebuilds the identical index on demand. Flips the last in-linopy 🔵 row to 🟢. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): sink the ⊥ boundary rows to the bottom (#744) Order the matrix as the seven in-linopy snapshot-MI rows first (entry → ops → storage SOC → output → snapshots param), then the two ⊥ rows that are outside linopy (stochastic, n.snapshots) last. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): split PyPSA-side MI usages into their own table (#744) The feasibility matrix is now exactly the seven in-linopy snapshot-MI rows (all 🟢). The two ⊥ rows move to a dedicated section, "Observed PyPSA MultiIndex usages — not linopy's to solve": stochastic and n.snapshots are observed PyPSA usages that provably need no in-linopy solution and are cheap for PyPSA to handle at its boundary. They fail the in-linopy-MI test on opposite axes -- stochastic is inside linopy but not an MI; n.snapshots is an MI but not inside linopy -- which the new table states directly. Drop the now-unused ⊥ tag/legend. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): BLUF-first rewrite; fix glyph overload; condense (#744) Three structural fixes from review, one coherent pass: - Lead with a Verdict (BLUF): feasible with zero linopy changes, flat in/flat out, one open PyPSA-side item (n.snapshots). Everything after is evidence for a claim the reader already holds, instead of arguing up to it. - Fix the per-column glyph overload: feasibility uses glyphs (✅ tested / 🔲 achievable / ❌ no), desirable uses plain words (better / parity / worse) -- a glyph no longer means two things across columns. - Hoist the settled-vs-open framing above the matrix (Scope + the two-table split already embody it), drop the now-redundant decoder paragraph and restated conclusion. Net 211 → 160 lines; same evidence and call sites, less argument. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): correct "zero changes" → net subtraction; add strip analysis (#744) "Zero linopy changes" overclaimed. Feasibility needs no new capability, but adopting flat+aux is a net simplification, not a no-op: a few small additive things (positional snapshot alignment; shrink MI-input to accept-then-reset_index) against a large deletion of first-class-MI machinery. Add a "What linopy can strip" section quantifying the deletion from a read-only code sweep: ~300 lines, ~half of it one cluster -- the alignment.py level-projection subsystem (_project_onto_multiindex_levels / _enforce_implicit_projections / _LevelProjection + the projections plumbing through broadcast_to_coords), which exists solely to make a single MI level align like a dimension and is dead under flat+aux; plus ~50 lines of netcdf MI flatten/reconstruct. Notes what stays (assign_multiindex_safe, internal _term/groupby stacking, semantics §11 aux-conflict). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): frame the verdict as four wins, not just code-strip (#744) The case for dropping first-class MI isn't only "~300 fewer lines". Reframe the Verdict around four axes, each backed by a section below: - simpler implementation (What linopy can strip) - simpler mental model (What reset_index changes) - enables features (level groupby works; removes the #752 internals coupling) - no UX cost (MI still accepted in / re-stacked out; only level-tuple .sel lost) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): frame the mental-model win as linopy's own (#744) The simpler mental model is linopy's: it stops having to know pd.MultiIndex exists or cover its quirks (level alignment, index-corruption guards, reconstruction) and becomes purely xarray-native -- plain dims + aux coords, what it's already built on. MI lives only at the boundary as input/output sugar, never inside linopy. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): rewrite Verdict in the maintainer's voice (#744) Lead with the linopy maintainer's actual question -- can the library be simplified without destabilising PyPSA, its primary consumer -- and answer it: PyPSA's interface doesn't move (MI in, MI-indexed out as boundary sugar), every in-linopy use has a tested identical-model flat+aux form, so nothing PyPSA relies on breaks. Stability is the frame; the wins (less to maintain, xarray-native internals, unblocks work) follow from it, and the one open item is PyPSA's own decoupled n.snapshots call. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): attribute cases to PyPSA; state the generalization assumption (#744) The rows are PyPSA's MI uses *of* linopy, not linopy's own -- reword accordingly. And make the epistemics explicit: PyPSA is the one consumer audited, so these are a proof set, not a proof of universality. The argument still generalises (after reset_index everything is ordinary xarray linopy already handles; the only MI-specific capability lost for any user is level-tuple .sel), but we now state the assumption that no other user has an infeasible MI case, and invite counterexamples. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): dense rewrite — Decision + Appendix shape (#744) Restructure to one decision screen + appendix (the chosen shape), 216 → 141 lines: - Verdict: claim + 3 wins + open item + proof-set caveat, tightened. - The evidence: both tables (in-model matrix, boundary usages) under one heading; the three post-matrix findings paragraphs collapse to one caption; Scope and the side-finding fold in. - The payoff: strip table + a line; "stays"/"additive" compressed. - Appendix: the three overlapping mechanics sections merged/condensed (reset_index + data-model table, dim-coordinate sub-decision, transition shape) + decision record. Each claim stated once; explanatory prose turned into captions. Tables and all call-site links preserved verbatim; only the #756/#757 links in a memory-cost aside were trimmed (the point survives without them). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): fix "MI-indexed results back" — linopy returns flat (#744) linopy does not hand MI-indexed results back; that contradicts flat-in/flat-out. It returns a flat solution; PyPSA re-applies its own n.snapshots index when mapping the result onto components (assign_solution already does this), so PyPSA's results stay MI-indexed -- but the re-stack is PyPSA's, not linopy's. MI is boundary sugar PyPSA owns, never inside linopy's model. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): resolve "no change" vs "input sugar" contradiction (#744) Separate feasibility from the entry policy. Feasibility needs no new linopy capability (the rewrites use existing ops; entry is a plain reset_index). Whether linopy accepts an MI and flattens it (input sugar, backwards-compatible) or rejects MI so callers flatten first (purest, breaks existing callers) is a boundary policy, now surfaced as an explicit Decision-record item with a recommendation (accept-as-sugar, optional deprecation path to flat-only) -- not smuggled in as both "no change" and "accept sugar" at once. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): consolidate into a Design decisions table (#744) Replace the prose "decision record" + scattered "small additive work" mentions with one top-level Design decisions table: internal model, MI-on-input, snapshot alignment, output, n.snapshots -- each with options and a recommendation/status. Also fold in the input-policy correctness fix: MI in / flat out is a silent contract change, so silent auto-flatten is ruled out -- reject, or accept-and-warn (undecided). Slim the Verdict to point at the table; only n.snapshots is flagged as the substantive open item, and it's PyPSA's call. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): MI surface is bigger — two-layer payoff + corrected sweep (#744) The ~300 estimate was a floor. Two corrections from a deeper measure: - assign_multiindex_safe (the #303 "would corrupt the index" workaround, 39 call sites) is snapshot-MI-driven, not a general helper: internal stacks use create_index=False, so the snapshot MI is the only thing that triggers the corruption warning. Under flat+aux all 39 sites revert to plain .assign(). The first sweep wrongly marked it "stays" -- corrected, with a strip-table row. - Add the diffuse "cognitive tax" layer the line-count misses: 6 isinstance(MI) guards, 17 level-ops, ~40 quirk-comments, and 169 lines of MI edge-case tests across 10 files. Reframe the mental-model pillar around that defensive surface (guards, the 39-site corruption workaround, quirk-comments, edge-case tests) rather than just lines. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): add latent-risk and MI-not-promoted arguments (#744) Two more points strengthening the case: - Latent risk: MI's edge surface almost certainly isn't fully covered (the #303 corruption is one gap that already bit), so keeping MI carries unknown-bug risk that flat+aux retires. Folded into the mental-model pillar and the payoff. - MI isn't a promoted feature, so few users lean on it -- lowers the cost of the lost level-tuple .sel and makes the "no other infeasible case" generalization safer. Folded into the proof-set caveat. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): add the second MI surface — multi-key groupby (#744) The "MI in exactly one place — snapshot" claim was false. Verified here (both semantics): a multi-key groupby (DataFrame grouper) mints a stacked `group` MultiIndex, and PyPSA-Eur's add_CCL_constraints consumes it richly (groupby(country×carrier).sum() then .intersection / .loc[MI] / .sel(group=index), solve_network.py L1064/L1080-1095). add_EQ/add_BAU are single-key (no group MI) -- correcting an over-broad relayed claim. Integrate at "2nd open item, flagged harder": scope the Verdict's snapshot claim, turn one open item into two (n.snapshots + multi-key groupby), add a "Second MI surface" section and a Design-decisions row, and make the proof-set caveat honest (the generalization already cracked once). Framing per FBumann: it's the same flat+aux change, linopy-owned (groupby returns flat group + key aux coords) -- but a public-API change with external consumers, so it needs a deprecation path, not a boundary fix. Blast radius (-Earth/-DE/plugins) not yet scoped. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): fold multi-key groupby into the matrix as the 🔲 row (#744) Per FBumann: now that the deprecation is just the standard legacy/v1 path (warn under legacy, raise/flat+aux under v1), multi-key groupby is the same kind of in-linopy MI as the snapshot rows -- siloing it overstated its difference. Move it into table 1 as the 8th row, marked 🔲 (the one achievable-not-yet-tested row, vs the 7 tested ✅ -- which also finally exercises the glyph legend). Replace the standalone "Second MI surface" section with a tight caption (verified both semantics; PyPSA-Eur CCL cite; legacy/v1 migration; blast radius the only open part). Soften the earlier "harder / needs a deprecation path" framing throughout: it reaches external code but rides the standard switch, so it's guided, not a hard break. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(v1): multi-key groupby returns a flat group dim under v1 (#744) A frame grouper (`groupby(DataFrame).sum()`, e.g. PyPSA-Eur's CCL country×carrier) mints a stacked `group` MultiIndex today. Under v1 it now returns a flat `group` dim with the keys as ordinary aux coords — uniform with every other dim, no MI; select via groupby/where, not a `.sel(group=tuple)` level tuple. Legacy keeps the stacked MultiIndex and emits a DeprecationWarning pointing at the change. Scope: only the cases where a stacked group MI survives today — the DataFrame grouper and `observed=True`. The default `observed=False` list-of-names path that already unstacks into separate dims is untouched. The legacy deprecation warning is limited to the user-facing frame grouper. This is the first transition slice for dropping first-class MultiIndex in v1 (the strip of the now-dead-under-v1 MI machinery follows at legacy EOL). Affected groupby tests split into @pytest.mark.legacy (MI + asserts the warning) / @pytest.mark.v1 (flat+aux) twins. Full suite green under both semantics. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: make the MI `u` fixture semantics-aware for the v1 MI drop (#744) Toward v1 disallowing MultiIndex entirely: the shared `u` fixture now yields a (level1, level2) MultiIndex variable under legacy and the equivalent flat `dim_3` dim + level1/level2 aux coords under v1. Moved it out of the central `m` fixture so `m` is MI-free. The two v1 tests that asserted MI-specific behavior are rewritten to the flat+aux idiom: the level-projection "raises" test becomes explicit per-level weighting via the aux coord; the MI inner-join alignment test is marked legacy (flat-dim alignment is covered by the other align tests). Prep only — MI is still allowed under v1 at this point; no v1 test relies on it now. Suite green under both semantics. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(v1): disallow MultiIndex dim-coords entirely (#744) v1 now rejects a pandas MultiIndex dimension anywhere it would enter the model; legacy keeps it with a DeprecationWarning. This supersedes the branch's earlier "MI-allowed-but-strict" v1 design — under v1 a snapshot (or any dim) must be a flat dim with the levels as auxiliary coords (`reset_index`). enforce_no_multiindex(obj, *, context) scans an object's dim-indexes for a MultiIndex and raises (v1) / warns (legacy) with a message naming what the user did. It's wired at the caller-facing boundaries where MI can enter a persistent object, so the error is precise: - add_variables → context="variable 'x'" (catches coords and DataArray bounds) - add_constraints → context="constraint 'c'" (catches MI introduced via arithmetic) Tests: MultiIndex is now a legacy-only concept, so MI-building tests are marked legacy (or their fixtures skip under v1), and the obsolete @v1 tests that asserted the old MI-under-v1 semantics are removed and replaced by a v1 reject test. Full suite green under both semantics. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(v1): guard add_objective against MultiIndex; hoist imports (#744) add_objective assigns the expression directly (bypassing Objective.__init__), so add the enforce_no_multiindex check there too — an MI forced onto an expression and handed to the objective now rejects under v1 with the "objective ..." context. Covered by a v1 reject test alongside the add_variables/add_constraints ones. Also move the `enforce_no_multiindex` import to module level in model.py (no import cycle — semantics imports none of model/variables/constraints/objective). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): track open-items.md; mark #744 resolved (#744) Commit the v1-rollout checklist (previously untracked) as the shared status tracker, refreshed to the current state: the one open design decision (#744, MultiIndex storage) is resolved — v1 disallows MI — and the remaining work is organised by goals.md's three stages (opt-in / default / 1.0). The transition-surface-complete + changelog items are placed at stage 1, matching goals.md step 1 (warn on legacy, raise on v1). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(v1): repr shows flattened MultiIndex as its level coords A flat dim carrying decomposed-MI levels as aux coords now renders like a first-class MultiIndex — header 'dim (l0, l1)', rows '[(a, 1)]', var refs 'x[(a, 1)]' (incl. explicit LP/MPS export names). Shared helpers in common.py: index_level_coords / dim_level_names / label_coord / get_printout_labels. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Fabian <fab.hof@gmx.de> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
What
Brings
LinearExpression.groupbyto parity withxarray.Dataset.groupbyfor grouping by coordinate names — on the fast path, with no breaking change. Two related pieces:1. Group by an attached non-dimension coordinate (closes #750).
Both now work and take the fast path, mirroring
xarray. (Grouping by a dimension name always worked — only attached non-dim coordinates were broken.) The old workaround was to detach the coord first (expr.drop_vars("period").groupby(period).sum()).2. Multi-key grouping by names now takes the fast path (closes #753).
It now rides the reindex fast path and returns the same output — one dimension per key, byte-identical to the fallback, sparse fill cells included. The
pd.DataFramegrouper is untouched and keeps its compact stacked-MultiIndexoutput, so this is non-breaking.How
In
LinearExpressionGroupby:_resolve_groupnormalizes a key: unwrap a single-element list (groupby(["period"])→ scalar), and resolve a string coord name to its coordinate so it takes the fast path.sum()drops every coordinate aligned to the grouped dimension before reshaping, so an attached aux coord (including the one being grouped by) no longer collides on the finalrename.groupbyproperty detaches a free (non-indexed) coordinate before handing it to xarray — fixing theuse_fallback=Truepath — but never aMultiIndexlevel.unstacked back into one dimension per key.Memory
One dimension per key is a dense cartesian grid, so a sparse key crossing materialises mostly-fill cells (measured ~100× vs the compact
DataFramegrouper for a diagonal crossing — see #740). AUserWarningnudges sparse/high-cardinality users to theDataFramegrouper; it reads the collapsedMultiIndexlevels, so it is O(observed), not O(N), and fires beforeunstackallocates the grid. Getting separate dims and compact storage would need a sparse/long-format kernel — tracked in #757 (the groupby-densification follow-up) under the sparse-kernel umbrella #756.Tests
TestGroupbyByAttachedCoordinateasserts groupedvars/coeffsagainst hard-coded results on a deterministic model: single-key (name/DataArray×use_fallback), 2-D variable, dimension-coordinate-by-name, single-element list,MultiIndexlevel, and apytest.raisesrow pinning that a list ofDataArrays is unsupported.TestMultiKeyFastPathcovers the multi-key fast path: fast == fallback (list/tuple, including a sparse crossing), separate-dims-not-stacked, sparse-combination-filled, DataFrame-grouper-stays-compact, and the blow-up warning (fires when sparse, silent when dense). Full suite green; ruff + mypy clean.Context: relation to PyPSA usage and #744
This is a building block for the flat-dimension + auxiliary-level-coord direction discussed in #744. If
n.snapshotsbecomes a flat dimension carryingperiod/scenariolevel coordinates (instead of a stackedpd.MultiIndex), aggregating an expression over a level becomesexpr.groupby("period").sum()— exactly what this PR makes work, now also for multiple levels at once (groupby(["period", "scenario"])).PyPSA still carries per-period workarounds citing a broken MultiIndex groupby (pydata/xarray#6836); that upstream bug is now fixed (verified on xarray 2025.9.0), but those comments are about MultiIndex grouping and per-period rolling, orthogonal to this PR.
Closes #750, #753. Drafted with an agent (Claude Code).
🤖 Generated with Claude Code