Skip to content

fix: netcdf export corrupting dynamic attributes when directly assigning df#1522

Merged
lkstrp merged 4 commits intoPyPSA:masterfrom
lkstrp:fix-1521
Jan 13, 2026
Merged

fix: netcdf export corrupting dynamic attributes when directly assigning df#1522
lkstrp merged 4 commits intoPyPSA:masterfrom
lkstrp:fix-1521

Conversation

@lkstrp
Copy link
Copy Markdown
Member

@lkstrp lkstrp commented Jan 13, 2026

Closes #1521

This should also be catched and sanitized upfront with #1487. Also cleaning up how we store to netcdf would help

@lkstrp lkstrp requested a review from FabianHofmann January 13, 2026 11:23
Copy link
Copy Markdown
Contributor

@FabianHofmann FabianHofmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wonderful!

assert status == "ok"


def test_1522(tmp_path):
Copy link
Copy Markdown
Contributor

@FabianHofmann FabianHofmann Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not a huge fan of numbered tests, is that good practice? mostly asking for future purposes

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have strong feelings. Certainly good to keep the reference to the issue, but could be in docstring.

Copy link
Copy Markdown
Member Author

@lkstrp lkstrp Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah sorry, that was a quick merge. I guess it's just a matter of style and builds on what we already have. When a test is specifically designed to test a bug, it's better to add it here rather than bloating the normal tests. But we don't need to name them test_xx, so let's keep those in test_bugs.py, but without numbered test names in future

@lkstrp lkstrp merged commit 9f18a85 into PyPSA:master Jan 13, 2026
25 of 26 checks passed
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.

marginal costs being assigned to incorrect component type when loading from NetCDF

3 participants