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

Cleanup _pylab_helpers. #13581

Merged
merged 1 commit into from
Sep 18, 2019
Merged

Cleanup _pylab_helpers. #13581

merged 1 commit into from
Sep 18, 2019

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Mar 3, 2019

_activeQue + figs can be combined into a single OrderedDict.

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

"""Destroy figure *fig*."""
canvas = getattr(fig, "canvas", None)
manager = getattr(canvas, "manager", None)
num = getattr(manager, "num", None)
Copy link
Member

Choose a reason for hiding this comment

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

Is this always equivalent, since it seems to be working in reverse?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This basically gets fig.canvas.manager.num with the provision that each of these can be None. If this num is not registered in the figs dict, destroy will catch that as it first checks cls.has_fignum(num).
It's true that this may cause problems if two managers share the same num, but then the num->manager mapping is already invalid (set_active will already have overwritten the entry in the dict) so I'm not too worried about that.

*_activeQue*:
list of *managers*, with active one at the end

Singleton to handle a set of "numbered" figures and managers, and keep
Copy link
Member

Choose a reason for hiding this comment

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

I find the whole docstring a bit unclear. It seems to assume prior knowledge on figures, figure managers and their relation. Would this be a more complete description?

    Singleton to maintain the relation between figures and their managers, and
    keep track of and "active" figure and manager.

    When created via pyplot, a figure is associated with its own figure
    manager, that handles the interaction between the figure and the backend.

    This association is established via a unique identifier, called
    *figure number* or *manager number*. While figure numbers are often actual
    numbers, they can actually be any hashable value. The number is attached
    to the figure via the `.Figure.number` attribute; and it's mapped to a
    manager in the `.Gcf.figs` dictionary.

    This class is never instantiated; it consists of an `OrderedDict` mapping
    figure/manager numbers to managers, and a set of class methods that
    manipulate this `OrderedDict`.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually the number is an attribute of the manager (which is itself an attribute of the canvas), but other than that, looks good. Will incorporate a slightly edited version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pushed edited version. (Note that the identifier does not create the association, it's just used for bookkeeping.)

_activeQue + figs can be combined into a single OrderedDict.
@QuLogic
Copy link
Member

QuLogic commented Sep 18, 2019

Two approvals...

@QuLogic QuLogic added this to the v3.3.0 milestone Sep 18, 2019
@QuLogic QuLogic merged commit 1285c42 into matplotlib:master Sep 18, 2019
@anntzer anntzer deleted the pylab_helpers branch September 18, 2019 08:41
QuLogic added a commit to QuLogic/matplotlib that referenced this pull request Aug 5, 2020
QuLogic added a commit to QuLogic/matplotlib that referenced this pull request Aug 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants