Repository navigation
fix: keep auxiliary coordinates of dense containers in netcdf round trip - #1024
Open
MaykThewessen wants to merge 3 commits into
Open
MaykThewessen wants to merge 3 commits into
MaykThewessen wants to merge 3 commits into
Conversation
MaykThewessen
added a commit
to MaykThewessen/linopy
that referenced
this pull request
Oct 8, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Merging this PR will not alter performance
Comparing Footnotes
|
This was referenced Oct 8, 2026
read_netcdf dropped every non-index coordinate except MultiIndex levels to keep out the scalar coordinates of other containers that xarray attaches when selecting one container's data variables. That also dropped the auxiliary coordinates of dense variables, constraints, expressions and parameters, such as the levels on a flat dimension that v1 recommends instead of a MultiIndex. Only drop scalar coordinates owned by another container, matched on the longest prefix since names may contain dashes. Coordinates with dimensions only attach to the container whose dimensions they share. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
FabianHofmann
force-pushed
the
fix/netcdf-keep-aux-coords
branch
from
October 20, 2026 07:41
4b5234f to
8901a8f
Compare
FabianHofmann
pushed a commit
to MaykThewessen/linopy
that referenced
this pull request
Oct 20, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ux-coords # Conflicts: # doc/release_notes.rst
This branch has not been deployed
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
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.
Note
AI-assisted. Claude wrote the code and tests, and ran the checks listed below. I reviewed the diff.
Changes proposed in this Pull Request
read_netcdfdropped the auxiliary (non-index) coordinates of every dense variable, constraint, named expression and ofModel.parameters, also for plain naive values. With v1 recommending a flat dimension with levels as auxiliary coordinates instead of aMultiIndex, a round trip through netcdf (and sosolve(remote=...)) loses those levels. Frozen constraints already kept theirs.The drop in
get_prefixwas there because selecting one container's data variables from the merged dataset attaches the scalar coordinates of all other containers (e.g.constraints-c0-snapshotafterx.sel(snapshot=0) >= 1). Coordinates with dimensions can only attach to the container whose dimensions they share, so the read now drops only scalar coordinates owned by another container. Ownership uses the longest-prefixownerslookup from #1025, so dash-named containers resolve correctly. This also restores a container's own scalar coordinates, and makes the separateMultiIndexlevel bookkeeping inget_prefixunnecessary.Overlap with #1021: both touch
read_netcdf/get_prefixinlinopy/io.py. The code merges without conflicts; only adjacent release note bullets conflict. The tests of both pass on the merge.Checklist
AGENTS.md).doc.doc/release_notes.rstof the upcoming release is included.What was checked (AI-generated)
test_model_to_netcdf_aux_coordsintest/test_io.py: a flatsnapshotdimension withtime(datetime) andperiodaux coords on a variable, a dense constraint, a named expression and a parameter; a constraintc-firstfromx.sel(snapshot=0)whose scalar coords would leak intocunder a plain prefix match; and a dash-named parametermy-paramwith scalar coords. Assertsassert_model_equal, which compares coordinates. On 0578f9c: fails with master'sio.pyand passes on the branch, under both legacy and v1 semantics.MultiIndexlevels. Probing a file shows the coords xarray attaches on selection are the scalar ones of other containers.test/test_io.pyandtest/test_csr.pyon 0578f9c: 1033 passed, 941 skipped (solver-dependent).ruff checkandruff format --checkclean.mypyreports no errors in the changed files (2 existing ones insolvers.pyandremote/oetc.py).fix/netcdf-tz-aware-coordsat 0333f66):linopy/io.pymerges cleanly,doc/release_notes.rsthas an adjacent-bullet conflict. On the merge,test/test_io.pyplustest/test_csr.py: 1051 passed.🤖 Generated with Claude Code