Skip to content

Make FigureCanvasWx ctor signature similar to FigureCanvasBase. #10606

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

Closed
wants to merge 1 commit into from

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Feb 26, 2018

Currently, FigureCanvasWx has a signature of (parent, id, figure),
unlike the Canvas classes of other backends, which only take figure as
argument. Change FigureCanvasWx to be more similar to other backends.

A similar fix will later be applied to FigureManagerWx to bring it in
line with other Manager classes.

The ultimate goal is to get rid of new_figure_manager and
new_figure_manager_given_figure in the backend definitions, which can
be replaced by return FigureManager(FigureCanvas(Figure(...)), num) and
return FigureManager(FigureCanvas(fig, num)) respectively.

Also deprecate the aliases FigureCanvasWx.frame and .tb (which are
named .window and .toolbar in all other backends).

PR Summary

PR Checklist

  • Has Pytest style unit tests
  • Code is PEP 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 this to the v3.0 milestone Feb 26, 2018
Currently, `FigureCanvasWx` has a signature of `(parent, id, figure)`,
unlike the Canvas classes of other backends, which only take `figure` as
argument.  Change `FigureCanvasWx` to be more similar to other backends.

A similar fix will later be applied to `FigureManagerWx` to bring it in
line with other Manager classes.

The ultimate goal is to get rid of `new_figure_manager` and
`new_figure_manager_given_figure` in the backend definitions, which can
be replaced by `return FigureManager(FigureCanvas(Figure(...)), num)` and
`return FigureManager(FigureCanvas(fig, num))` respectively.

Also deprecate the aliases `FigureCanvasWx.frame` and `.tb` (which are
named `.window` and `.toolbar` in all other backends).
@DietmarSchwertberger
Copy link
Contributor

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 call Reparent 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 modifying FigureCanvasWx to accept figure=None to have it a 'standard' wx signature of parent, 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 the ToolManager 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.

@anntzer
Copy link
Contributor Author

anntzer commented Feb 27, 2018

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.

@DietmarSchwertberger
Copy link
Contributor

I don't know about Gtk, but for Qt calling setParent 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.

@anntzer
Copy link
Contributor Author

anntzer commented Feb 27, 2018

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.

@DietmarSchwertberger
Copy link
Contributor

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.

@anntzer
Copy link
Contributor Author

anntzer commented Feb 27, 2018

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.

@WeatherGod
Copy link
Member

WeatherGod commented Feb 27, 2018 via email

@anntzer
Copy link
Contributor Author

anntzer commented Feb 27, 2018

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.)

@DietmarSchwertberger
Copy link
Contributor

DietmarSchwertberger commented Feb 27, 2018

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 call wx.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.

@anntzer
Copy link
Contributor Author

anntzer commented Feb 27, 2018

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.

@DietmarSchwertberger
Copy link
Contributor

DietmarSchwertberger commented Feb 28, 2018

I've just checked.
If I'm right, then the entry point into the wx backend is new_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 for id could be omitted, as suggested.
The tb/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 calling frame.SetToolBar and so GetToolBar 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 the toolbar property. (The ToolManager approach will probably be more useful.)

@anntzer
Copy link
Contributor Author

anntzer commented Feb 28, 2018

The entry points from mpl's pov into the backends are indeed new_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.

@DietmarSchwertberger
Copy link
Contributor

Would it be an option to modify FigureCanvasWx to accept variable arguments like FigureCanvasWx(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 modify FigureFrameWx to accept the required arguments?
Actually, for standalone windows the call to cls.FigureCanvas seems to be more a call to a factory function than to a Canvas 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 require FigureFrameWx being derived from FigureFrameCanvas or a common base class instead of holding a FigureCanvasWx reference in FigureFrameWx.canvas.
For an implementation from scratch, this would probably be the best option.

It's complicated to make things simple...

@tacaswell tacaswell modified the milestones: v3.0, v3.1 Jul 7, 2018
@jklymak jklymak modified the milestones: v3.1.0, v3.2.0 Feb 8, 2019
@timhoffm timhoffm modified the milestones: v3.2.0, v3.3.0 Aug 16, 2019
@anntzer
Copy link
Contributor Author

anntzer commented Sep 11, 2019

@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/2424 http://trac.wxwidgets.org/ticket/15237 https://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 the figure 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. with parent=..., id=...) -- but still it would be nice if all canvas subclasses could be created by pyplot just passing a figure parameter.

@anntzer
Copy link
Contributor Author

anntzer commented Mar 26, 2020

@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 :)

@RobinD42
Copy link
Contributor

In general using Reparent 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. Using DestroyLater is probably sufficient.

@anntzer
Copy link
Contributor Author

anntzer commented Mar 26, 2020

Great, thanks for your input!

@DietmarSchwertberger
Copy link
Contributor

DietmarSchwertberger commented Mar 28, 2020

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.

@anntzer
Copy link
Contributor Author

anntzer commented Mar 29, 2020

Thanks for providing a clarifying example. As I mentioned above, I was really unable to find any 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.

@DietmarSchwertberger
Copy link
Contributor

Thanks for providing a clarifying example. As I mentioned above, I was really unable to find any 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:

grafik

grafik

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...

@DietmarSchwertberger
Copy link
Contributor

Thanks for providing a clarifying example. As I mentioned above, I was really unable to find any 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:

grafik

grafik

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.

@anntzer
Copy link
Contributor Author

anntzer commented Mar 29, 2020

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, which does 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 a hint.

The fact that the author of wxPython considers Reparent()ing "okay" is also a hint 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 (not right 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.

@anntzer
Copy link
Contributor Author

anntzer commented Mar 29, 2020

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.py
index 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 call FigureManagerWx(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.

@anntzer
Copy link
Contributor Author

anntzer commented May 14, 2020

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.

@anntzer anntzer marked this pull request as draft May 14, 2020 14:48
@RobinD42
Copy link
Contributor

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 the arg0 is an instance of wx.Window, not wx.Object.

@anntzer
Copy link
Contributor Author

anntzer commented May 19, 2020

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 just return 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 expression FigureManager(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.

@tacaswell tacaswell modified the milestones: v3.3.0, v3.4.0 May 27, 2020
@QuLogic QuLogic modified the milestones: v3.4.0, v3.5.0 Jan 21, 2021
@QuLogic QuLogic modified the milestones: v3.5.0, v3.6.0 Aug 21, 2021
@timhoffm timhoffm modified the milestones: v3.6.0, unassigned Apr 30, 2022
@anntzer
Copy link
Contributor Author

anntzer commented Jul 5, 2022

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

@anntzer anntzer closed this Jul 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants