Skip to content

Destroy figures by manager instance, not by number. #16819

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 1 commit into from
Mar 19, 2020
Merged

Destroy figures by manager instance, not by number. #16819

merged 1 commit into from
Mar 19, 2020

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Mar 18, 2020

This avoids destroying the wrong figure when two managers share the same
number.

(I don't think this can be easily tested as what really matters is UI
closing?)

Closes #15203.

PR Summary

PR Checklist

  • Has Pytest style unit tests
  • Code is Flake 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

@anntzer
Destroy figures by manager instance, not by number.
9e6f112 
This avoids destroying the wrong figure when two managers share the same
number.

(I don't think this can be easily tested as what really matters is UI
closing?)
@anntzer anntzer added this to the v3.3.0 milestone Mar 18, 2020
timhoffm
timhoffm reviewed Mar 18, 2020
View reviewed changes
Copy link
Member

@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

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

Can we easily find out if there are multiple figures with the same number. If so, we should issue a warning on that.

@anntzer
Copy link
Contributor Author

anntzer commented Mar 18, 2020

Can we easily find out if there are multiple figures with the same number.

Not really in the general case (unless you add machinery to store all figure instances somewhere). We could do that in destroy() (if cls.figs[manager.num] != manager) but do we really want to? For example one could just be creating their own managers independently of pyplot and not care about numbers at all, but Gcf.destroy will still get involved when closing figures because of the way things are set up...

@tacaswell
Copy link
Member

The wx on osx failure might be real (a hanging subprocess). Restarted, lets see if it passes the second time..

tacaswell
tacaswell approved these changes Mar 18, 2020
View reviewed changes
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.

modulo CI passing.

@timhoffm timhoffm merged commit 66c3d4a into matplotlib:master Mar 19, 2020
@anntzer anntzer deleted the destroymanager branch March 19, 2020 09:14
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

@timhoffm timhoffm timhoffm approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v3.3.0
Development

Successfully merging this pull request may close these issues.

Closing figures is done by number
3 participants
@anntzer @tacaswell @timhoffm