I have not yet looked at the details.
My interest in Matplotlib is more for embedding than for using the standalone windows.
I'm not really familiar with the inner mechanisms of the standalone windows.
From a quick look, my opinion is that the changes make embedding even harder than now.
Setting the parent to toplevel and having to callReparent might be possible most of the time, but is so much against any wx design principle, that I would call it a no-go. It can have (platform dependent) side effects as well.
Before, I was even thinking about modifyingFigureCanvasWx to acceptfigure=None to have it a 'standard' wx signature ofparent, id=-1. Actually, providing a figure has only the minor added value of being able to specify the size in inches instead of pixels.

The flow for standalone windows:

• import matplotlib or matplotlib.pyplot
• create figure
• create plots etc.
• show, i.e. create a frame with a canvas, draw to a bitmap and blit the bitmap to the screen
• any of these in any combination: re-use, close, re-create

The flow for embedding:

• import wx and matplotlib.backends.backend_wxagg.FigureCanvasWxAgg
• create frame or other toplevel window
• create and embed the canvas together with sizers and other controls
• fill it with plots
• re-use it until the program terminates; add more canvas instances if required

Maybe it would be the best to separate the canvas class for standalone windows and for embedding if it helps to keep the standalone code clean.
Actually, an embeddable canvas class and theToolManager would be the only ones that are required by embedding code. For use of the standalone windows, the user will usually not import the backend class himself so the external API does not really matter.

The model I would like to stick to is the one of the Qt and Gtk backends, where the FigureManager class is passed in a canvas and responsible for instantiating a window, then reparents the window into the canvas. The benefit is the one mentioned above: making the setup code as similar as possible between backends.

If that's so much against "normal" wx behavior, then perhaps separate classes for embedding and for standalone use could be a solution, indeed.

I don't know about Gtk, but for Qt callingsetParent is not the standard design pattern either. The only difference between Qt and wx is that Qt does not have the id argument. (For wx it is not widely used either.)

Really, re-parenting is anything else than standard. Things should be kept simple and straightforward (the Qt embedding example could be much more concise to make things clear).

Btw.GetTopWindow on wx may return any of the toplevel windows if there are multiple.

For Qt calling setParent explicitly is non-standard,but you can create widgets and windows without any parent and add them to a container; they will then be directly reparented to the containing widget/window (which is what we do via setCentralWidget). As far as I can tell this is fairly standard.

The use of GetTopWindow in wx is because I could not find a way to create a window/widget on wx without any parent (like for Qt), so that was the only thing I could parent to.

IMHO the Qt example should be updated accordingly.

A wx canvas should follow the wx requirements. Reparenting is not a good idea. There's absolutely no point for FigureCanvasWx not accepting or requiring a parent as argument when being embedded. The id argument is not really useful with Python, but any other wx widget would accept it.
Also with interactive use, the canvas should be created on the correct frame if there is more than one.

The qt example already creates canvases without parents:https://github.com/matplotlib/matplotlib/blob/master/examples/user_interfaces/embedding_in_qt_sgskip.py#L34

The point is that with Qt, you more or less always need to 1) create the widget and 2) insert it into a container. Because step 2 will reparent the widget to the container, there's no need to indicate the parent in step 1). See e.g. the example given in the "official" docs:http://doc.qt.io/qt-5/qhboxlayout.html#details

Again if that's so unnatural for wx then we should think of some other design.

The PR proposed something with a normal deprecation warning and indicated how to specify the parent (via Reparent), so@WeatherGod's points are taken into account. (But that doesn't help with the un-wx-ishness pointed out by@DietmarSchwertberger.)

Your Qt example looks much clearer than what I've found on the current documentation page (https://matplotlib.org/examples/user_interfaces/embedding_in_qt5.html)

For wx the widget will actually be created on the parent window. It could (in theory) already receive events which would then propagate to the wrong parent and if that is deleted at the same time a core dump could occur. Sure, most of the time there will be no problems, but I would very much prefer to keep the arguments and avoid re-parenting.

