-
Notifications
You must be signed in to change notification settings - Fork 415
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
Refactor xarray grid deltas calculation to handle axis order flexibly #1260
Conversation
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.
Looks good, just a minor question.
src/metpy/calc/tools.py
Outdated
@@ -787,35 +800,48 @@ def lat_lon_grid_deltas(longitude, latitude, **kwargs): | |||
longitude = np.asarray(longitude) | |||
latitude = np.asarray(latitude) | |||
|
|||
# Determine dimension order for offset slicing | |||
take_y = make_take(latitude.ndim, kwargs.pop('y_dim', -2)) |
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.
Think this and x_dim
could become a kwarg-only argument?
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.
Sure. Looking back at this, I'm not sure why I did it the way it is now.
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.
Waiting on CI
Description Of Changes
As noted in #1249 (comment), the grid deltas calculation does not handle dimension/axis order that are not (..., y, x), whereas a goal of MetPy's xarray integration is to be able to flexibly handle dimension order. This PR refactors
lat_lon_grid_deltas
andgrid_deltas_from_dataarray
to now flexibly handle dimension order. There were also some accompanying changes:find_axis_number
helper (similar tofind_axis_name
) has been added to the DataArray accessorkind
argument togrid_deltas_from_dataarray
, which has options "actual" and "nominal" to pave the way for Generalize vorticity (and related functions) to allow for map projections #893bump minimum xarray version to 0.13.0, which is needed for Pint Quantities to be inside xarray objects(doesn't quite work with Pint v0.9)axis
/axes
argument documentation to fully resolve Documentation for axes in gradient calc #1249Checklist