Skip to content

Fix add_variables ignoring coords for DataArray bounds#614

Open
FBumann wants to merge 13 commits intomasterfrom
fix/add-variables-dataarray-coords
Open

Fix add_variables ignoring coords for DataArray bounds#614
FBumann wants to merge 13 commits intomasterfrom
fix/add-variables-dataarray-coords

Conversation

@FBumann
Copy link
Collaborator

@FBumann FBumann commented Mar 13, 2026

Summary

Two bugs in add_variables when bounds are DataArrays and coords is provided. Both lower and upper are affected identically.

This is a focused extract from #551 — bug fixes only, no as_dataarray refactoring.

Bug 1: DataArray coords silently ignored → NaN bounds

lower = xr.DataArray([0, 0, 0], dims=["time"], coords={"time": [0, 1, 2]})
var = m.add_variables(lower=lower, coords=[pd.RangeIndex(5, name="time")], name="x")

# master:  var.data.lower.values → [0, 0, 0, NaN, NaN]  (silent NaN)
# fixed:   ValueError: Coordinates for dimension 'time' do not match
lower = xr.DataArray([0, 0, 0], dims=["time"], coords={"time": [10, 11, 12]})
var = m.add_variables(lower=lower, coords=[pd.RangeIndex(5, name="time")], name="y")

# master:  var.data.lower.values → [NaN, NaN, NaN, NaN, NaN, 0, 0, 0]  shape (8,)!
# fixed:   ValueError: Coordinates for dimension 'time' do not match

The fix validates DataArray bounds against coords: mismatched or extra dims raise ValueError, missing dims are broadcast via expand_dims, reordered coords (same values) are reindexed.

Bug 2: Dict coords crash for all input types

m.add_variables(lower=0, coords={"x": [0, 1, 2]}, name="x")

# master:  CoordinateValidationError (dims not inferred from dict keys for scalar upper=inf)
# fixed:   works — shape (3,)

Fixed by inferring dims from dict keys in as_dataarray and numpy_to_dataarray for scalar/0-dim inputs.

Changes

File Change
linopy/model.py _validate_dataarray_bounds + _coords_to_dict helpers, called in add_variables
linopy/common.py Infer dims from dict keys for scalars/0-dim arrays
test/test_common.py 45 new parameterized tests (16 fail on master, all pass here)

No existing tests were modified or removed — all changes to test/test_common.py are purely additive. All 32 existing test functions from master are preserved.

Test results: master vs this branch

Category # Master Fixed
All types × dict-coords 9 Crash (CoordinateValidationError) Pass
DataArray coord mismatch/extra dims 5 Silent NaN (wrong bounds, no error) ValueError
Reordered coords (same values) 1 Pass (xarray alignment, no NaN) Explicit reindex
Reordered coords (different values) 1 Silent NaN (wrong bounds, no error) ValueError
All types × seq-coords, mixed combos, broadcast, inferred coords, string/datetime, MultiIndex 29 Pass Pass
Total 45 29 pass, 16 fail 45 pass

Full test suite: 2570+ passed, 236 skipped.

🤖 Generated with Claude Code

FBumann and others added 2 commits March 13, 2026 09:18
When DataArray bounds were passed to add_variables with explicit coords,
the coords parameter was silently ignored because as_dataarray skips
conversion for DataArray inputs. Now validates DataArray bounds against
coords: raises ValueError on mismatched or extra dimensions, and
broadcasts missing dimensions via expand_dims.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@FBumann FBumann marked this pull request as draft March 13, 2026 08:27
FBumann and others added 10 commits March 13, 2026 09:31
Test MultiIndex coords (validation skip), xarray Coordinates object,
dims-only DataArrays, and upper bound mismatch detection.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Infer dims from dict keys when dims is None and the input is a scalar.
Previously this raised xarray's CoordinateValidationError because
xarray can't broadcast a 0-dim value to coords without explicit dims.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Consolidate add_variables tests into TestAddVariablesBoundsWithCoords
class with parameterized tests covering all bound types (scalar,
np.number, numpy, pandas, list, DataArray, DataArray-no-coords) x
both coord formats (sequence, dict). Also fixes as_dataarray for
scalars with dict coords by inferring dims from dict keys.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Test DataArray+numpy, DataArray+scalar, DataArray+DataArray combos for
lower/upper. Also test both bounds covering different dim subsets with
broadcast, and that only the mismatched bound raises ValueError.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add numpy-0d and dataarray-0d to the parameterized bound type tests.
Fix numpy_to_dataarray to infer dims from dict keys for 0-dim arrays,
matching the scalar fix in as_dataarray.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Cover three gaps: coords inferred from bounds (no coords arg) for
DataArray and pandas, multi-dimensional coord specifications with
both scalar and DataArray bounds, and real-world coordinate types
(string regions, datetime index) including mismatch detection.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Verify lower(time, space) and upper(space, time) align correctly
via xarray broadcast.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When a DataArray bound has the same coordinate values as coords but in
a different order, reindex to match instead of raising ValueError.
Still raises when the values actually differ (not just reordered).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove unreachable hasattr(coords, "dims") branch in _coords_to_dict
(xarray Coordinates are Mappings, caught by isinstance check above).
Add Any type annotations to parameterized test arguments.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@FBumann FBumann marked this pull request as ready for review March 13, 2026 09:14
@FBumann
Copy link
Collaborator Author

FBumann commented Mar 13, 2026

@FabianHofmann Sorry for all the branches. I really wanted to make the fix more universal and establish a distinction between converting to a dataarray and converting to a dataarray and validate, but i hot lost in the process.
This PR now soley focuses on add_variables() bugs and inconsistencies.

FBumann added a commit to FBumann/fluxopt that referenced this pull request Mar 13, 2026
Pin to the fix/add-variables-dataarray-coords branch which fixes
DataArray coords handling in add_variables.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@FBumann FBumann requested a review from FabianHofmann March 13, 2026 11:37
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