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 accessor convert to base units #2422

Merged
merged 2 commits into from
Apr 11, 2022

Conversation

kgoebber
Copy link
Collaborator

Description Of Changes

This PR adds to the metpy xarray accessor to have a convert_to_base_units() function, which will allow for the simplification of complex units following calculations using quantify xarray DataArrays.

Added a test and a simple example to the xarray tutorial.

Checklist

@kgoebber kgoebber requested review from a team and jthielen as code owners April 10, 2022 02:51
@kgoebber kgoebber requested review from dopplershift and removed request for a team April 10, 2022 02:51
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.

Looks good to me. @jthielen how do you feel about the spelling of this? It doesn't look like pint-xarray has a corresponding method for us to match.

@dopplershift dopplershift added Type: Feature New functionality Area: Xarray Pertains to xarray integration labels Apr 11, 2022
@dopplershift
Copy link
Member

We can ignore Codacy here. I keep trying to get it to set the "issue" as ignored, but every analysis keeps flagging it.

@jthielen
Copy link
Collaborator

@jthielen how do you feel about the spelling of this? It doesn't look like pint-xarray has a corresponding method for us to match.

Yep, I noticed that as well, though I would suspect that if it ever did get added it would be consistent with the underlying Pint method of to_base_units. For now, having convert_to_base_units as this PR has now seems best for consistency with our other existing methods. But, if we did want to make the switch at this point to having more Pint-like to, etc. as preferred, then I guess we could just go with to_base_units and avoid introducing another aliased method.

Copy link
Collaborator

@jthielen jthielen left a comment

Choose a reason for hiding this comment

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

Looks good to me (pending decision on method name)!

@dopplershift
Copy link
Member

dopplershift commented Apr 11, 2022

Until there's actually something to be consistent with elsewhere, we shouldn't invent something that may be consistent with it. So given that the current naming fits with our existing API, that's what seems to be best for our users.

@dopplershift dopplershift merged commit 096be48 into Unidata:main Apr 11, 2022
@github-actions github-actions bot added this to the May 2022 milestone Apr 11, 2022
@dopplershift dopplershift modified the milestones: May 2022, July 2022 Jun 2, 2022
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
Status: Done
Development

Successfully merging this pull request may close these issues.

add xarray accessor interface for to_base_units
3 participants