-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
sanitize norm extrema to be floats #10721
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
Conversation
What type of data was failing? Does the test cover the failure mode? This seems reasonable to me, but it'd be good to make sure we know what failed and that we have a test to gaurd for future.. |
If you set
The changes I made to |
If I run the following on master and import numpy as np
import matplotlib
import matplotlib.colors as mcolors
class MyArray(np.ndarray):
def __isub__(self, other):
raise RuntimeError
def __add__(self, other):
raise RuntimeError
data = np.arange(-10, 10, 1, dtype=float)
mydata = data.view(MyArray)
for norm in [mcolors.Normalize(vmin=mydata.min(), vmax=mydata.max()),
mcolors.SymLogNorm(3, vmin=mydata.min(), vmax=mydata.max())]:
print(np.all(norm(mydata) == norm(data))) |
@jklymak thanks! It looks like you actually have to use the |
Ha, given that I just got a slap on the wrist for my test writing, please no apologies! We did change some image normalization stuff recently, hence I wanted to make sure that we didn't miss something. i.e. #10613 worse, that one didn't get in for the RC, but was in response to a bug report after the RC, so its possible we (ahem, I) fixed one thing and broke another. |
@jklymak updated the test. Here's a script that I verified should fail on 2.2.0:
|
Yep, that was #10613. Sorry about that. Your fix looks reasonable, as does some test along the lines of the above. |
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.
Thanks @ngoldbaum !
Backport PR #10721 on branch v2.2.x
PR Summary
Fix regression from 2.1.2 by ensuring norm extrema are always floats.
PR Checklist
PR Description
We had a report from a yt user complaining about saving plots to disk raising errors in some cases:
https://mail.python.org/mm3/archives/list/yt-users@python.org/thread/24OJRJVPK47RMG7XLF7J7YP4JQH24MG7/
I was able to reproduce this issue on matplotlib 2.2.0 but not on matplotlib 2.1.2 and earlier, so this is a regression.
I believe this behavior was unintentionally introduced by @anntzer in #6700 to avoid rounding issues with
float128
on linux. As a side-effect that PR changed how matplotlib deals with data with units attached.I've added a
_sanitize_extrema
helper so that we can usenp.asscalar
to avoid downcastingfloat128
data while still converting data to a numpy scalar under the hood. I've also expanded the existing test intest_colors.py
that handles ndarray subclass to also check this case as well.It would be great if this could be backported to 2.2.