fix: regression with tuple coords entries as xarray's (dim_name, values)#766
Open
FBumann wants to merge 3 commits into
Open
fix: regression with tuple coords entries as xarray's (dim_name, values)#766FBumann wants to merge 3 commits into
FBumann wants to merge 3 commits into
Conversation
_coords_to_dict treated a tuple coords entry as a bare value sequence
(like a list), which broke the standard xarray form
``coords=[("origin", origins)]`` — a scalar bound against such coords
raised an inscrutable "inhomogeneous shape" error from numpy. v0.7.0
delegated to xarray and supported this form; the #742 alignment refactor
regressed it (unreleased) and pinned the value-sequence reading in tests.
Restore xarray's convention: a tuple entry is ``(dims, data[, attrs])``
(first element names the dimension), while a bare value sequence uses a
list. Length-1 / scalar-value tuples raise a clear convention error, as
xarray does. Also name unnamed pd.Index entries on a copy so an entry's
.name matches its dim consistently across forms.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collapse TestCoordsToDict's per-section single-assertion tests into a few form-parameterized tests (one table for naming forms, one for the skipped unlabeled forms, one folding the five scattered "entry raises TypeError" cases), dropping the # -- section comments in favour of the parametrization. Move the end-to-end add_variables checks out of TestCoordsToDict into a new TestAddVariablesCoords class, since they exercise the model wiring rather than _coords_to_dict, and fold them into a single parametrized test. No behaviour change and no coverage lost. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The fallback message in _coords_to_dict still advertised `tuple` as a valid "unnamed sequence", which contradicts the (dim_name, values) tuple handling — a user following it would hit the tuple-convention error instead. Name the accurate forms (pd.Index / unlabeled list-range-ndarray / (dim_name, values) tuple). Adds a scalar-value tuple case to the parametrized raises table. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
FBumann
changed the title
FabianHofmann
approved these changes
Jun 5, 2026
Collaborator
FabianHofmann
left a comment
FabianHofmann
left a comment
There was a problem hiding this comment.
thanks for the follow-up and the tidying of the tests
Comment on lines
+578
to
+588
| ([("x", [0, 1, 2])], None, ("x",)), | ||
| ([pd.Index([0, 1, 2], name="x")], None, ("x",)), | ||
| ([pd.Index([0, 1, 2])], ["x"], ("x",)), | ||
| ([[0, 1, 2]], ["x"], ("x",)), | ||
| ([range(3)], ["x"], ("x",)), | ||
| ([np.array([0, 1, 2])], ["x"], ("x",)), | ||
| ([[0, 1, 2]], None, ("dim_0",)), | ||
| ([range(3)], None, ("dim_0",)), | ||
| ([np.array([0, 1, 2])], None, ("dim_0",)), | ||
| ([pd.Index([0, 1, 2])], None, ("dim_0",)), | ||
| ([("origin", ["a", "b"]), ("dest", ["x", "y"])], None, ("origin", "dest")), |
Collaborator
There was a problem hiding this comment.
crazy how many combinations this supports :D
Collaborator
Author
There was a problem hiding this comment.
Yes...
But it always has. Its now just really obvious!
FBumann
added a commit
to fluxopt/linopy
that referenced
this pull request
Jun 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Mandatory to pull this in before next release.
Fixes a bug which was introduced by the alignment refactor. tuple coords were treated differently. Its now consistent with v0.7.0 and xarray.
And additionally improving and reorganizing test coverage
Note
The following content was generated by AI.
Details
What
_coords_to_dicttreated a tuple coords entry as a bare value sequence (like a list), which broke the standard xarray formcoords=[("origin", origins)]: a scalar bound against such coords raised an inscrutable numpy "setting an array element with a sequence … inhomogeneous shape" error.v0.7.0 delegated coords to xarray and supported this form; the alignment refactor in #742 (unreleased) regressed it and pinned the value-sequence reading in tests.
Fix
Restore xarray's convention in
_coords_to_dict:(dims, data[, attrs])— its first element names the dimension.list(orrange/ndarray).(dim_name, values)error, mirroring xarray's own rejection.Unnamed
pd.Indexentries are now named on a copy so an entry's.namematches its dim consistently across forms (no caller mutation).Tests
(name, values)coverage (parse-level + end-to-end), the multi-tuple scalar-bound regression, and tuple edge cases.test_alignment.py's coords tests into form-parameterized tables and split the end-to-endadd_variableschecks into their own class.Full test suite passes (3787 passed, 29 skipped).
Docs
Corrected the unreleased release note to state the xarray
(dim_name, values)convention (the prior "bare-tuple entries behave like lists" wording described the regressed behaviour).