Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Add alpha-array support to _rgb_to_rgba#26520
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 ourterms of service andprivacy 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?
Add alpha-array support to _rgb_to_rgba#26520
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Hi@AALAM98mod100 - just leaving a note that if you want this to be reviewed make sure you mark it as "Ready for review". Cheers! |
Thanks for the PR! As far as I can tell, it looks good! However, I am a bit surprised to see that it doesn't work for SVGs? Especially since there is an embedded PNG in the SVG. This may be a another issue though... But do you have any comments on that? Maybe we should wait with adding SVG and PDF test images (although they are quite small, so not sure if it really is a problem). Right now we are in the final stages of releasing 3.8, so it may take some time to get additional feedback. |
| @image_comparison(['image_alpha_array'], remove_text=True) | ||
| def test_image_alpha_array(): | ||
| _, ax = plt.subplots() | ||
| # create a 2x2 grid. If alpha is applying correctly, | ||
| # the top half should not match the 0.5 valued pixel | ||
| # in the bottom half. | ||
| arr = np.array([[.5, .5], [.75, .5]]) | ||
| cmap = plt.get_cmap('gray') | ||
| norm = colors.Normalize() | ||
| arr_rgb = cmap(norm(arr))[:, :, :3] | ||
| alpha = np.ones_like(arr) | ||
| alpha[:1] = 0.2 | ||
| ax.imshow(arr, alpha=alpha, cmap='gray', interpolation='none') |
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.
This can be better written as acheck_figures_equal test. Compare a (n, m, 3) array plus additionalalpha to the same plot with a(n, m, 4) array.
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 is also concerning that the png and svg images are different....
| """ | ||
| Convert an RGB image to RGBA, as required by the image resample C++ | ||
| extension. | ||
| """ | ||
| rgba = np.zeros((A.shape[0], A.shape[1], 4), dtype=A.dtype) | ||
| if alpha is None or np.ndim(alpha) == 0: |
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.
what input isnp.ndim(alpha) == 0 addressing?
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.
Scalar alpha input should remaing unaffected by new code. This ensures just that
Just as a FYI, in the original issue thread, some incorrect result was observed using |
| elif A.shape[2] == 4: | ||
| array_alpha = self.get_alpha() | ||
| if array_alpha is not None and np.ndim(array_alpha) != 0: | ||
| A[:, :, 3] *= array_alpha # blend alphas of image and param |
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.
This modifiesA in place where as in the other branches we get a copy ofA back. Because it modifies in in place and we cacheA higher up in the call stack it will apply the alpha everytime it draws.
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.
The tests should usecheck_figures_equal and the multiple application of alpha needs to be fixed.
Uh oh!
There was an error while loading.Please reload this page.
PR summary
PR checklist
Plotting demo:
Code