Skip to content
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 unit- and axis-aware selection to the metpy accessors #971

Merged
merged 1 commit into from Dec 18, 2018
Merged

Add unit- and axis-aware selection to the metpy accessors #971

merged 1 commit into from Dec 18, 2018

Conversation

jthielen
Copy link
Collaborator

@jthielen jthielen commented Nov 20, 2018

This PR adds .loc[] and .sel() to both the DataArray and Dataset accessors to allow for unit-aware indexing/selection. Also, for DataArrays (for which axis types are uniquely determined), I also added in selection via the axis name ('time', 'vertical', etc.).

As an example, now something like this can be done:

temperature.metpy.sel(vertical=500 * units.hPa)

Closes #970.

Also, I reused some code from xarray with modification for this (the loc indexers). What is the proper way to cite this in the code given their license?

@dopplershift
Copy link
Member

Instead of copying in xarray code, can we just delegate to the loc attribute? It'd be nice to keep the licensing simpler, plus it would lessen the burden of updating with future changes to xarray.

@jthielen jthielen changed the title Add unit- and axis-aware selection to the metpy accessors WIP: Add unit- and axis-aware selection to the metpy accessors Nov 27, 2018
@jthielen
Copy link
Collaborator Author

While I couldn't think of a way to not reuse the _LocIndexer class approach, I've changed the internals of the classes to delegate more to the base .loc attributes rather than parallel the xarray code in going to the .sel method. If this approach is better, I can squash the commits and get rid of the old copy-and-modify approach. Otherwise, let me know what else I could try!

@dopplershift
Copy link
Member

I still need to give this a more detailed review, but I definitely prefer this to the copy-and-modify version. Thanks!

@dopplershift dopplershift added this to the 0.10 milestone Dec 7, 2018
@dopplershift dopplershift self-assigned this Dec 17, 2018
dopplershift
dopplershift previously approved these changes Dec 17, 2018
Copy link
Member

@dopplershift dopplershift left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work. The xarray handling code is getting a bit big, but not too unwieldy...yet. We may yet be broken by xarray because we're using things they consider outside their public API, but that's ok. Small price to pay for this nice bit of functionality.

@dopplershift dopplershift changed the title WIP: Add unit- and axis-aware selection to the metpy accessors Add unit- and axis-aware selection to the metpy accessors Dec 17, 2018
@dopplershift
Copy link
Member

If you can rebase this to merge cleanly, I think we're good to go here.

This commit adds the .loc indexer and .sel method to both the DataArray
and Dataset metpy accessors to allow for unit-aware indexing/selection.
For DataArrays (for which axis types are uniquely determined), this also
allows selection via the axis name ('time', 'vertical', etc.) instead of
the coordinate name.

Also, adds this new unit-aware selection to the xarray tutorial.
Copy link
Contributor

@jrleeman jrleeman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rebased and clean!

@jrleeman jrleeman merged commit 113de63 into Unidata:master Dec 18, 2018
@dopplershift dopplershift added Area: Xarray Pertains to xarray integration Type: Feature New functionality labels Jun 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Xarray Pertains to xarray integration Type: Feature New functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unit-Aware DataArray (and Dataset?) Selection
3 participants