-
-
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
Add Coordinates.from_xindex
method (+ refactor API doc)
#10000
base: main
Are you sure you want to change the base?
Conversation
Make it more consistent with ``Dataset``, ``DataArray`` and ``DataTree``. The ``Coordinates`` class is 2nd order compared to the former ones, but it is public API and useful (for creating coordinates from indexes and merging coordinates together) so it deserves its own (expanded) section + summary tables in the API reference doc.
Ticket number #10000, nice! 🎉 |
@@ -352,6 +353,35 @@ def _construct_direct( | |||
) | |||
return obj | |||
|
|||
@classmethod | |||
def from_xindex(cls, index: Index) -> Self: |
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.
Seems a little inconsistent that the signature of this method only accepts one index, when the __init__
accepts multiple?
I'm also a bit unclear why you need to pass a Mapping
from str names to indexes to __init__
but this method apparently doesn't require names?
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.
Ah yes I agree it is confusing and I'll clarify this.
__init__
is the low-level(-ish) constructor where we can pass any arbitrary mapping of coordinate variables (and optionally a mapping of Xarray indexes) that are just passed to a direct constructor with only little safety checks, whereas from_xindex
creates a new Coordinates object from exactly one Xarray index via creating the variables and indexes mappings from the index object.
For example with a custom coordinate transform index (#9543):
class CustomIndex(CoordinateTransformIndex):
...
index = CustomIndex(...)
writing
coords = Coordinates.from_xindex(index)
is equivalent but safer and more convenient than writing
vars = index.create_variables()
indexes = {name: index for name in vars}
coords = Coordinates(coords=vars, indexes=indexes)
Alternatively to Coordinates.from_xindex()
we could provide more specific constructors like Coordinates.from_transform()
(alongside the already existing Coordinates.from_pandas_multiindex()
), but Coordinates.from_xindex()
is more generic and future-proof, i.e., it will just work with any custom index that can be created from anything other than pre-existing Xarray coordinates.
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.
Okay thank you for the explanation!
This PR adds a new
Coordinates.from_xindex()
method as convenience for creating a newCoordinates
object directly from an existingxarray.Index
object. This will be useful, e.g., for #9543.I picked the name
from_xindex()
as it is consistent with other API likeDataset.set_xindex()
andDataset.xindexes
.I also refactored the API reference documentation so
Coordinates
has its own expanded section with subsections similar to the ones of the Dataset, DataArray and DataTree sections. The Coordinates class likely won't be used as much as the other ones, but since it has some utility and it is public API I thought it'd be worth better expose it.whats-new.rst
api.rst