-
Notifications
You must be signed in to change notification settings - Fork 367
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
fix imshow when image is RGBA #1906
Conversation
I see "SciTools-CLA-checker - CLA doesn't exist for all commits" but i don't understand what to do to correct this. |
Ok i have signed the CLA in the form https://scitools.org.uk/cla/v4/form and got the mail. |
The test
So i guess the fail is related to the coastline downloading and not to my changes? |
31828fe
to
68dbb1e
Compare
Ok tests are not failing anymore (i suspect a connection problem this weekend) |
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 I think the approach looks good. Just a few minor comments.
# if we don't pop alpha, imshow will apply (erroneously?) a | ||
# 1D alpha to the RGBA array | ||
# kwargs['alpha'] is guaranteed to be either 1D, 2D, or None | ||
alpha = kwargs.pop('alpha') |
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.
It looks like you are leaving the alpha in the kwargs now, so it will be handled by Matplotlib instead of down below when filling the alpha channel. Are there any weird interactions with showing an RGBA image with an alpha channel and specifying a 2d alpha? I guess I'm not sure what I would even expect in that case.
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.
Good question.
Before this patch, this wasn't a question as alpha kwargs was merged into the data.
import cartopy
import matplotlib.pyplot as plt
import matplotlib
import cartopy.crs as ccrs
import numpy as np
print("matplotlib : ", matplotlib.__version__)
dy, dx = (4,10)
# 2D alpha with stripes
alpha = np.ones((dy, dx))
alpha[:, ::2] = 0
plt.imshow(alpha)
plt.title("2D alpha")
matplotlib : 3.4.3
So with this 2D alpha for example, it should apply stripes :
So i checked what happens now that the alpha kwargs is simply passed to matplotlib :
# Create RGBA Image with linspace data and alpha
RGBA = np.linspace(0, 255*31, dx*dy*4, dtype=np.uint8).reshape((dy, dx, 4))
RGBA[:,:,3] = np.linspace(0, 255, dx, dtype=np.uint8).reshape(1,dx)
def plot():
fig = plt.figure(figsize=(8,3), dpi=120)
ax1 = fig.add_subplot(1, 3, 1, projection=ccrs.Orthographic(central_latitude=45))
ax1.set_title("Orthographic axis")
ax2 = fig.add_subplot(1, 3, 2, projection=ccrs.PlateCarree())
ax2.set_title("PlateCarree axis")
ax3 = fig.add_subplot(1, 3, 3)
ax3.set_title("no projection axis")
return fig, ax1, ax2, ax3
fig, ax1, ax2, ax3 = plot()
ax1.imshow(RGBA, alpha=alpha, transform=ccrs.PlateCarree())
ax2.imshow(RGBA, alpha=alpha, transform=ccrs.PlateCarree())
ax3.imshow(RGBA, alpha=alpha, )
fig.suptitle("RGBA u1 data in PlateCarre projection with alpha 2D")
matplotlib : 3.4.3
I don't see the alpha stripes.
So it looks like the 2D alpha kwargs has no impacts, as if no alpha kwargs was used.
It does nothing with cartopy, but with matplotlib too (so at least the cartopy behavior mimic matplotlib behavior?)
Which is weird because in the matplotlib documentation from imshow it is clearly stated that "If alpha is an array, the alpha blending values are applied pixel by pixel, and alpha must have the same shape as X".
SO it doesn't nothing, so do matplotlib.
Did i something wrong with matplotlib, or should i raise a ticket ?
(for info, the "problem" persist with RGBA u1 and with RGBA f4 too)
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.
Actually, before this patch I think the alpha did get converted to the data array as you mentioned (striped alpha). Which is actually not what Matplotlib does as you mention. So, we were actually deviating from Matplotlib there.
A quick look shows that the alpha 2d array only works with colormapped values on Matplotlib standard Axes. It does not apply to RGB or RGBA images. I think I agree with what Matplotlib is doing there, and I like what you've done here to force us to be more consistent with that. If someone wants a 2d alpha array, they should not be sending in RGB(A) data. (I guess RGB is a bit questionable, but I think it makes sense to keep those consistent)
In summary, I agree with what you've done here.
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.
Ok thanks
Co-authored-by: Greg Lucas <greg.m.lucas@gmail.com>
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.
I think this is the right fix. I'm going to leave open for a bit in case anyone else has other suggestions.
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.
i have reviewed the intent and implementation of this and support merging of this code onto master
Thanks, @ludwigVonKoopa! |
Hello ! It is my first PR on cartopy. I apologize if i did something wrong or forgot something
Problem
tldr : fix #1898
When using imshow with RGBA data, when data projection and axis projection are the same, there is no problems.
But when there is reprojection, the A (alpha channel) is skipped and forced to 1.
moreover, when
alpha
keyword is used, it completely broke data :but when RGBA image is np.float64 dtype (last example was np.uint8), the alpha channel works, but outlast alpha channel from image :
Implications
when reprojection is done, it should follow matplotlib usage for alpha.
Same plot with corrections :