For how a standalone window is created, I can't tell much, but also there the callwx.Panel.__init__(self, parent, id, size=wx.Size(w, h)) should use the final parent already.

It's a bit non-standard that FigureCanvasWx does not accept a size or position argument, but I would say that this is acceptable if it results in more standardization within Matplotlib. If FigureCanvasWx was written from scratch, it would probably be the best to use *args and **kwargs and accept e.g. size instead of figure.

https://matplotlib.org/devdocs/gallery/user_interfaces/embedding_in_qt_sgskip.html for the new example.

If you consider that even for the standalone case we should not rely (even purely internally) on reparenting then there's probably nothing that can be done to harmonize the backends.

I've just checked.
If I'm right, then the entry point into the wx backend isnew_figure_manager_given_figure(cls, num, figure).
So there the parent is not required and everything is internal to the wx backend. So, the API for embedding is different between the backends, but for standalone windows it does not matter. For me that looks fine.

The-1 arguments forid could be omitted, as suggested.
Thetb/toolbar/frame/window harmonization looks good. It makes things easier.

I'm not sure about the toolbar handling in general. Currently the toolbar is added to the frame sizer instead of callingframe.SetToolBar and soGetToolBar may not return the expected matplotlib toolbar.
Adding to the sizer is also quite non-standard and causes problems when wx calculates the required size.
See my last comment on#2716
For rev. 3.0 it would probably be best to switch to standard wx toolbars at the top of the window.
For embedding, I don't see a good use for thetoolbar property. (The ToolManager approach will probably be more useful.)

The entry points from mpl's pov into the backends are indeednew_figure_manager and new_figure_manager_given_figure(depending on the specific use case). For Qt and Gtk3, these essentially come down toreturn FigureManager(canvas, num)andreturn FigureManager(FigureCanvas(figure), num)so the hope of this PR was to get rid ofnew_figure_managerandnew_figure_manager_given_figure` by directly inlining the calls to the constructors (by fixing the constructors as needed to make this possible). I understand that this may not be possible to do in a "normal" wx setting.

Would it be an option to modifyFigureCanvasWx to accept variable arguments likeFigureCanvasWx(parent_or_figure=None, id=-1, figure=None, size=None). If the first argument is a figure, it would create it's own Frame.

The use cases:
FigureCanvasWx(parent, -1, figure): embedded
FigureCanvasWx(parent): embedded, create default figure
FigureCanvasWx(parent, size=(640,480)): embedded, create default figure with given size
FigureCanvasWx(figure): standalone, create canvas and frame with size specified from figure
FigureCanvasWx(): standalone, default figure and size; e.g. used in a script

Alternatively, would it be possible to replace
_BackendWx.FigureCanvas = FigureCanvasWx
with
FigureCanvas = FigureFrameWx and modifyFigureFrameWx to accept the required arguments?
Actually, for standalone windows the call tocls.FigureCanvas seems to be more a call to a factory function than to aCanvas class.
FigureWindow might have been a more appropriate name.
Unfortuntely Frame and Canvas have different methods and attributes, so this approach is not too straightforward. It would probably requireFigureFrameWx being derived fromFigureFrameCanvas or a common base class instead of holding aFigureCanvasWx reference inFigureFrameWx.canvas.
For an implementation from scratch, this would probably be the best option.

It's complicated to make things simple...

@DietmarSchwertberger Looking at this again I am a bit confused. Can you point to any docs online that argue against Reparent()ing widgets? Googling only gave me a couple of (fairly old) bug reports with Reparent() which were basically all resolved by patches to wxWidgets, and I didn't see a single instance where the wxWidgets maintainers said "don't use Reparent()" (e.g.https://trac.wxwidgets.org/ticket/2424http://trac.wxwidgets.org/ticket/15237https://trac.wxwidgets.org/ticket/17980). Of course I may have missed other references...

On the other hand one could indeed make FigureCanvas nicer to use with wxWidgets (or other toolkits when embedding, in fact) by 1) making thefigure parameter optional (creating a new figure if it is passed), and 2) allowing arbitrary keyword arguments, which all get passed to the GUI widget constructor (so you could call it e.g. withparent=..., id=...) -- but still it would be nice if all canvas subclasses could be created by pyplot just passing afigure parameter.

