You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
We have one more place where positional alignment leads to silent errors. Should we fix this immediately or as part of the v1 convention?
Note
This issue was written by AI while reviewing #824. All behaviors were verified by running reproducers against current master and against 12e54d9~1 (the commit before #802).
Bug
On the default (fast) path, groupby(...).sum() matches a pd.Series or DataArray grouper to the expression by position, ignoring its labels. A grouper whose index is ordered differently from the expression's coordinates silently regroups. The other grouper paths behave differently, so there are three alignment semantics side by side:
grouper
labels reordered relative to the data
pd.Series / DataArray, fast path
silently grouped by position — wrong result
pd.DataFrame, fast path
reindexed by label — correct
any grouper, use_fallback=True
raises (xarray exact join)
A grouper with a mismatching label set either fails deep in the scatter kernel with an unhelpful ValueError: array is not broadcastable to correct shape, or silently succeeds positionally when the sizes happen to match.
Reproducer
Both groupers below encode the identical mapping i -> g ({0: 1, 1: 1, 2: 2, 3: 2}), just stored in reversed order — so all four calls should build the same expression.
(The final ValueError misdiagnoses the reorder as a length mismatch — both sides have 4 entries along i. xarray's _resolve_group re-raises everyAlignmentError from its exact join with that hardcoded length message; the accurate diagnosis only survives as the chained cause.)
The same holds for N-D DataArray groupers (after #824) with reordered coordinates.
History
Not a regression from #802 — verified at 12e54d9~1: positional matching dates back to the introduction of the fast path in 769e540 (June 2023). Related to, but distinct from, #823/#824 (which fixed the dimensions of multi-dim groupers, not alignment).
Suggested fix
Align indexed groupers to the expression's indexes before grouping:
same label set, different order → reorder by label (a reindex),
different label set → raise ValueError,
grouper without an index (nothing to align by) → keep positional matching.
This makes all three grouper paths consistent and matches the v1 arithmetic convention's §8 ("shared dimensions must carry the same labels": align by label, never by position; only a label-set difference raises), so groupby groupers would follow the same rule as operands.
We have one more place where positional alignment leads to silent errors. Should we fix this immediately or as part of the v1 convention?
Note
This issue was written by AI while reviewing #824. All behaviors were verified by running reproducers against current
masterand against12e54d9~1(the commit before #802).Bug
On the default (fast) path,
groupby(...).sum()matches apd.SeriesorDataArraygrouper to the expression by position, ignoring its labels. A grouper whose index is ordered differently from the expression's coordinates silently regroups. The other grouper paths behave differently, so there are three alignment semantics side by side:pd.Series/DataArray, fast pathpd.DataFrame, fast pathuse_fallback=TrueA grouper with a mismatching label set either fails deep in the scatter kernel with an unhelpful
ValueError: array is not broadcastable to correct shape, or silently succeeds positionally when the sizes happen to match.Reproducer
Both groupers below encode the identical mapping
i -> g({0: 1, 1: 1, 2: 2, 3: 2}), just stored in reversed order — so all four calls should build the same expression.pd.Seriesgrouper:DataArraygrouper — same silent flip:The fallback refuses the reordered grouper outright instead of regrouping:
(The final
ValueErrormisdiagnoses the reorder as a length mismatch — both sides have 4 entries alongi. xarray's_resolve_groupre-raises everyAlignmentErrorfrom its exact join with that hardcoded length message; the accurate diagnosis only survives as the chained cause.)The same holds for N-D
DataArraygroupers (after #824) with reordered coordinates.History
Not a regression from #802 — verified at
12e54d9~1: positional matching dates back to the introduction of the fast path in 769e540 (June 2023). Related to, but distinct from, #823/#824 (which fixed the dimensions of multi-dim groupers, not alignment).Suggested fix
Align indexed groupers to the expression's indexes before grouping:
ValueError,This makes all three grouper paths consistent and matches the v1 arithmetic convention's §8 ("shared dimensions must carry the same labels": align by label, never by position; only a label-set difference raises), so groupby groupers would follow the same rule as operands.