Skip to content

fix: keep auxiliary coordinates of dense containers in netcdf round trip - #1024

Open
MaykThewessen wants to merge 3 commits into
PyPSA:masterfrom
MaykThewessen:fix/netcdf-keep-aux-coords
Open

MaykThewessen wants to merge 3 commits into
PyPSA:masterfrom
MaykThewessen:fix/netcdf-keep-aux-coords

Conversation

@MaykThewessen

@MaykThewessen MaykThewessen commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

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_netcdf dropped the auxiliary (non-index) coordinates of every dense variable, constraint, named expression and of Model.parameters, also for plain naive values. With v1 recommending a flat dimension with levels as auxiliary coordinates instead of a MultiIndex, a round trip through netcdf (and so solve(remote=...)) loses those levels. Frozen constraints already kept theirs.

The drop in get_prefix was there because selecting one container's data variables from the merged dataset attaches the scalar coordinates of all other containers (e.g. constraints-c0-snapshot after x.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-prefix owners lookup from #1025, so dash-named containers resolve correctly. This also restores a container's own scalar coordinates, and makes the separate MultiIndex level bookkeeping in get_prefix unnecessary.

Overlap with #1021: both touch read_netcdf/get_prefix in linopy/io.py. The code merges without conflicts; only adjacent release note bullets conflict. The tests of both pass on the merge.

Checklist

  • AI-generated content is marked (see AGENTS.md).
  • Code changes are sufficiently documented; i.e. new functions contain docstrings and further explanations may be given in doc.
  • Unit tests for new features were added (if applicable).
  • 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.
What was checked (AI-generated)
  • test_model_to_netcdf_aux_coords in test/test_io.py: a flat snapshot dimension with time (datetime) and period aux coords on a variable, a dense constraint, a named expression and a parameter; a constraint c-first from x.sel(snapshot=0) whose scalar coords would leak into c under a plain prefix match; and a dash-named parameter my-param with scalar coords. Asserts assert_model_equal, which compares coordinates. On 0578f9c: fails with master's io.py and passes on the branch, under both legacy and v1 semantics.
  • Mutation checks on the test (at 8901a8f): matching the shortest instead of the longest prefix fails it, and keeping all coordinates (no drop) fails it.
  • History: the drop came in with 4b16c1a (2023, "fix import of multiindexed coord names"), which kept only MultiIndex levels. Probing a file shows the coords xarray attaches on selection are the scalar ones of other containers.
  • test/test_io.py and test/test_csr.py on 0578f9c: 1033 passed, 941 skipped (solver-dependent).
  • ruff check and ruff format --check clean. mypy reports no errors in the changed files (2 existing ones in solvers.py and remote/oetc.py).
  • Merge with fix: write timezone-aware coordinates in Model.to_netcdf #1021 (fix/netcdf-tz-aware-coords at 0333f66): linopy/io.py merges cleanly, doc/release_notes.rst has an adjacent-bullet conflict. On the merge, test/test_io.py plus test/test_csr.py: 1051 passed.

🤖 Generated with Claude Code

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>
@codspeed

codspeed Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 181 untouched benchmarks
⏩ 181 skipped benchmarks1


Comparing MaykThewessen:fix/netcdf-keep-aux-coords (0578f9c) with master (4117547)2

Open in CodSpeed

Footnotes

  1. 181 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. ↩

  2. No successful run was found on master (09d4fde) during the generation of this report, so 4117547 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

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
FabianHofmann force-pushed the fix/netcdf-keep-aux-coords branch from 4b5234f to 8901a8f Compare October 20, 2026 07:41
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>

This branch has not been deployed

No deployments
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.

2 participants