Skip to content

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

Merged
timhoffm merged 1 commit intomatplotlib:masterfrom
anntzer:destroymanager
Mar 19, 2020
Merged

Destroy figures by manager instance, not by number.#16819
timhoffm merged 1 commit intomatplotlib:masterfrom
anntzer:destroymanager

Conversation

@anntzer
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member

tacaswell commented Mar 18, 2020

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
Copy Markdown
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