@RobinD42 Any chance you could comment on this issue? Namely, is it OK to reparent wx widgets? This would help making the API more consistent across backends, as all other backends create their canvases with no parent and later reparent it as needed.
Thanks for your input :)

In general usingReparent should be okay, the AUI package, for example, depends on the ability to reparent. Although as@DietmarSchwertberger mentioned there could be some possible (but probably rare and/or low-impact) glitches. For example, some widget types will inherit some attributes (like colors and font) from their parent when the widget is created, and there are some situations where those attributes could vary based on the type of the parent. (Such as Notebook pages vs. child of a Frame).

Another thing to watch out for is if the temporary parent is to be destroyed after the reparenting, it should be done only if there are not any pending events for the old parent or the child that was moved. UsingDestroyLater is probably sufficient.

Great, thanks for your input!

To make it clear:

Code as of now:

figure = self.matplotlib_figure = Figure()self.matplotlib_canvas = FigureCanvas(self, wx.ID_ANY, figure)sizer.Add(self.matplotlib_canvas, 1, wx.ALL | wx.EXPAND, 3)

This PR will often require code like this:

topwindow = wx.GetApp().GetTopWindow()topwindow.Freeze()try:    self.matplotlib_figure = Figure()    self.matplotlib_canvas = FigureCanvas(figure)    self.matplotlib_canvas.Reparent(self)finally:    topwindow.Thaw()sizer.Add(self.matplotlib_canvas, 1, wx.ALL | wx.EXPAND, 3)

The Freeze/Thaw calls are required to avoid flickering. See attached file which will create flickering, at least on Windows.
matplotlib_example_reparent.py.txt

IMHO that's a big mess. Please don't do it. It does not have any advantage. There's no cross-toolkit code anyway when it comes to widget creation.

An improvement would look like that, i.e. without the figure argument:

self.matplotlib_canvas = FigureCanvas(self)sizer.Add(self.matplotlib_canvas, 1, wx.ALL | wx.EXPAND, 3)

The figure argument might make sense in an interactive environment, but not for embedding where plotting often starts a long time after creating the canvas.

Thanks for providing a clarifying example. As I mentioned above, I was really unable to findany reference anywhere online warning against Reparent()ing in wx; at least now that gives me something to try (except that I don't have graphical access to a Windows machine these days due to the lockdown, but I'll try your code once I can). I'm also curious whether this flickering is/should be considered a bug? At least it doesn't occur locally (Arch+wxGTK). Also I guess I wonder how things work with AUI, given that@RobinD42 says it depends on Reparent()ing?

IMHO that's a big mess. Please don't do it.

My goal is not to do things that people don't like, I am genuinely just trying to keep things as simple as possible, but not more.

It does not have any advantage. There's no cross-toolkit code anyway when it comes to widget creation.

Qt and GTK and Tk all share (basically) the same canvas and manager creation path (FigureManager(FigureCanvas(Figure(...)))) so it was not obvious to me why wx could not do the same.

Thanks for providing a clarifying example. As I mentioned above, I was really unable to findany reference anywhere online warning against Reparent()ing in wx; at least now that gives me

Well, it's pefectly possible, but nobody would do this usually, so why should there be a warning?
I'm using Reparent with wxGlade, but certainly not for normal GUI programs.

Btw. the Freeze/Thaw does not fix flickering in this example. This might be a bug. Usually, Freeze/Thaw is working, but on several occasions I had to remove it as it made things unstable.

Also I guess I wonder how things work with AUI, given that@RobinD42 says it depends on Reparent()ing?

AUI is for dragging e.g. palettes or subwindows around, so the window is moved from one to another. The flickering takes place where the user drags a window. So it's not an issue here.

My goal is not to do things that people don't like, I am genuinely just trying to keep things as simple as possible, but not more.

