-
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
Add unit-awareness to azimuth_range_to_lat_lon function #1977
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.
Can you add a with pytest.warns(UserWarning, match='not a Pint-Quantity'):
to the call in test_azimuth_range_to_lat_lon
? That way we check that the warning is actually issued and we silence the warning when running tests.
src/metpy/calc/tools.py
Outdated
@@ -887,6 +888,16 @@ def azimuth_range_to_lat_lon(azimuths, ranges, center_lon, center_lat, geod=None | |||
else: | |||
g = geod | |||
|
|||
try: # convert range units to meters | |||
ranges = ranges.to('meters').m |
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.
Been trying to use this more:
ranges = ranges.to('meters').m | |
ranges = ranges.m_as('meters') |
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.
Got it.
src/metpy/calc/tools.py
Outdated
except AttributeError: # no units associated | ||
warnings.warn('Range values are not a Pint-Quantity, assuming values are in meters.') | ||
try: # convert azimuth units to degrees | ||
azimuths = azimuths.to('degrees').m |
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.
azimuths = azimuths.to('degrees').m | |
azimuths = azimuths.m_as('degrees') |
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.
Got it.
Description Of Changes
This PR resolves #1970 by adding some unit handling and issue a warning if ranges and/or azimuths are supplied without units to alert the user to the assumed behavior. Test slightly updated to account for different use cases of units or no units attached to arrays.
Checklist