Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Remove custom backend_nbagg.show(), putting logic in manager show.#23100
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
The _Backend class machinery can autogenerate a backend_module.show()function, which (in short) calls manager.show() on all managers and thencalls the optionally-provided mainloop() function.backend_nbagg currently completely bypasses this autogeneration becauseit wants to additionally fiddle with callbacks and Gcf (it doesn't needany mainloop()), but that logic can just as well go toFigureManagerNbAgg.show() instead. This way it benefits from thedefault-generated show; moreover, it seems a bit strange that figuresshown with manager.show() (instead of plt.show()) do indeed getdisplayed, but just don't have their callbacks/Gcf-ness adjusted.This is also related to the move towards putting show()-generatinginformation on the canvas class for inheritability.
| # one. Disable this behaviour, as it results in figures being put as | ||
| # the active figure after they have been shown, even in non-interactive | ||
| # mode. | ||
| if hasattr(self, '_cidgcf'): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Is there any way to add tests to argue that this is working as expected?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Actually, there was previously no test coverage for the corresponding old code paths (as can be checked e.g. viapytest --cov --pyargs matplotlib.tests.test_backend_nbagg && coverage html: the entire show() function was untested), so I'll ask for an out there and just point out that I simply moved some code around (also, my understanding of nbagg is on the weaker side...).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
fair.
The _Backend class machinery can autogenerate a backend_module.show()
function, which (in short) calls manager.show() on all managers and then
calls the optionally-provided mainloop() function.
backend_nbagg currently completely bypasses this autogeneration because
it wants to additionally fiddle with callbacks and Gcf (it doesn't need
any mainloop()), but that logic can just as well go to
FigureManagerNbAgg.show() instead. This way it benefits from the
default-generated show; moreover, it seems a bit strange that figures
shown with manager.show() (instead of plt.show()) do indeed get
displayed, but just don't have their callbacks/Gcf-ness adjusted.
This is also related to the move towards putting show()-generating
information on the canvas class for inheritability (#23090).
PR Summary
PR Checklist
Tests and Styling
pytestpasses).flake8-docstringsand runflake8 --docstring-convention=all).Documentation
doc/users/next_whats_new/(follow instructions in README.rst there).doc/api/next_api_changes/(follow instructions in README.rst there).