Implement DataTree.isel and DataTree.sel #9588
Conversation
TomNicholas
added
the
topic-DataTree
xarray/core/datatree.py
Outdated
| for k in node_indexers: | ||
| if k not in node._node_coord_variables and k in node_result.coords: | ||
| del node_result.coords[k] |
There was a problem hiding this comment.
So this is basically saying "if the result of indexing would cause an inherited coordinate to be duplicated, then remove that duplicated coordinate from the result"?
Doesn't this imply an inefficiency, in that we are potentially doing the work to index coordinates that we're only going to drop afterwards?
There was a problem hiding this comment.
I added a comment to explain. It is indeed a small source of inefficiency.
xarray/core/datatree.py
Outdated
| # TODO: reimplement in terms of map_index_queries(), to avoid | ||
| # redundant index look-ups on child nodes |
There was a problem hiding this comment.
Is this related to my comment above or a separate issue?
| expected = DataTree.from_dict( | ||
| { | ||
| "/": xr.Dataset(coords={"x": 2}), | ||
| "/child": xr.Dataset({"foo": 4}), | ||
| } | ||
| ) | ||
| actual = tree.isel(x=-1) | ||
| assert_equal(actual, expected) |
There was a problem hiding this comment.
When this passes the x coordinate gets duplicated doesn't it? As the result is a non-indexed scalar. Does that mean this would have failed if you had used the definition of assert_identical from #9473 to check for duplication?
There was a problem hiding this comment.
IIUC this should also pass
actual = tree.isel(x=[0])
assert_identical(actual, expected)because the result will have inherited instead of duplicated coordinates instead.
There was a problem hiding this comment.
I wrote special logic for removing duplicated scalar coordinates to handle just this!
There is no longer a need to implement a different version of inheritance for assert_identical than for assert_equal, because we no longer allow any cases in the DataTree data model where inheritance is ambiguous:
- Deduplication of coordinates with an index removes them from child nodes.
- Other coordinates are no longer inherited.
These methods handle dimensions that are not missing on some nodes properly, as discussed in #8949
Adding additional indexing methods (e.g.,
head,tail,thin,interp,reindex), should be pretty easy to do in this style.api.rst