NotificationsYou must be signed in to change notification settings
Fork7.9k
Star21.3k

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

Jump to bottom

Merged

jklymak merged 2 commits intomatplotlib:masterfromefiring:alpha_array

Sep 12, 2020

Merged

ENH: support alpha arrays in collections#6268

jklymak merged 2 commits intomatplotlib:masterfromefiring:alpha_array

Sep 12, 2020

Conversation

Copy link

Member

efiring commentedApr 4, 2016•
edited
Loading

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.

efiring added this to the2.1 (next point release) milestone

Apr 4, 2016

efiring added the status: needs review label

Apr 4, 2016

Copy link

Member

WeatherGod commentedApr 5, 2016

This doesn't relate to the idea of 2D colormaps, right? (The second
dimension being alpha, typically indicating level of confidence in the data)

On Sun, Apr 3, 2016 at 10:24 PM, Eric Firingnotifications@github.com
wrote:

This is apossible alternative to#6250
#6250. It applies the
alpha array in Colormap.call. It probably needs quite a bit more work
for adequate documentation and argument checking at higher levels. And
tests, of course.
You can view, comment on, or merge this pull request online at:
#6268
Commit Summary
ENH: support alpha arrays when color mapping
File Changes
M lib/matplotlib/collections.py
https://github.com/matplotlib/matplotlib/pull/6268/files#diff-0 (19)
M lib/matplotlib/colors.py
https://github.com/matplotlib/matplotlib/pull/6268/files#diff-1 (40)
Patch Links:
https://github.com/matplotlib/matplotlib/pull/6268.patch
https://github.com/matplotlib/matplotlib/pull/6268.diff
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#6268

Copy link

MemberAuthor

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.)

tacaswell modified the milestones:2.1 (next point release),2.2 (next next feature release)

Sep 24, 2017

Copy link

Member

jklymak commentedMay 2, 2018

I'm not sure how obsolete this one is....

efiring force-pushed thealpha_array branch fromb2245b6 toa185a2aCompare

June 11, 2018 01:02

efiring modified the milestones:needs sorting,v3.0

Jun 11, 2018

jklymak reviewed

Jul 9, 2018

View reviewed changes

Copy link

Member

jklymak left a 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 modified the milestones:v3.0,v3.1

Jul 9, 2018

jklymak added the status: needs tests label

Aug 17, 2018

dstansby removed the status: needs review label

Nov 3, 2018

jklymak modified the milestones:v3.1.0,v3.2.0

Feb 7, 2019

timhoffm modified the milestones:v3.2.0,v3.3.0

Aug 16, 2019

jklymak added the stale label

May 26, 2020

jklymak modified the milestones:v3.3.0,v3.4.0

May 26, 2020

Copy link

Member

jklymak commentedJul 14, 2020

@efiring,I would like this to go in. Any appetite for resurrecting?

Copy link

MemberAuthor

efiring commentedJul 16, 2020

OK, I will have a look.

efiring force-pushed thealpha_array branch froma185a2a to441c4a6Compare

August 10, 2020 02:29

efiring added status: work in progress and removed stale labels

Aug 10, 2020

efiring marked this pull request as draft

August 10, 2020 02:31

efiring mentioned this pull request

Aug 10, 2020

Wishlist: Multidimensional colormaps#4369

Closed

efiring removed the status: needs tests label

Sep 6, 2020

efiring removed the status: work in progress label

Sep 6, 2020

efiring marked this pull request as ready for review

September 6, 2020 23:30

efiring changed the title~~ENH: support alpha arrays when color mapping~~ENH: support alpha arrays in collections

Sep 7, 2020

Copy link

MemberAuthor

efiring commentedSep 7, 2020

I think this is ready for review now. It actually does more than I originally intended.
codecov/project/tests is unhappy; do I need to do something about that?

jklymak reviewed

Sep 8, 2020

View reviewed changes

lib/matplotlib/artist.pyShow resolvedHide resolved

lib/matplotlib/tests/test_collections.py

		y = np.arange(4)
		z = np.arange(9).reshape((3, 3))
		alpha = z / z.max()
		alpha_flat = alpha.ravel()

Copy link

Member

jklymakSep 8, 2020

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?

Copy link

MemberAuthor

efiringSep 8, 2020

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.

