-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
ENH: support alpha arrays in collections #6268
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
Conversation
This doesn't relate to the idea of 2D colormaps, right? (The second On Sun, Apr 3, 2016 at 10:24 PM, Eric Firing notifications@github.com
|
@WeatherGod, this could be used for that purpose; it is up to the user to generate the alpha array on a 0-1 scale based on whatever measure of level of confidence, or anything else, the user wishes to encode via transparency. The alpha is applied here after the colormapping, so it can be based on a different set of data values and a different norm; it is not done by modifying alpha in the colormap LUT. (That can also be done, but it is a completely independent and rather different operation.) |
I'm not sure how obsolete this one is.... |
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 looks to need examples and tests?
@efiring, I would like this to go in. Any appetite for resurrecting? |
OK, I will have a look. |
I think this is ready for review now. It actually does more than I originally intended. |
y = np.arange(4) | ||
z = np.arange(9).reshape((3, 3)) | ||
alpha = z / z.max() | ||
alpha_flat = alpha.ravel() |
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.
So at somepoint someone will put some masked or NaN into alpha. What will happen, and what do you recommend?
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.
Fail on nan; ignore mask. I don't think it is worthwhile for us to try to propagate nans and masked arrays from the alpha input.
9c24406
to
20b894b
Compare
Squashed, and passing tests; it's ready, as far as I can see. |
self.pchanged() | ||
self.stale = True | ||
martist.Artist._set_alpha_for_array(self, alpha) | ||
if np.ndim(alpha) not in (0, 2): |
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.
Do you not want to check this before calling the setter?
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.
No, I think it is better to have the more basic argument-checking from the setter before checking the number of dimensions.
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.
Wouldn't that leave self.alpha
in an inconsistent state?
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.
Why would that matter? An exception is being raised either way. Look at _ImageBase.set_data
. All of the validation is being done after the attribute assignment:
self._A = cbook.safe_masked_invalid(A, copy=True)
It looks like the set_data
code would be slightly more efficient and readable if the attribute assignment were done at the end, but that's not the issue here. For set_alpha
, both efficiency and logical order of validation checks are better with the ndim check after set_alpha_for_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.
To resolve this, can we easily write a test that shows how it works?
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 don't see anything to resolve. I have already included a test for this case, raising an exception if the wrong number of dimensions is supplied for an image.
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.
FWIW, if we already have cases of inconsistent state on invalid input, then I'm slightly less concerned about adding one more.
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 impossible to do all validation before setting attributes, given independent setters, because valid alpha depends on data, and vice-versa. That's why one of the tests I added calls update_scalarmappable
; that's where the validation has to be, because that is where we have both alpha and data and can cross-check them.
@QuLogic Thank you. I think I have addressed everything you have found so far. |
Previously this was supported only for images. Now it works for collections, and when directly calling a colormap or to_rgba_array.
cfdb753
to
9e0861e
Compare
I have rebased again because the Travis merge test seemed to have gotten stuck (I think it actually ran and passed, but failed to report that back correctly), and a bunch of stuff was merged recently. |
Now, after a simple rebase, testing is throwing up what I think are unrelated errors--something about jupyter on windows? |
lut[:, -1] = alpha | ||
# If the bad value is set to have a color, then we | ||
# override its alpha just as for any other value. | ||
alpha = (alpha * 255).astype(np.uint8) |
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.
Stall a couple of code paths not tested? (L625 and 632)
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.
Modulo tests that @jklymak requested.
On 2020/09/11 2:15 PM, Jody Klymak wrote:
a couple of code paths not tested? (L625 and 632)
OK, working on those.
|
Fixup and clarification in colors; more tests Tweak whats_new
d4eff40
to
c9d0741
Compare
Thanks @efiring This will be pretty helpful in creating graphics with two-dimensions in Z. |
This started by supporting alpha arrays in collections with color mapping but it now also works when a single color is provided, or when a list of color specifications is given.
Tests are included.
There is an entry in next_whats_new.