-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Datatree alignment docs #9501
Datatree alignment docs #9501
Conversation
ds_weekly.sizes | ||
ds_monthly.sizes | ||
|
||
We cannot store these non-alignable variables on a single :py:class:`~xarray.Dataset` object, because they do not exactly align: |
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.
I guess it would be more correct to say that we cannot store them unchanged.
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.
As someone who doesn't use DataTree yet, this is v nice & clear!
…las/xarray into datatree_alignment_docs
Co-authored-by: Maximilian Roos <[email protected]>
…las/xarray into datatree_alignment_docs
Co-authored-by: Stephan Hoyer <[email protected]>
Data Alignment | ||
~~~~~~~~~~~~~~ |
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.
TODO: add comment about open_groups
being useful if your data doesn't align
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 the only note I have on open_groups, it probably deserves more. https://github.com/pydata/xarray/blob/main/doc/getting-started-guide/quick-overview.rst?plain=1#L284
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.
gonna prioritize merging this and improving documentation for open_groups
later
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.
What's here looks good. I think my main question is about the things that are left as TODOs. Do they need to be done in this PR, or could/should they be tracked under a new issue, so this PR can be merged?
doc/user-guide/hierarchical-data.rst
Outdated
|
||
.. note:: | ||
If you were a previous user of the prototype `xarray-contrib/datatree <https://github.com/xarray-contrib/datatree>`_ package, this is different from what you're used to! | ||
In that package the data model was that nodes actually were completely unrelated. The data model is now slightly stricter. |
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.
Possible nit (feel free to ignore): Would it be clearer to say the information (or specifically Dataset
object) contained on each node was unrelated?
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.
Yeah that's a great point, it would definitely be both more clear and more accurate to say that instead.
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.
clarified in 9b8fc9b
.. note:: | ||
This requirement of aligned dimensions is similar to netCDF's concept of `inherited dimensions <https://www.unidata.ucar.edu/software/netcdf/workshops/2007/groups-types/Introduction.html>`_, as in netCDF-4 files dimensions are `visible to all child groups <https://docs.unidata.ucar.edu/netcdf-c/current/groups.html>`_. | ||
|
||
This alignment check is performed up through the tree, all the way to the root, and so is therefore equivalent to requiring that this :py:func:`~xarray.align` command succeeds: |
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.
Before getting to this statement, I had added a comment saying we should make it clear that the alignment check ensures alignment with all ancestors, not just the immediate parent. But this covers it nicely!
doc/user-guide/hierarchical-data.rst
Outdated
Coordinate Inheritance | ||
~~~~~~~~~~~~~~~~~~~~~~ | ||
|
||
Notice that in the trees we constructed above (LINK OR DISPLAY AGAIN?) there is some redundancy - the ``lat`` and ``lon`` variables appear in each sibling group, but are identical across the groups. |
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.
LINK OR DISPLAY AGAIN
I'm tempted to say display it again after this paragraph.
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.
done in d2918bb
doc/user-guide/hierarchical-data.rst
Outdated
We can override inherited coordinates with newly-defined ones, as long as those newly-defined coordinates also align with the parent nodes. | ||
|
||
EXAMPLE OF THIS? WOULD IT MAKE MORE SENSE TO USE DIFFERENT DATA TO DEMONSTRATE THIS? | ||
|
||
EXAMPLE OF INHERITING FROM A GRANDPARENT? | ||
|
||
EXPLAIN DEDUPLICATION? |
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.
Is the plan to include these points in this PR, or merge what is here (maybe with this commented out) and then add more content later?
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.
I was going to do it in this PR, but given that everyone seems to be happy with what's here already, and this is a natural break point, perhaps I will just merge this for now.
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.
A follow-up issue could be to "document the subtleties of coordinate inheritance"
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.
(I had hoped to add these bits before you reviewed it @owenlittlejohns )
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.
Removed that content for use in a future PR in 6cab6f8
…las/xarray into datatree_alignment_docs
Adds a dedicated section on datatree alignment and coordinate inheritance to the Hierarchical Data page. Intended to complement what's already been added to the Data Structures page by @flamingbear, in a more narrative form. I've also tried to separate out the concept of alignment from coordinate inheritance.
This shouldn't really be merged until a few other things are fixed, particularly #9499.
Tests addedUser visible changes (including notable bug fixes) are documented inwhats-new.rst
New functions/methods are listed inapi.rst
cc @shoyer @eni-awowale @owenlittlejohns