-
Notifications
You must be signed in to change notification settings - Fork 413
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
Floating comparisons #497
Floating comparisons #497
Conversation
3ecf9aa
to
67230b1
Compare
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.
Minor nits
metpy/calc/tools.py
Outdated
r"""Compare values for greater or close to boolean masks. | ||
|
||
Returns a boolean mask for values greater than or equal to a target within a specified | ||
absolute or relative tolerance (as in np.isclose). |
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.
How about :func:\
numpy.isclose`` so we can get cross doc linking to work?
metpy/calc/tools.py
Outdated
r"""Compare values for less or close to boolean masks. | ||
|
||
Returns a boolean mask for values less than or equal to a target within a specified | ||
absolute or relative tolerance (as in np.isclose). |
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.
Same
There were edge cases where greater/less than or equal to would suffer from precision issues and drop an extra point in the sounding. Modify them to use our approximate comparisons.
The initial way this bug was found was by not convering the output of parcel_profile in the test. It is removed now and tests that those comparisons are working.
67230b1
to
b836862
Compare
Adds functions to compare floating point values in a greater/less than or near to sense. I also removed the conversion in the CAPE/CIN test that helped @tjwixtrom find the bug initially. Closes #488