-
Notifications
You must be signed in to change notification settings - Fork 416
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
Error in absolute_momentum (bug in normal_component?) #1948
Comments
The likely problem is that your
I also think we should adjust our documentation to make it clear that the data in the xarray DataArray needs to be a pint Quantity. |
@23ccozad This is intended to work without having Pint Quantities inside your DataArrays, as MetPy's xarray accessor is supposed to flexibly handle both Quantity-in-DataArray and unit-on-attribute cases for input data. @ShunsukeHoshino Would you be able to report what the |
@jthielen Good to know that this should work using the units attribute in the DataArray. I'm guessing that @ShunsukeHoshino has appropriate units on the original dataset and interpolated cross-section dataset and that maybe the units attribute is being lost in |
@23ccozad That's a good possibility as well, since the multiplications in MetPy/src/metpy/calc/cross_sections.py Line 295 in edb9828
to norm_wind = normal_component(u.metpy.convert_units('m/s'), v.metpy.convert_units('m/s'), index=index) may solve that kind of issue. |
@jthielen I was originally thinking about putting the units back on the DataArray by adding
following MetPy/src/metpy/calc/cross_sections.py Line 205 in 15d0c80
|
Well, I had a minor preference for @23ccozad's solution since it mirrored the handling of However, @jthielen's suggestion is more robust to the potential case of u and v components having different units (however unlikely that is). So I think that's the way we should handle it. |
@jthielen What's the rationale for explicitly converting to "m/s" rather than just using |
@dopplershift I don't think there was a true rationale, just that it was the easiest-to-apply carry-over from the original implementation where units had to be stripped. Only reason to keep it now is that momentum in another speed unit like Also, fun little note on the side: unit attribute handling was part of the original implementation, but I believe it got cleaned away somewhere in the "great xarray refactor." |
The refactor making a bunch of that superfluous is what I was thinking too. I think if we want to return momentum in "m/s" that's fine, but the code might be better structured to convert as the last step, making the purpose explicit. |
Thank you for suggestion.
This works well. Thank you very much! |
Glad you were able to find a solution @ShunsukeHoshino. Thanks for bringing up this issue so that |
I want to get absolute momentum with xarray.DataArray like:
But DimensionalityError occurs:
So I try:
nc = normal_component(cross['u'], cross['v'])
but result (
nc
) has no attribute.How can I use absolute_momentum ?
My environment is follows:
The text was updated successfully, but these errors were encountered: