FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Allow array alpha for imshow by tillahoffmann · Pull Request #8183 · matplotlib/matplotlib · GitHub

  1. matplotlib
  2. matplotlib

Repository navigation

Allow array alpha for imshow - #8183

Closed
tillahoffmann wants to merge 10 commits into
matplotlib:masterfrom
tillahoffmann:arrayalpha
ConversationCommits (10)ChecksFiles changed
Closed

Allow array alpha for imshow#8183
tillahoffmann wants to merge 10 commits into
matplotlib:masterfrom
tillahoffmann:arrayalpha

Conversation

tillahoffmann commented Mar 2, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor

This PR allows the alpha argument of imshow to be an array with shape matching the image to support pixel-by-pixel opacity.

dstansby commented Mar 2, 2017

Copy link
Copy Markdown
Member

Thanks for submitting this, it looks like a good improvement!

A couple of things that need to be fixed:

There's lots of information that should cover the above at http://matplotlib.org/devel/index.html - let us know if you need any help with anything though.

dstansby added this to the 2.1 (next point release) milestone Mar 2, 2017
tacaswell reviewed Mar 2, 2017
View reviewed changes
Comment thread lib/matplotlib/image.py
hid_output = output
output = np.ma.masked_array(
hid_output[..., 0], hid_output[..., 3] < 0.5)
hid_output[..., 0], hid_output[..., 3] == 0)

tacaswell Mar 2, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think this is going to break a test, but may fix an issue that @QuLogic found with imshow after #8024

tillahoffmann Mar 3, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Yes, this line is indeed causing the problem. In particular, the alpha channel is interpolated near the boundary of an image. The < 0.5 implementation will set pixels that are "more than half transparent" to fully transparent whereas the == 0 implementation only treats them as masked if they are fully transparent.

In short, the boundaries in a few tests differ slightly.

I believe the == 0 implementation is preferable because it effectively provides anti aliasing on the boundary by restoring the alpha channel. Here is a zoomed-in example.


tillahoffmann Mar 3, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Hm, but this causes trouble if there are bad data in image. For example, bad values in one of the tests are shown as blue. If we restore the alpha channel, all bad blue values will become transparent. As a compromise, I have rerendered the problematic images (changes only occur at the boundary) and I only restore the alpha channel if _array_alpha is given.

tacaswell commented Mar 2, 2017

Copy link
Copy Markdown
Member

I have to run to dinner, but I suspect the ==0 change is what is breaking the tests, which should be pulled out into it's own PR and put (with re-rendering the tests) on 2.0.x.

QuLogic commented Mar 7, 2017

Copy link
Copy Markdown
Member

Please rebase instead of merging, and don't make extra empty commits just to trigger CI.

dstansby commented Mar 7, 2017

Copy link
Copy Markdown
Member

FYI, we (members of the matplotlib organisation) can manually trigger CI if needed, and you should be able to manually trigger it on your own fork.

tillahoffmann commented Mar 8, 2017

Copy link
Copy Markdown
Contributor Author

Ok, will do that in the future.

tacaswell self-assigned this Mar 8, 2017

tacaswell commented Mar 8, 2017

Copy link
Copy Markdown
Member

Please leave this one for me to review / merge.

QuLogic mentioned this pull request Mar 13, 2017
Closed
tacaswell modified the milestones: 2.1 (next point release), 2.2 (next next feature release) Aug 29, 2017

jklymak commented Jul 16, 2019

Copy link
Copy Markdown
Member

This looks useful, but fell under the radar. @tillahoffmann do you have the bandwidth to rebase, and we can try and resurrect? If no one touches it for a while, please ping (and re-ping). Sorry for the lack of response here.

jklymak modified the milestones: needs sorting, v3.2.0 Jul 16, 2019

tillahoffmann commented Jul 25, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Will close this one and send a new PR because the implementation has changed enough to make a new implementation easier.

jklymak commented Jul 25, 2019

Copy link
Copy Markdown
Member

Don’t be shy at harassing us

tillahoffmann mentioned this pull request Jul 25, 2019
Merged
4 of 6 tasks

tacaswell commented Jul 25, 2019

Copy link
Copy Markdown
Member

🐑 I thought this had gone in already! This one is definitely on me...

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

tacaswell tacaswell left review comments

Assignees

tacaswell

Labels

status: needs rebase

Projects

None yet

Milestone

v3.2.0

Development

Successfully merging this pull request may close these issues.

5 participants

Back | FazBrowse Home | New Git URL