This change does not make anything simpler. A signature compatible to normal wx widgets would be a simplification. Currently always glue code in the GUI creation is required which should not be required or better with the application logic (figure creation).

E.g. here are wxGlade screenshots of how to embed a matplotlib canvas:

Qt and GTK and Tk all share (basically) the same canvas and manager creation path (FigureManager(FigureCanvas(Figure(...)))) so it was not obvious to me why wx could not do the same.

And all steps before and after are toolkit specific...

Thanks for providing a clarifying example. As I mentioned above, I was really unable to findany reference anywhere online warning against Reparent()ing in wx; at least now that gives me

Well, it's pefectly possible, but nobody would do this usually, so why should there be a warning?
I'm using Reparent with wxGlade when windows are edited, but certainly not for normal GUI programs.

Btw. the Freeze/Thaw does not fix flickering in this example. This might be a bug. Usually, Freeze/Thaw is working, but on several occasions I had to remove it as it made things unstable.

Also I guess I wonder how things work with AUI, given that@RobinD42 says it depends on Reparent()ing?

AUI is for dragging e.g. palettes or subwindows around, so the window is moved from one to another. The flickering takes place where the user drags a window. So it's not an issue here.

My goal is not to do things that people don't like, I am genuinely just trying to keep things as simple as possible, but not more.

This change does not make anything simpler. A signature compatible to normal wx widgets would be a simplification. Currently always glue code in the GUI creation is required which should not be required or better with the application logic (figure creation).

E.g. here are wxGlade screenshots of how to embed a matplotlib canvas:

Qt and GTK and Tk all share (basically) the same canvas and manager creation path (FigureManager(FigureCanvas(Figure(...)))) so it was not obvious to me why wx could not do the same.
The pattern is completely different: with the Qt approach you don't create the widget on the screen as without specifying a parent where would it be created? With wx you immediately create it. Actually, creating a non-toplevel widget without a parent is not really useful.

And also, all steps before and after are toolkit specific...

Sorry, I'm not willing to spend more time on issues which are obvious. I have better things to do.

Well, it's pefectly possible, but nobody would do this usually, so why should there be a warning?
I'm using Reparent with wxGlade when windows are edited, but certainly not for normal GUI programs.

