-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Fix for all NANs in contour #25334
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
Fix for all NANs in contour #25334
Conversation
How does the original issue behave with this change? (Maybe you can add the smoke test back?) |
Do you also want to change the |
done @greglucas, PTAL |
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 to me!
One minor nit that you can take or leave.
lib/matplotlib/tests/test_contour.py
Outdated
|
||
def test_all_nan(): | ||
x = np.array([[np.nan, np.nan], [np.nan, np.nan]]) | ||
assert_array_almost_equal(plt.contour(x).levels.tolist(), |
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.
I'm pretty sure numpy's testing will compare the list and levels array for you, so I don't think you need to convert it to a list here unless you were doing the bare assert like above.
assert_array_almost_equal(plt.contour(x).levels.tolist(), | |
assert_array_almost_equal(plt.contour(x).levels, |
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.
Yeah right, removed to_list
conversion. Thanks.
Thanks @xtanion and congratulations on your first merged PR in Matplotlib! Hope to see you around! |
PR Summary
As discussed in #25330, we're removing the typecase to float with
z.min().astype(float)
.Closes #14124.