Skip to content
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

Fix loading user-defined icons for Qt plot window #22135

Merged
merged 1 commit into from Jan 8, 2022

Conversation

timhoffm
Copy link
Member

@timhoffm timhoffm commented Jan 6, 2022

PR Summary

Closes #22131.

I'll refrain from writing a test for this because that would be quite cumbersome. One can manually test the correctness be deleting one of the _large icons. Then the plot window falls back to the low-res version of that icon.
It's also unlike to break again, because the comment should make future authors aware that we have to have the fall back for user icons.

@timhoffm timhoffm added this to the v3.5.2 milestone Jan 6, 2022
@QuLogic
Copy link
Member

QuLogic commented Jan 7, 2022

This affects Tk as well, I think.

@oscargus
Copy link
Contributor

oscargus commented Jan 7, 2022

Yes, here:

with Image.open(button._image_file.replace('.png', '_large.png')
if size > 24 else button._image_file) as im:
image = ImageTk.PhotoImage(im.resize((size, size)), master=self)

@daniilS
Copy link
Contributor

daniilS commented Jan 7, 2022

I've copied the changes to the Tk backend in #22147

Copy link
Member

@tacaswell tacaswell left a comment

Choose a reason for hiding this comment

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

I am also in favor of @QuLogic 's suggestion, but OK to merge either way.

@timhoffm
Copy link
Member Author

timhoffm commented Jan 7, 2022

Thanks, I found a cleaner way of doing this due to @QuLogic's suggestion and @daniilS's PR for Tk (#22147).

lib/matplotlib/backends/backend_qt.py Outdated Show resolved Hide resolved
@QuLogic QuLogic merged commit bf3c651 into matplotlib:main Jan 8, 2022
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Jan 8, 2022
QuLogic added a commit that referenced this pull request Jan 8, 2022
…135-on-v3.5.x

Backport PR #22135 on branch v3.5.x (Fix loading user-defined icons for Qt plot window)
@timhoffm timhoffm deleted the fix-icon-load branch January 8, 2022 08:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: png icon image fails to load for manually defined tool buttons
5 participants