-
Notifications
You must be signed in to change notification settings - Fork 43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WIP] add DataTree.to_netcdf #26
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great, thanks @jhamman. The only thing I can think of to add would be some kind of test that netcdf -> xarray.DataTree -> netcdf
roundtripping works. That would probably require committing a small data file though.
(This can of course wait for another PR though)
Co-authored-by: Tom Nicholas <35968931+TomNicholas@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is one last problem with dataless nodes - see suggestion.
(Also this would have been easier if I had written an assert_tree_equal
function)
@TomNicholas - I think this is ready to go. I've special cased the empty node in read and write now. Easy to update in the future if the data model evolves a bit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent @jhamman - thank you!
I might make minor changes (e.g. call a dedicated assert_tree_equal
function) later, but this looks solid for now :)
* first attempt at to_netcdf * lint * add test for roundtrip and support empty nodes * Apply suggestions from code review Co-authored-by: Tom Nicholas <35968931+TomNicholas@users.noreply.github.com> * update roundtrip test, improves empty node handling in IO Co-authored-by: Tom Nicholas <35968931+TomNicholas@users.noreply.github.com>
This addresses #16.
I've filled in
_datatree_to_netcdf
to support writing aDataTree
to aNETCDF4
file. I have a few questions for @TomNicholas that I'll highlight with comments on the changes.