Skip to content

fix: don't leak aux coords of summed dims onto the term dimension#829

Open
FBumann wants to merge 2 commits into
masterfrom
fix-295-aux-coord-term-leak
Open

fix: don't leak aux coords of summed dims onto the term dimension#829
FBumann wants to merge 2 commits into
masterfrom
fix-295-aux-coord-term-leak

Conversation

@FBumann

@FBumann FBumann commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Fixes the remaining alignment-leak half of #295.

Note

The following content was generated by AI.

Changes proposed in this Pull Request

LinearExpression._sum drops a summed dimension's index coordinates via reset_index, but an auxiliary (non-dimension) coordinate on that dimension — e.g. a B(A) group label — rode the .stack() onto the internal _term helper dimension. The leaked coordinate then broke later arithmetic:

>>> v = v.assign_coords({"B": ("A", [311, 311, 322])})
>>> (1 * v).sum("A") >= xr.DataArray(10)
CoordinateValidationError: coordinate B has dimensions ('_term',), but these are not a subset of the DataArray dimensions ()

Aux coords whose dims intersect the summed dims are now dropped before stacking. Aux coords on the remaining dimensions still propagate unchanged. QuadraticExpression.sum and the groupby fallback route through the same _sum and are covered by the same change.

Only the leak is fixed here — it is a plain bug under both semantics. The other half of #295's residual (a conflicting aux coord being silently kept in binary operations) is a semantics change and stays with the v1 work (convention §11).

Checklist

  • AI-generated content is marked (see AGENTS.md).
  • Code changes are sufficiently documented.
  • Unit tests for new features were added: aux coord on a summed dim is dropped (incl. the Auxiliary non-dimension coordinates leak into expressions and break alignment #295 reproducer), aux coord on a kept dim propagates.
  • A note for the release notes doc/release_notes.rst of the upcoming release is included.
  • I consent to the release of this PR's code under the MIT license.

@codspeed-hq

codspeed-hq Bot commented Jul 14, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 173 untouched benchmarks
⏩ 173 skipped benchmarks1


Comparing fix-295-aux-coord-term-leak (7fb776a) with master (c02f5db)

Open in CodSpeed

Footnotes

  1. 173 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

LinearExpression._sum dropped a summed dim's index coords via
reset_index but let auxiliary coords defined on that dim ride the stack
onto _term, breaking later arithmetic with CoordinateValidationError
(#295). Drop them before stacking; aux coords on remaining dims still
propagate.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@FBumann FBumann force-pushed the fix-295-aux-coord-term-leak branch from 4aaf556 to e610c04 Compare July 14, 2026 14:58
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant