Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Separate alpha and rbg interpolation then recombine to fix issue11316#12062

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

Merged
dstansby merged 2 commits intomatplotlib:masterfromdabana:fix-for-issue11316
Mar 1, 2019

Conversation

@dabana
Copy link
Contributor

PR Summary

Addresses issue#11316.

@tacaswell suggested two solutions:

  1. to change theoutput array starting as (1, 1, 1, 0)
  2. re-sample the RGB and A arrays separatly

I tested both solutions. Solution 1 did not work. I tested variousoutput arrays but it does not have any impact on the result: grey boundaries are still visible. Finally, solution was the right one. I interpolate the alpha channel and the 3 rgb channels a 2 separate images. I then recombine into a 4-channel RGBA output.

This is a crude fix to the issue. Help wanted to improve elegance and/or efficiency.

PR Checklist

  • Has Pytest style unit tests
  • Code isFlake 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

@jklymak
Copy link
Member

Thanks@dabana Looks promising - I'm surprised this doesn't hit any tests. Since it doesn't, can you make some?

Copy link
Member

@tacaswelltacaswell left a comment

Choose a reason for hiding this comment

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

Overall 👍

Left some comments about making the computation more efficient (both memory and cpu)

@tacaswelltacaswell added this to thev3.1 milestoneSep 7, 2018
@tacaswell
Copy link
Member

Thanks@dabana ! Can you also add a test of this? The example in original issue would be a perfect test image.

@dabana
Copy link
ContributorAuthor

@tacaswell I believe this is ready to merge.

@timhoffm
Copy link
Member

I don‘t have a complete overview of the topic, but can we save the alpha interpolation if there is no alpha involved?

Copy link
Member

@tacaswelltacaswell left a comment

Choose a reason for hiding this comment

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

Who ever merges this should use squash merge if the duplicate images are not rebased out.

@tacaswell
Copy link
Member

@dabana Sorry this sat for so long. Could you possible resbase / squash this a bit so that we only have one copy of the images you added?

@dabana
Copy link
ContributorAuthor

@dabana Sorry this sat for so long. Could you possible resbase / squash this a bit so that we only have one copy of the images you added?

Sorry@tacaswell I don't think I am getting what you mean. I just synced my fork with the base repo and I rebased this branch with the master. There were no conflicts and I do not see any duplicates of my images in the baseline_image directory.

@jklymak
Copy link
Member

@dabana the 8 commits have different versions of the image files in them. If we merge as is, each version of the images will be put in the repo separately, despite the fact the last version is the one we want.

@jklymak
Copy link
Member

For how to squash, the somewhat poorly named link here should help: Please do the backup step 😉!https://matplotlib.org/devel/gitwash/development_workflow.html#rewriting-commit-history

@tacaswell
Copy link
Member

@dabana you missed theforce push step (the git command line helpfully provides you safe but wrong advise)

I think you want to do

git checkout fix-for-issue11316git resest --hard afa6123  # THIS ONE IS A BIT DANGEROUS, but `git reflog` can save yougit push --force-with-lease   # THIS ONE IS THE DANGEROUS ONE, but more force pushes can help

@tacaswelltacaswell added the Release criticalFor bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. labelFeb 26, 2019
@tacaswell
Copy link
Member

the docs CI is failing because it runs on the branch head instead of simulating the merge and then running the tests so you don't have the fixes from master branch for these docs build issues.

@dabana
Copy link
ContributorAuthor

the docs CI is failing because it runs on the branch head instead of simulating the merge and then running the tests so you don't have the fixes from master branch for these docs build issues.

Is there anything I can do about it to help the merge?

@tacaswell
Copy link
Member

If you re-base the branch on to current master (and then force-push again) the docs will pass, however I don't think we are going to hold the PR over that.

We are just waiting for a second core-dev to review and merge this.

Copy link
Member

@dstansbydstansby left a comment

Choose a reason for hiding this comment

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

👍 thanks a lot!

@dstansbydstansby merged commit7effa1e intomatplotlib:masterMar 1, 2019
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull requestMar 1, 2019
dstansby added a commit that referenced this pull requestMar 1, 2019
…062-on-v3.1.xBackport PR#12062 on branch v3.1.x (Separate alpha and rbg interpolation then recombine to fix issue11316)
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@jklymakjklymakjklymak left review comments

@tacaswelltacaswelltacaswell approved these changes

@dstansbydstansbydstansby approved these changes

Assignees

No one assigned

Labels

Release criticalFor bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.

Projects

None yet

Milestone

v3.1.0

Development

Successfully merging this pull request may close these issues.

6 participants

@dabana@jklymak@tacaswell@timhoffm@dstansby@QuLogic
[8]ページ先頭
©2009-2025 Movatter.jp