Support compute=False from DataTree.to_netcdf #10625
Conversation
github-actions
bot
added
topic-backends
topic-zarr
| writer = ArrayWriter() | ||
|
|
||
| # TODO: figure out how to properly handle unlimited_dims | ||
| try: |
There was a problem hiding this comment.
It'd be nice to refactor this to a common function used in both the netCDF and the Zarr writer. Do you see a way to do that? At first glance the "validate region / encoding" bit seems to make this hard.
If there is no easy way to do that, can you please add a comment to both functions to remind future contributors to keep the logic in sync?
There was a problem hiding this comment.
Yeah, I think this would be tricky.
I'm not sure a comment makes sense here -- there's no intrinsic reason why the implementations need to match, although hopefully this suggestion would be somewhat obvious? There are also unit tests, of course.
There was a problem hiding this comment.
I think this separate datatree_io.py file has come to the end of it's usefulness. In a follow-up I can just merge it into the respective backends.
There was a problem hiding this comment.
Definitely agreed! In the long term, we might even implement Dataset IO in terms of DataTree IO. This would let us avoid redundant code paths, similar to how we currently implement many DataArray operations in terms of Dataset.
|
This is ready for a final review now that tests are passing. |
|
Just a heads up, I am going to submit this shortly so I can start iterating on follow-ups |
Split out of #10624
This PR combines adds support for
compute=FalsefromDataTree.to_netcdfandto_zarr. To do so, I refactored the internals of these methods to use Xarray's lower level data store interface directly, rather than callingDatasetmethods.whats-new.rst