Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Fix wx canvas type injection.#10428
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
NavigationToolbar2Wx should use `type(self.canvas)` when instantiatingthe canvas to make it work with all of wx, wxagg, wxcairo (instantiatingthe corresponding canvas class in each case). Conversely, FigureFrameWxshould explicitly use FigureCanvasWx; other backends (wxagg, wxcairo)override the method accordingly.I got these inverted in my previous PR and this breaks the wx backend.
At the moment the As you wrote, in 'master'
In
What is missing here is an assignment to
In So the options are:
I don't have a strong preference for either of the options, but a small preference for the derived classes as I prefer explicit over implicit. If you prefer the single What is the situation about deprecating the |
Removal of the Toolbar alias came in in#9938 and I would support either keeping it out (and fixing the example), or, if we want to have it back, it should be present for all backends (as a "standard backend interface"). I would prefer keeping a single class to make it easier to write renderer-independent code. (And yes, having a single alias for Toolbar would also help that purpose, but again in that case it seems preferable to let all relevant backends have it.) |
For 3.0 the Toolbar alias certainly should be removed. I've seen that the The |
I agree strongly with you that native widgets should be preferred (see e.g.#8683), but see also#7696 for arguments against it. Can you fix the examples in a separate PR (or list the examples that need to be fixed)? SubplotToolWx seems to essentially be a copy of the body of configure_subplots. I'm fine with deprecating it or making it private (in general I think all these are backend implementation details...). |
I've just submitted PR 10518 to fix the inconsistencies. With this, 10518 and 10429 we're probably done for 2.2. Regarding native vs. Matplotlib widgets: I think for Subplot it would make sense to implement it in Matplotlib, but not as separate window. The best would be to have the plot borders draggable... |
Backport PR#10428 on branch v2.2.x
NavigationToolbar2Wx should use
type(self.canvas)
when instantiatingthe canvas to make it work with all of wx, wxagg, wxcairo (instantiating
the corresponding canvas class in each case). Conversely, FigureFrameWx
should explicitly use FigureCanvasWx; other backends (wxagg, wxcairo)
override the method accordingly.
I got these inverted in my previous PR and this breaks the wx backend.
PR Summary
PR Checklist