If you look at the docs for wxWindow::Reparent() (https://docs.wxwidgets.org/3.0/classwx_window.html#a7977b749284e65aecfed2ce146799cb9) it doesn't say anything. Compare with e.g. the SafelyProcessEvent doc immediately below, whichdoes warn that that function shouldn't be used. Or even with the corresponding function in Qt, QWidget::setParent() (https://doc.qt.io/qt-5/qwidget.html#setParent), which warns that it's unlikely the user needs to call this function.

Certainly the lack of warning doesn't necessarily mean that a function is safe, but it is a strong hint in that direction, especially when other functions are explicitly documented as unsafe/unneeded. The fact that nowhere else online could I find a warning against reparenting is also a hint in that direction -- again, not a proof, but ahint.

The fact that the author of wxPython considers Reparent()ing "okay" is also ahint in the same direction.

Btw. the Freeze/Thaw does not fix flickering in this example. This might be a bug. Usually, Freeze/Thaw is working, but on several occasions I had to remove it as it made things unstable.

Again, I thank you for providing that example, which I will try on Windows ASAP (notright now for the practical reason explained above, which is beyond my control...). Before that, I simply had no practical example of case where Reparent()ing would be bad.

E.g. here are wxGlade screenshots of how to embed a matplotlib canvas:

Again, the practical examples you give are useful. I have written a fair amount of Qt GUIs, but not used graphical GUI builders (whether for Qt or for wx), so it isn't obvious to me what they require.

And also, all steps before and after are toolkit specific...

I would like to believe that one of Matplotlib's strong points is its ability to push back GUI-specific code as far as possible. Obviously a barrier has to be drawn somewhere; the only question is where.

Sorry, I'm not willing to spend more time on issues which are obvious. I have better things to do.

I can only hope that you realize I am making a genuine effort in trying to understand why certain patterns would not be suitable for wx, when most of the evidence available points in the other direction.

Also, trying to keep this discussion constructive... an alternative API which may work for all cases and not rely at all on reparenting may be along the lines of (untested)

diff --git i/lib/matplotlib/backends/backend_wx.py w/lib/matplotlib/backends/backend_wx.pyindex 9d4b773e7..8ae427b06 100644--- i/lib/matplotlib/backends/backend_wx.py+++ w/lib/matplotlib/backends/backend_wx.py@@ -7,6 +7,7 @@ Hunter (jdhunter@ace.bsd.uchicago.edu). Copyright (C) Jeremy O'Donoghue & John Hunter, 2003-4. """+from collections import namedtuple import logging import math import pathlib@@ -436,6 +437,9 @@ class GraphicsContextWx(GraphicsContextBase):                 alpha=int(a))+_DelayedFigureCanvasWx = namedtuple("_DelayedFigureCanvasWx", "cls figure")++ class _FigureCanvasWxBase(FigureCanvasBase, wx.Panel):     """     The FigureCanvas contains the figure and does event handling.@@ -508,6 +512,13 @@ class _FigureCanvasWxBase(FigureCanvasBase, wx.Panel):         wx.WXK_NUMPAD_DELETE: 'delete',     }+    def __new__(cls, arg0, *args, **kwargs):+        if isinstance(arg0, wx.Object):+            return super().__new__(cls, *args, **kwargs)+        else:+            assert isinstance(arg0, Figure) and not args and not kwargs+            return _DelayedFigureCanvasWx(cls, arg0)+     def __init__(self, parent, id, figure):         """         Initialize a FigureWx instance.@@ -1040,6 +1051,8 @@ class FigureManagerWx(FigureManagerBase):     """      def __init__(self, canvas, num, frame):+        if isinstance(canvas, _DelayedFigureCanvasWx):+            canvas = canvas.cls(self, -1, canvas.figure)         _log.debug("%s - __init__()", type(self))         FigureManagerBase.__init__(self, canvas, num)         self.frame = frame

Now you can still construct FigureCanvasWx using the wx-standard signature (or did you want to make other changes to it? e.g. support attaching the Figure instance later?), but internally Matplotlib can still callFigureManagerWx(FigureCanvasWx(figure)) -- it's just that in that case, FigureCanvasWx will return a placeholder object which will be turned into a real canvas by the FigureManager constructor.

I finally had a chance to test@DietmarSchwertberger's example on Windows and can confirm the flickering problem there, so I agree that the approach here is not directly acceptable.@RobinD42 do you have any suggestions about this?
See also the suggested alternative API in the post immediately above.

IIUC, if the change allows the developer a way to create the UI object immediately without needing to reparent it later, then I think it sounds like a good idea and should satisfy@DietmarSchwertberger's concerns. One change I suggest, however, is to check if thearg0 is an instance ofwx.Window, notwx.Object.

The point is to allow Matplotlib to create some kind of canvas-like object (which is actually just a placeholder), which can be passed to the figure manager (which will actually be instantiating the correct canvas).

The main reason to do so is that right now it is rather tricky to write new backends. For example, in mplcairo, I have on the one hand FigureCanvas = FigureCanvasWxCairo (which inherits from the base FigureCanvasWx for the UI parts and from my own FigureCanvasCairo for the rendering parts) -- that's fine, that's the point of the backend --, but I additionally need to write a couple of factory functions (new_figure_manager(), new_figure_manager_given_figure()) which abstract out how to create a correct FigureManager for that backend: for most backends these are justreturn FigureManagerFoo(FigureCanvasFoo(Figure())), but because FigureCanvasWx has a different signature, this is different too. With the change here (and some more...), we could change the "write a new backend API" to just "you need to provide a FigureCanvas class and a FigureManager class such that the expressionFigureManager(FigureCanvas(Figure())) returns the right thing", and now mplcairo.wx would just need to define FigureCanvasWxCairo and could just straightforwardly import FigureManagerWx from matplotlib.backends.backend_wx.

Superseded by#22925 (see discussion re: API design in linked issues).