-
-
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
Merged
dcherian
merged 12 commits into
pydata:main
from
benbovy:add-coordinate-from-xindex-method
Feb 12, 2025
Merged
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
734bffb
add Coordinates.from_xindex method
benbovy 95938b0
doc: refactor Coordinates API reference
benbovy e110790
add tests
benbovy 50d30fb
update what's new
benbovy ad757fa
fix doc build?
benbovy 288c934
docstring tweaks
benbovy 8113302
doc (api): add missing Coordinates.sizes property
benbovy 07e6a37
update what's new (documentation)
benbovy ad78599
improve docstrings on Coordinates creation
benbovy 7aff9ce
Merge branch 'main' into add-coordinate-from-xindex-method
benbovy 0fbbd92
doc: move what's new entries after last release
benbovy 86ceb91
Merge branch 'main' into add-coordinate-from-xindex-method
dcherian File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back 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.
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, whereasfrom_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):
writing
is equivalent but safer and more convenient than writing
Alternatively to
Coordinates.from_xindex()
we could provide more specific constructors likeCoordinates.from_transform()
(alongside the already existingCoordinates.from_pandas_multiindex()
), butCoordinates.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!