Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork8.1k
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation
WeatherGod commentedApr 5, 2016
This doesn't relate to the idea of 2D colormaps, right? (The second On Sun, Apr 3, 2016 at 10:24 PM, Eric Firingnotifications@github.com
|
efiring commentedApr 5, 2016
@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 hereafter the colormapping, so it can be based on a different set of data values and a different norm; it isnot done by modifying alpha in the colormap LUT. (That can also be done, but it is a completely independent and rather different operation.) |
jklymak commentedMay 2, 2018
I'm not sure how obsolete this one is.... |
jklymak 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.
This looks to need examples and tests?
jklymak commentedJul 14, 2020
@efiring,I would like this to go in. Any appetite for resurrecting? |
efiring commentedJul 16, 2020
OK, I will have a look. |
efiring commentedSep 7, 2020
I think this is ready for review now. It actually does more than I originally intended. |
Uh oh!
There was an error while loading.Please reload this page.
| 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 to20b894bCompareefiring commentedSep 8, 2020
Squashed, and passing tests; it's ready, as far as I can see. |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
| self.pchanged() | ||
| self.stale=True | ||
| martist.Artist._set_alpha_for_array(self,alpha) | ||
| ifnp.ndim(alpha)notin (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 leaveself.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 doneafter the attribute assignment:
self._A=cbook.safe_masked_invalid(A,copy=True)
It looks like theset_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. Forset_alpha, both efficiency and logical order of validation checks are better with the ndim check afterset_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 doall 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 callsupdate_scalarmappable; that's where the validation has to be, because that is where we have both alpha and data and can cross-check them.
efiring commentedSep 9, 2020
@QuLogic Thank you. I think I have addressed everything you have found so far. |
Previously this was supported only for images. Now it works forcollections, and when directly calling a colormap or to_rgba_array.
efiring commentedSep 11, 2020
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. |
efiring commentedSep 12, 2020
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)
QuLogic 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.
Modulo tests that@jklymak requested.
Uh oh!
There was an error while loading.Please reload this page.
efiring commentedSep 12, 2020 via email
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 testsTweak whats_new
jklymak commentedSep 12, 2020
Thanks@efiring This will be pretty helpful in creating graphics with two-dimensions in Z. |
Uh oh!
There was an error while loading.Please reload this page.
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.