Skip to content

Update backend_nbagg for removal of Gcf._activeQue. #15901

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
Dec 17, 2019
Merged

Update backend_nbagg for removal of Gcf._activeQue. #15901

merged 1 commit into from
Dec 17, 2019

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Dec 10, 2019

Looks like nbagg was (ab)using _activeQue to have "pyplot-managed
figures that can never be gcf()" (not really sure why? this was here
ever since nbagg was first merged in #3008). Now that _activeQue has been
merged into figs this is no longer possible, but I guess the patch is
the closest in semantics?

Sorry I missed that in #13581. If that's too much of a problem we can revert #13581, but intentionally making Gcf.figs and Gcf._activeQue go out of sync is... a bit too clever?

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 anntzer added the Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. label Dec 10, 2019
@anntzer anntzer added this to the v3.2.0 milestone Dec 10, 2019
@timhoffm
Copy link
Member

What is _activeQue after all? I do not find that anywhere else in the code. That would be sort of ok, because the PR removes all it's usages in backend_nbagg.py. However the PR message and the comment in l.262ff seems to indicate that it's still a valid attribute. Am I missing something?

@anntzer
Copy link
Contributor Author

anntzer commented Dec 15, 2019

Before #13581 there was both Gcf.figs and Gcf._activeQue. figs was a mapping of manager numbers to manager instances, and _activeQue a list of managers such that the active manager was the one at the end. I thought that they were always updated together and thus could just combine them together (making figs an OrderedDict), but I missed the fact that nbagg is the sole place which removes something from activeQue (making it nonactivable) without removing it from figs.

timhoffm
timhoffm reviewed Dec 16, 2019
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.

Comment in line 262ff should be updated. There is no activeQue anymore.

@anntzer
Update backend_nbagg for removal of Gcf._activeQue.
2692cf8 
Looks like nbagg was (ab)using _activeQue to have "pyplot-managed
figures that can never be gcf()" (not really sure why? this was here
ever since nbagg was first merged in).  Now that _activeQue has been
merged into figs this is no longer possible, but I *guess* the patch is
the closest in semantics?
@anntzer
Copy link
Contributor Author

anntzer commented Dec 16, 2019

done

timhoffm
timhoffm approved these changes Dec 16, 2019
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.

Anybody can merge after CI pass.

@anntzer anntzer merged commit bf584bd into matplotlib:master Dec 17, 2019
@anntzer anntzer deleted the nbactive branch December 17, 2019 09:36
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Dec 17, 2019
@anntzer
Backport PR matplotlib#15901: Update backend_nbagg for removal of Gcf…
0965958 
…._activeQue.
@meeseeksmachine meeseeksmachine mentioned this pull request Dec 17, 2019
Merged
timhoffm added a commit that referenced this pull request Dec 17, 2019
@timhoffm
Merge pull request #15956 from meeseeksmachine/auto-backport-of-pr-15…
f725924 
…901-on-v3.2.x

Backport PR #15901 on branch v3.2.x (Update backend_nbagg for removal of Gcf._activeQue.)
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
Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Projects
None yet
Milestone
v3.2.0
Development

Successfully merging this pull request may close these issues.
3 participants
@anntzer @timhoffm @tacaswell