-
-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Fixed bilinear interpolation for SegmentedBivarColormap
#30824
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
base: main
Are you sure you want to change the base?
Conversation
20e10e5 to
9f1bc1e
Compare
9f1bc1e to
9b4ec6d
Compare
|
As a sanity check of the 1D analogue, >>> import matplotlib as mpl
>>> greys = mpl.colormaps["Greys"]
>>> print(greys(0))
(np.float64(1.0), np.float64(1.0), np.float64(1.0), np.float64(1.0))
>>> print(greys(255))
(np.float64(0.0), np.float64(0.0), np.float64(0.0), np.float64(1.0)) |
b3d74fc to
4c66b4e
Compare
greglucas
left a comment
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.
Overall, this looks good to me and reads nice to do the math in numpy-space rather than Agg-space.
We should probably add a short change-note entry indicating this is fixing a minor bug in the interpolation scheme as it does change some results.
|
cc @trygvrad for a review |
|
I realized that because the
I've added an entry, but let me know if it's in the wrong place |
|
I think it should be in |
|
The code looks reasonable to me, it is good to get this fixed. @ayshih If you are actively using bivariate colormaps, I would be interested it knowing more about your use case. We will need to add more bivariate colormaps to matplotlib as the project develops and in that context it is useful to hear from users. |
6baabe1 to
5a56a90
Compare
Ah, now I understand the system. Moved.
Heh, the bivariate colormaps look neat, but I haven't actually used them for anything. This PR came to be because I was befuddled why my fixes to the Agg resampler in #30184 affected colormaps. |
PR summary
The lookup table generated by
SegmentedBivarColormapis slightly inaccurate. Most notably, the darkest color in the lookup table is not as dark as the darkest color in the input, and the same goes for the brightest color. This has been glossed over in the tests due to the liberal use ofatol.SegmentedBivarColormapuses the image-plotting resampler (Agg) to perform bilinear interpolation, and these inaccuracies result from a bug in the transform definition plus a second bug that would be fixed by #30184. However, just fixing these two bugs is not a good solution. The Agg resampler – which is intended for producing good-enough images for visual display – is problematic because it divides each pixel into 256 subpixels, which means it internally calculates 256 * M - 1 intermediate color candidates between the end colors, but the color table wants 254 intermediate colors between the end colors (to get 256 colors in total). That means fixing the aforementioned two bugs results in a non-smooth color table because 254 does not divide into 256 * M - 1 cleanly.So, this PR instead takes the approach of eschewing the Agg resampler entirely and performing the bilinear interpolation using straight NumPy. Now the extreme colors are exactly what they should be.
I'll note that the tests still need to use a bit ofUpdate: now fixed.atolbecause the definition ofBiOrangeBlueuses input that is rounded to 3 decimal places.PR checklist