-
-
Notifications
You must be signed in to change notification settings - Fork 7.8k
Bug causes to_rgba to fail inside cm.py #10890
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
Comments
Can you provide a minimal example that triggers this? Thanks! |
if the input image has been normalized to zero (-127 to +127 range) and is a float it will trigger this section.
|
Can you provide a minimal self contained example written in python that doesn’t use an external file that triggers this? Thanks. |
Wow. ok... that was an adventure. Hope this is meaningful... import matplotlib.pyplot as plt
import numpy as np
test = np.array([[[ 2.4265968e+01, -7.1251709e+01, 1.1596016e+01],
[ 4.1356895e+01, -6.1591396e+01, 1.8787498e+01],
[ 6.0224663e+01, -5.3484486e+01, 2.7164375e+01],
[ 6.2998756e+01, -6.2542957e+01, 2.8223625e+01],
[ 5.0635292e+01, -8.5200035e+01, 2.2165154e+01]],
[[ 3.0379570e+01, -5.6437828e+01, 1.2119255e+01],
[ 4.8944832e+01, -4.6965721e+01, 2.5350716e+01],
[ 6.3876762e+01, -4.4302704e+01, 3.1537758e+01],
[ 5.7498558e+01, -6.3408409e+01, 2.1198906e+01],
[ 4.3169197e+01, -8.9243187e+01, 1.1302132e+01]],
[[ 3.0193733e+01, -5.1721474e+01, 5.0541916e+00],
[ 4.6437859e+01, -4.7197594e+01, 2.3918770e+01],
[ 5.5936256e+01, -4.9692642e+01, 2.7207161e+01],
[ 4.3793571e+01, -6.9284180e+01, 9.7025375e+00],
[ 3.2357903e+01, -8.4746979e+01, 1.1040497e-01]],
[[ 2.8536415e+01, -6.0043530e+01, -6.5765381e-03],
[ 4.0998665e+01, -5.7634468e+01, 2.0513100e+01],
[ 4.3069878e+01, -6.6510132e+01, 1.6839577e+01],
[ 3.2806328e+01, -7.4617233e+01, 2.3700790e+00],
[ 3.2372643e+01, -6.7550941e+01, 3.2968445e+00]],
[[ 3.3632256e+01, -7.0232697e+01, 3.0147018e+00],
[ 4.4523140e+01, -7.1141457e+01, 2.0268974e+01],
[ 3.8679253e+01, -8.2500259e+01, 1.0053246e+01],
[ 3.2569786e+01, -7.6080200e+01, 3.6950302e+00],
[ 3.4381584e+01, -5.4521236e+01, 7.9723282e+00]]], dtype=np.float32)
plt.imshow(test)
plt.show() |
Ah, so they fixed it by forcing you to normalize... when the real problem is that someone messed up on a logic statement... If you add the parentheses correctly, the function will work on non-normalized floats! I think it was fixed and reduced functionality incorrectly. |
Note that the old code would catch the errors without the clipping requirement.... (with parentheses) |
For 2.2 we enforce the range 0-1 by clipping, regardless of the value of In 2.1 this same code threw an error. I agree that if you change the brackets it does not throw an error, and returns a plot. I'm not sure how the plot values get turned into valid RGBA values. I'd have to look more. But all the documentation states that if you pass floats, they should be normalized between 0 and 1. I don't think the current behaviour is incorrect to enforce that. However, I'm sure we can discuss, particularly if this is a regression. |
I suppose that if the documentation says that the input should be normalized... it is the 'correct' place to go from, however it does make it so that this error check is meaningless as neither the normalize or the value checks can ever fail. I can tell you that if you just put the brackets in (2.1.2) the result you get is a graph of the data normalized across the full color range... unfortunately I didn't take a screenshot and just updated to 2.2.2. My feeling is that the normalization requirement should only be there if it is truly needed, and since it is not... It is better to effectively do the normalization in the color space rather than to corrupt the data by clipping. If the normalized input was a hard requirement, I would prefer it throw an error if I passed it un-normalized data rather than show me a false image (with a screen flag I might not catch). This can be the kind of problem you chase your tail around for hours as a student... not realizing how 'severe' the clipping was in your image. (note that up to 100x range is lost in the clipping for our example above) |
There is a big warning that tells you your data was clipped and what to do about it. My personal opinion is that you can't just normalize all data because sometimes a data set with a range between 0.2 and 0.4 is meant to be comapred to a data range between 0 and 1. So do you only auto normalize if you are outside 0-1? At some point the user has to decide, so I'm pretty happy with a hard clip and warn strategy. |
I get your point, it is all about desired affect vs. hidden effect. If you want to compare things that are 0.2 to 0.4 and 0.4 to 0.6 will it compare them 'correctly'? Yes - according to the documentation. So I have to agree that is the right way to go due to the fact that the 'auto normalize' that I was seeing could hide this effect on you (and gives you something outside of the docs as well. But.... the original parentheses... it is an error :-) |
I am not sure if I was supposed to close that or not... :-/ |
Up to you! If you think there is more dicsussion to be had, I'm fine for it to be re-opened. I'm just a junior member around here, and not always right by a long stretch, so others may want to chime in. |
Not sure how to re-open. Someone should fix those parentheses at least for OCD sake X-D Ok... I'm learning... |
The statement is correct as-is because "or" has higher precedence than "and": |
That is very interesting as adding the parentheses and running locally caused a change in behavior for me. |
@efiring you have read the table backwards; the top is lowest precedence, so |
Oops! Sorry. |
The original error was fixed, so I don't see why this is still open. If there is still a problem, since it is not the original one, please open a new issue for it. |
I was running a test with some new images and plotting, code called cm.to_rgba() to convert the files for keras processing.
Matplotlib 2.1.2 (but verified in the current repository)
It 'works' however it throws an error, by mistake due to this error in code:
If xx.min() <0 then this code piece triggers.
Should be:
Sorry.. not familiar enough with making pull requests and changes myself to do this!
The text was updated successfully, but these errors were encountered: