Skip to content
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

avoid np.nan values in colors array returned by axes3d._shade_colors #3031

Merged
merged 2 commits into from May 13, 2014

Conversation

jrjohansson
Copy link
Contributor

Fix bug in Axes3D._shade_colors

There is a problem with handling NaN values in the shade array in on line 1677 in axes3d.py. If proj3d.mod(n) the corresponding value in shade is nan, and it is masked out from the shade array using mask on lines 1681 - 1682 but not line 1688. As a result the colors array will include nan values, and the following warnings are displayed when for example calling the bar3d function

.../lib/python3.4/site-packages/matplotlib-1.4.x-py3.4-linux-x86_64.egg/mpl_toolkits/mplot3d/axes3d.py:1679: RuntimeWarning: invalid value encountered in true_divide
    for n in normals])
.../lib/python3.4/site-packages/matplotlib-1.4.x-py3.4-linux-x86_64.egg/matplotlib/colors.py:401: RuntimeWarning: invalid value encountered in greater
    if (c.ravel() > 1).any() or (c.ravel() < 0).any():
.../python3.4/site-packages/matplotlib-1.4.x-py3.4-linux-x86_64.egg/matplotlib/colors.py:401: RuntimeWarning: invalid value encountered in less
    if (c.ravel() > 1).any() or (c.ravel() < 0).any():

(because here the array c includes nan values and the comparison operations fail).

This PR fixes this by avoiding nan in the shades array in Axes3D._shade_colors.

@tacaswell
Copy link
Member

@WeatherGod Can you take a look at this?

@WeatherGod
Copy link
Member

If we are just simply silencing the warnings, why not just simply use numpy.errstate() context manager?

http://docs.scipy.org/doc/numpy/reference/generated/numpy.errstate.html

Or, if it isn't available in the earliest version of numpy we support (I can't remember what it is), then we can simply use np.geterr() and np.seterr() ourselves. It makes it more explicit what it is that we are doing.

@tacaswell
Copy link
Member

It seems we check for >= 1.5

@jrjohansson
Copy link
Contributor Author

This PR doesn't just silence warnings, it prevents the errors from happening. In particular the second change, which prevents '_shade_colors' from returning a colors array that contains nan values.

@WeatherGod
Copy link
Member

Ah, right. Well, the problem I have with putting in an arbitrary value of zero is if the minimum shade value isn't zero, then the normalization will give a negative value for the norm() for anything zeroed, which might not look correct. Furthermore, it is entirely possible for this computation to produce a negative color, so we haven't really solved the problem.

I wonder if masked arrays would be better here? the Normalize() object would already support that properly, I am just not entirely sure what the level of support is in colors.py for a masked array of RGBA values.

@jrjohansson
Copy link
Contributor Author

That's a good point, shade shouldn't be set to zero without taking account for it in the normalization. Masked array might be a possible solution but a simpler solution without downsides as far as I can see would be to set shade[~mask] = min(shade[mask]) instead of zero, or still set shade[~mask] = 0 but include all values in the call to normalize a few lines below norm = Normalize(min(shade), max(shade)). In fact the need for mask could be eliminated completely by modifying the if ... else ... statement in the for loop, like if proj3d.mod(n) else 0. The case proj3d.mod(n) == 0 occur when the normal n is undefined, which can happen when bar3d is called with some value in the dz argument is zero (zero height in 3d histogram). In that case the exact value of the shade is not important because the area of the corresponding facet is zero.

WeatherGod added a commit that referenced this pull request May 13, 2014
avoid np.nan values in colors array returned by axes3d._shade_colors
@WeatherGod WeatherGod merged commit 67da9ed into matplotlib:master May 13, 2014
@WeatherGod
Copy link
Member

Yup, that looks good. Thanks for the fix!

@jrjohansson jrjohansson deleted the fix-axes3d-shade-color branch May 14, 2014 02:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants