Changing cmap(np.nan) to 'bad' value rather than 'under' value

[greglucas]

Previously, cmap(nan) would return the 'lower' value due to casting of ints within the __call__ function. This adds an np.isnan() check to return the bad value instead. New tests added to make sure -inf, nan, and inf, return the lower, bad, upper values respectively.

Addresses issue #9892

PR Summary

PR Checklist

• Has Pytest style unit tests
• Code is Flake 8 compliant
• New features are documented, with examples if plot related
• Documentation is sphinx and numpydoc compliant
• Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
• Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

[tacaswell]

Thanks @greglucas !

[jklymak]

Thanks, looks great, but there is a "Future Warning" coming from numpy that needs fixing!

[jklymak]

This triggers a test failure: FutureWarning: in the future, boolean array-likes will be handled as a boolean array index. I guess X is a list for these and needs to be an array?

[greglucas]

Interesting... That is actually the behavior I want for this case. This is warning because it is using numpy version 1.11, newer versions are just fine as far as I can tell. The failure is actually because of scalars being passed in rather than lists I think. It is producing a FutureWarning because there are 'good' values currently, but this can produce an error in the situation where a single nan is passed in cmap(np.nan):

X = np.nan
xa = np.array([X])
xa[np.isnan(X)] # errors because it tries to access the first index, which doesn't exist

Right now, this works as expected for newer versions of numpy. Let me think if there is a better way of doing this...

[greglucas]

@jklymak, I have modified the approach and tried to simplify the logic in the code to address the warning that was found before in earlier numpy versions. I removed the masked array creation and got rid of the need to store whether the inputs were scalar or not. I also added some more tests to check for arrays, masked arrays, and scalars now too.

[jklymak]

This looks right to me, but maybe should have another review after its passed the tests...

Many changes from my first review.

[tacaswell]

Won't removing this change the return type of cm(.5)?

[greglucas]

Yes, good catch! I apparently neglected to read the return statement of the method requiring a tuple if the input is a scalar.
Old: (0.798216, 0.280197, 0.469538, 1.0)
New: array([0.798216, 0.280197, 0.469538, 1.0])

I was trying to clean the code up a bit and not have multiple if statements for the two different cases. A simple fix to keep things consistent would be:

if np.iterable(X):
    # Return a tuple if the input is scalar
    rgba = tuple(rgba)

I should be able to push a new change with this addition up later today

[jklymak]

I guess w a new test that catches this?

[greglucas]

Updated with scalar tuple return and new tests. I didn't see any other tests for return values from other RGBA routines, I might open another PR to add some of those to help with this.

As a side thought, I was looking at the reason for a tuple return and it looks to be due to immutability and not overwriting a cached RGBA value. Another option would be to add a .copy() call to the returns where this is relevant, so that a copy of the array is always returned and no overwriting is possible. This would have the benefit of users being able to do mathematical array manipulations on the RGBA values easier and being more consistent by not returning different types for scalar/ndarray inputs.

[tacaswell]

In the interest of getting this PR in, I suggest we push the return-type change to a second PR?

[greglucas]

Yes, I agree with that. I was just opening up food for thought. The return type change could be non-trivial in terms of possibly messing up other areas of code that rely on a tuple being returned, I haven't looked in-depth into it at all.

For now, I believe this should be good to go.

[tacaswell]

Thanks @greglucas . Congratulations on your first Matplotlib PR 🎉 ! Hopefully we will hear from you again!

[greglucas]

Thank you, @tacaswell! I will try and help out where I can. You all do great (and often un-thanked) work on this project that I use all the time, so thank you for the work you do.