jklymak self-requested a review

September 8, 2020 05:00

efiring force-pushed thealpha_array branch 2 times, most recently from9c24406 to20b894bCompare

September 8, 2020 20:51

Copy link

MemberAuthor

efiring commentedSep 8, 2020

Squashed, and passing tests; it's ready, as far as I can see.

QuLogic reviewed

Sep 9, 2020

View reviewed changes

lib/matplotlib/artist.py OutdatedShow resolvedHide resolved

lib/matplotlib/collections.py OutdatedShow resolvedHide resolved

lib/matplotlib/colors.py OutdatedShow resolvedHide resolved

lib/matplotlib/image.py OutdatedShow resolvedHide resolved

lib/matplotlib/image.py

		self.pchanged()
		self.stale = True
		martist.Artist._set_alpha_for_array(self, alpha)
		if np.ndim(alpha) not in (0, 2):

Copy link

Member

QuLogicSep 9, 2020

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?

Copy link

MemberAuthor

efiringSep 9, 2020

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.

Copy link

Member

QuLogicSep 10, 2020

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?

Copy link

MemberAuthor

efiringSep 10, 2020

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.

Copy link

Member

jklymakSep 11, 2020

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?

Copy link

MemberAuthor

efiringSep 12, 2020

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.

Copy link

Member

QuLogicSep 12, 2020

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.

Copy link

MemberAuthor

efiringSep 12, 2020

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.

Copy link

MemberAuthor

efiring commentedSep 9, 2020

@QuLogic Thank you. I think I have addressed everything you have found so far.

Alpha argument can be an array

821617b

Previously this was supported only for images.  Now it works forcollections, and when directly calling a colormap or to_rgba_array.

efiring force-pushed thealpha_array branch fromcfdb753 to9e0861eCompare

September 11, 2020 20:02

Copy link

MemberAuthor

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 mentioned this pull request

Sep 11, 2020

FIX: pcolor/pcolormesh honour edgecolors kwarg when facecolors is set 'none'#12226

Closed

3 tasks

Copy link

MemberAuthor

efiring commentedSep 12, 2020

Now, after a simple rebase, testing is throwing up what I think are unrelated errors--something about jupyter on windows?

jklymak reviewed

Sep 12, 2020

View reviewed changes

lib/matplotlib/colors.py

		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)

Copy link

Member

jklymakSep 12, 2020

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 approved these changes

Sep 12, 2020

View reviewed changes

Copy link

Member

QuLogic left a 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.

doc/users/next_whats_new/alpha_array.rst OutdatedShow resolvedHide resolved

Copy link

MemberAuthor

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.

Fixups from review

c9d0741

Fixup and clarification in colors; more testsTweak whats_new

efiring force-pushed thealpha_array branch fromd4eff40 toc9d0741Compare

September 12, 2020 02:45

jklymak approved these changes

Sep 12, 2020

View reviewed changes

jklymak merged commit1b1afe1 intomatplotlib:master

Sep 12, 2020

Copy link

Member

jklymak commentedSep 12, 2020

Thanks@efiring This will be pretty helpful in creating graphics with two-dimensions in Z.

efiring deleted the alpha_array branch

September 12, 2020 19:21

Labels

None yet

7 participants

Movatterモバイル変換

Uh oh!

ENH: support alpha arrays in collections#6268

ENH: support alpha arrays in collections#6268

Uh oh!

Conversation

efiring commentedApr 4, 2016• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

WeatherGod commentedApr 5, 2016

tests, of course.

Uh oh!

efiring commentedApr 5, 2016

Uh oh!

jklymak commentedMay 2, 2018

Uh oh!

jklymak left a comment

Choose a reason for hiding this comment

Uh oh!

jklymak commentedJul 14, 2020

Uh oh!

efiring commentedJul 16, 2020

Uh oh!

efiring commentedSep 7, 2020

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

efiring commentedSep 8, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

efiring commentedSep 9, 2020

Uh oh!

efiring commentedSep 11, 2020

Uh oh!

efiring commentedSep 12, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

QuLogic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

efiring commentedSep 12, 2020 via email

Uh oh!

jklymak commentedSep 12, 2020

Uh oh!

Uh oh!

efiring commentedApr 4, 2016•
edited
Loading