Skip to content

[Bug]: Tool Manager example broken #22088

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
StefRe opened this issue Jan 3, 2022 · 4 comments · Fixed by #22743
Closed

[Bug]: Tool Manager example broken #22088

StefRe opened this issue Jan 3, 2022 · 4 comments · Fixed by #22743
Labels
GUI: Qt status: has patch patch suggested, PR still needed
Milestone

Comments

@StefRe
Copy link
Contributor

StefRe commented Jan 3, 2022

Bug summary

The Tool Manager example doesn't work.

Code for reproduction

python toolmanager_sgskip.py

Actual outcome

Traceback (most recent call last):
  File "C:\Programs\Python38\lib\site-packages\matplotlib\backends\backend_qt.py", line 930, in handler
    self.trigger_tool(name)
  File "C:\Programs\Python38\lib\site-packages\matplotlib\backend_bases.py", line 3391, in trigger_tool
    self.toolmanager.trigger_tool(name, sender=self)
  File "C:\Programs\Python38\lib\site-packages\matplotlib\backend_managers.py", line 379, in trigger_tool
    self._trigger_tool(name, sender, canvasevent, data)
  File "C:\Programs\Python38\lib\site-packages\matplotlib\backend_managers.py", line 394, in _trigger_tool
    tool.trigger(sender, canvasevent, data)
  File "C:\Programs\Python38\lib\site-packages\matplotlib\backends\backend_qt.py", line 970, in trigger
    NavigationToolbar2QT.configure_subplots(
  File "C:\Programs\Python38\lib\site-packages\matplotlib\backends\backend_qt.py", line 753, in configure_subplots
    if self._subplot_dialog is None:
AttributeError: 'types.SimpleNamespace' object has no attribute '_subplot_dialog'

Expected outcome

  • tools being added
  • no crash

Additional information

No response

Operating system

No response

Matplotlib Version

3.5.1

Matplotlib Backend

No response

Python version

No response

Jupyter version

No response

Installation

No response

@anntzer
Copy link
Contributor

anntzer commented Jan 3, 2022

Good catch, this is likely a regression from #21681 (3.5.1, so labeling the fix as 3.5.2), something like

diff --git i/lib/matplotlib/backends/backend_qt.py w/lib/matplotlib/backends/backend_qt.py
index c55937a019..1e0856adec 100644
--- i/lib/matplotlib/backends/backend_qt.py
+++ w/lib/matplotlib/backends/backend_qt.py
@@ -3,6 +3,7 @@ import operator
 import os
 import sys
 import traceback
+from weakref import WeakKeyDictionary
 
 import matplotlib as mpl
 from matplotlib import _api, backend_tools, cbook
@@ -646,7 +647,6 @@ class NavigationToolbar2QT(NavigationToolbar2, QtWidgets.QToolBar):
 
         self.coordinates = coordinates
         self._actions = {}  # mapping of toolitem method names to QActions.
-        self._subplot_dialog = None
 
         for text, tooltip_text, image_file, callback in self.toolitems:
             if text is None:
@@ -755,14 +755,11 @@ class NavigationToolbar2QT(NavigationToolbar2, QtWidgets.QToolBar):
         self.canvas.drawRectangle(None)
 
     def configure_subplots(self):
-        if self._subplot_dialog is None:
-            self._subplot_dialog = SubplotToolQt(
-                self.canvas.figure, self.canvas.parent())
-            self.canvas.mpl_connect(
-                "close_event", lambda e: self._subplot_dialog.reject())
-        self._subplot_dialog.update_from_current_subplotpars()
-        self._subplot_dialog.show()
-        return self._subplot_dialog
+        subplot_dialog = SubplotToolQt(
+            self.canvas.figure, self.canvas.parent())
+        subplot_dialog.update_from_current_subplotpars()
+        subplot_dialog.show()
+        return subplot_dialog
 
     def save_figure(self, *args):
         filetypes = self.canvas.get_supported_filetypes_grouped()
@@ -805,8 +802,13 @@ class NavigationToolbar2QT(NavigationToolbar2, QtWidgets.QToolBar):
 
 
 class SubplotToolQt(QtWidgets.QDialog):
-    def __init__(self, targetfig, parent):
-        super().__init__()
+    _keep_alive = WeakKeyDictionary()
+
+    def __new__(cls, targetfig, parent):
+        if targetfig in cls._keep_alive:
+            return cls._keep_alive[targetfig]
+        self = cls._keep_alive[targetfig] = super().__new__(cls, parent)
+        QtWidgets.QDialog.__init__(self)
         self.setWindowIcon(QtGui.QIcon(
             str(cbook._get_data_path("images/matplotlib.png"))))
         self.setObjectName("SubplotTool")
@@ -848,6 +850,11 @@ class SubplotToolQt(QtWidgets.QDialog):
         self._defaults = {}
         self._export_values_dialog = None
         self.update_from_current_subplotpars()
+        targetfig.canvas.mpl_connect("close_event", lambda e: self.reject())
+        return self
+
+    def __init__(self, targetfig, parent):
+        pass
 
     def update_from_current_subplotpars(self):
         self._defaults = {spinbox: getattr(self._figure.subplotpars, name)

(moving instance tracking to SubplotToolQt itself) would be one way to fix this.

@anntzer anntzer added the GUI: Qt label Jan 3, 2022
@anntzer anntzer added this to the v3.5.2 milestone Jan 3, 2022
@StefRe
Copy link
Contributor Author

StefRe commented Jan 3, 2022

@anntzer Thanks for the super fast reply 👍 .

When using the Tk backend (added plt.matplotlib.use('tkagg')) there's another issue: the "Configure subplots" dialog works at first but then freezes: open the dialog and move some sliders - after a short while the dialog freezes (python 3.8.2 64bit Windows). I guess I should open a separate issue for this, shouldn't I?

@anntzer
Copy link
Contributor

anntzer commented Jan 3, 2022

Please do (I don't have a solution for that off the top of my head).

@oscargus oscargus added the status: has patch patch suggested, PR still needed label Feb 28, 2022
@TheRikke
Copy link

TheRikke commented Mar 4, 2022

I have the same issue and can confirm that the patch fixed it.

QuLogic added a commit to QuLogic/matplotlib that referenced this issue Mar 31, 2022
`configure_subplots` requires a) an object from which it can get the
`canvas` property, and b) somewhere it can store its handle for re-use.

The pseudo-toolbar that is used for most `Tool` classes is temporary and
does not provide the latter. However, tools inherit from `ToolBase`
which contains a `canvas` property and they exist for the lifetime of
the toolbar, so can be used instead.

Fixes matplotlib#22088
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GUI: Qt status: has patch patch suggested, PR still needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants