Skip to content

tk/wx: Fix saving after the window is closed #17391

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
May 20, 2020
Merged

tk/wx: Fix saving after the window is closed #17391

merged 5 commits into from
May 20, 2020

Conversation

QuLogic
Copy link
Member

@QuLogic QuLogic commented May 12, 2020 •
edited
Loading

PR Summary

Fixes #16909.
Fixes #17388.

PR Checklist

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

@QuLogic QuLogic added this to the v3.2.2 milestone May 12, 2020
anntzer
anntzer approved these changes May 12, 2020
View reviewed changes
Copy link
Contributor

@anntzer anntzer left a comment

Choose a reason for hiding this comment

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

seems reasonable; conditional on ci

QuLogic
QuLogic commented May 12, 2020
View reviewed changes
lib/matplotlib/tests/test_backends_interactive.py
plt.show()

# Ensure that the window is really closed.
plt.pause(0.5)
Copy link
Member Author

Choose a reason for hiding this comment

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

BTW, if anyone can think of a better way to do this, I'd change it.

I tried fig.canvas.flush_events(), but that crashes because the main window is gone. If you ignore any crashes, then the test doesn't fail like it should on master.

@QuLogic
Copy link
Member Author

QuLogic commented May 13, 2020

The test shows that that wx is also broken the same way. The Qt backend is also failing, but a little differently and might be a reproducibility thing.

@QuLogic
Copy link
Member Author

QuLogic commented May 13, 2020

Partially fixed wx, but I guess the failure on other Python versions is a little different. Don't know what's up with Qt on macOS.

@QuLogic
Copy link
Member Author

QuLogic commented May 14, 2020

wx is fixed, but Qt is broken on macOS, as the figure appears to resize incorrectly, being 469 pixels after opening the window instead of 480 before.

@QuLogic QuLogic mentioned this pull request May 15, 2020
Closed
QuLogic added 5 commits May 15, 2020 22:16
@QuLogic
tk: Skip setting cursor if window is unavailable.
1241d59 
This is really optional, and fails in `Figure.savefig` if `plt.show()`
is called before it.
@QuLogic
TST: Smoke test saving with interactive backends.
2acbdd0 
We've had a report with Tk, and I think with WebAgg previously, so at
least do a simple test that it works.
@QuLogic
wx: Fix saving after window is closed.
b65d1e7 
We don't need to run `gui_repaint` if the window is gone, so just skip
it.
@QuLogic
wx: Fix saving to a BytesIO.
04b047b 
In classic wx, there were multiple functions for overloaded C++ methods,
but in phoenix there's just the one.
@QuLogic
TST: Skip backend savefig equality check on Qt5+macOS.
c0e7fbe 
It's currently failing because of incorrect resizing when the window is
opened.

matplotlib#7472
@QuLogic QuLogic changed the title tk: Skip setting cursor if window is unavailable tk/wx: Fix saving after the window is closed May 16, 2020
@QuLogic
Copy link
Member Author

QuLogic commented May 16, 2020

I decided to skip the equality check on Qt5+macOS, as I can't debug this myself, we have an open issue for it, and this PR is really about fixing saving itself (which is fine on that backend/OS).

@QuLogic QuLogic requested a review from anntzer May 16, 2020 02:18
@QuLogic QuLogic mentioned this pull request May 20, 2020
Closed
@tacaswell
Copy link
Member

Restarted the stalled CI, anyone can merge on green.

@QuLogic
Copy link
Member Author

QuLogic commented May 20, 2020

It seems like it might be stuck, but I don't know where. I've pushed a commit to debug this.

@tacaswell
Copy link
Member

well, the OSX jobs passed now, but the linux ones seem to be having infrastructure issues.

@tacaswell tacaswell merged commit 26fd914 into matplotlib:master May 20, 2020
@lumberbot-app
Copy link

lumberbot-app bot commented May 20, 2020

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

  1. Checkout backport branch and update it.
$ git checkout v3.2.x
$ git pull
  1. Cherry pick the first parent branch of the this PR on top of the older branch:
$ git cherry-pick -m1 26fd914ab0918c791e6dd87a9e11d0df07d6e56c
  1. You will likely have some merge/cherry-pick conflict here, fix them and commit:
$ git commit -am 'Backport PR #17391: tk/wx: Fix saving after the window is closed'
  1. Push to a named branch :
git push YOURFORK v3.2.x:auto-backport-of-pr-17391-on-v3.2.x
  1. Create a PR against branch v3.2.x, I would have named this PR:

"Backport PR #17391 on branch v3.2.x"

And apply the correct labels and milestones.

Congratulation you did some good work ! Hopefully your backport PR will be tested by the continuous integration and merged soon!

If these instruction are inaccurate, feel free to suggest an improvement.

@QuLogic QuLogic deleted the tk-cursor branch May 20, 2020 03:12
QuLogic pushed a commit to QuLogic/matplotlib that referenced this pull request May 20, 2020
@tacaswell @QuLogic
Backport PR matplotlib#17391: tk/wx: Fix saving after the window is c…
0bb70bf 
…losed
timhoffm added a commit that referenced this pull request May 20, 2020
@timhoffm
Merge pull request #17457 from QuLogic/auto-backport-of-pr-17391-on-v…
36d4576 
…3.2.x

Backport PR #17391 on branch v3.2.x
@QuLogic QuLogic mentioned this pull request Jun 15, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tacaswell tacaswell tacaswell approved these changes

@anntzer anntzer anntzer approved these changes

Assignees
No one assigned
Labels
GUI: tk GUI: wx
Projects
None yet
Milestone
v3.2.2
Development

Successfully merging this pull request may close these issues.

savefig error: tkinter.TclError: invalid command name "." plot save and plot show
3 participants
@QuLogic @tacaswell @anntzer