Skip to content

Make figure parameter optional when constructing canvases. #18746

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

Merged
merged 1 commit into from
Feb 5, 2021

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Oct 15, 2020

Auto-instantiating a Figure seems reasonable enough, and helps with GUI
builders. (The auto-instantiated figure must be a plain Figure(), not
a a pyplot-registered figure, both because most likely one should not
use pyplot when building GUIs, and also more prosaically because that
would run into a circular dependency (pyplot needs a canvas already
instantiated (actually a manager that holds a canvas) to store into its
global registry).

Closes #17349.

PR Summary

PR Checklist

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (run flake8 on changed files to check).
  • New features are documented, with examples if plot related.
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • Conforms to Matplotlib style conventions (install flake8-docstrings and pydocstyle<4 and run flake8 --docstring-convention=all).
  • New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).

@tacaswell tacaswell added this to the v3.4.0 milestone Oct 16, 2020
@tacaswell
Copy link
Member

We are going to redudently create and discard a FigureCanvasBase in the Figure init, are we worried about that?

I am 👍 on this, but it needs atleast one test that exercises it!

@anntzer
Copy link
Contributor Author

anntzer commented Oct 17, 2020

Initializing a FigureCanvasBase should be pretty cheap (it's mostly just setting a bunch of attributes, the most complex likely being creating the CallbackRegistry but as you know I'm trying to move that to the Figure instance anyways).
Test added.

@jklymak
Copy link
Member

jklymak commented Feb 4, 2021

linter needs to pass...

@anntzer
Copy link
Contributor Author

anntzer commented Feb 4, 2021

should be better now...

@QuLogic
Copy link
Member

QuLogic commented Feb 4, 2021

Seems like you didn't rebase on latest master? Docs should be fixed by now.

Auto-instantiating a Figure seems reasonable enough, and helps with GUI
builders.  (The auto-instantiated figure must be a plain Figure(), not
a a pyplot-registered figure, both because most likely one should not
use pyplot when building GUIs, and also more prosaically because that
would run into a circular dependency (pyplot needs a canvas already
instantiated (actually a manager that holds a canvas) to store into its
global registry).
@anntzer
Copy link
Contributor Author

anntzer commented Feb 4, 2021

retry.

@QuLogic QuLogic merged commit 4da4cf9 into matplotlib:master Feb 5, 2021
@anntzer anntzer deleted the autofigure branch February 5, 2021 07:45
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.

Closer-to-native signatures for native canvas and toolbar classes
4 participants