Skip to content

[TST] Qt/Pyside 6.5.1 dependency test failures #25988

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
github-actions bot opened this issue May 27, 2023 · 8 comments · Fixed by #26005
Closed

[TST] Qt/Pyside 6.5.1 dependency test failures #25988

github-actions bot opened this issue May 27, 2023 · 8 comments · Fixed by #26005

Comments

@github-actions
Copy link

The weekly build with nightly wheels from numpy and pandas
has failed. Check the logs for any updates that need to be
made in matplotlib.
https://github.com/matplotlib/matplotlib/actions/runs/5097297090

@QuLogic
Copy link
Member

QuLogic commented May 27, 2023

This seems to be something to do with the PySide6 release earlier today.

@QuLogic
Copy link
Member

QuLogic commented May 29, 2023

I can reproduce this locally, and the timer works, is called, the quit key is triggered, and the script exits, but something appears wrong getting the close event.

@QuLogic
Copy link
Member

QuLogic commented May 30, 2023

FWICT, the problem seems to be mixed signals across multiple figures. The test in question has a second portion (for Agg-based backends only, which is why Cairo ones don't fail) which opens a figure to lock in the backend, then attempts to switch backends and confirm that it fails. This occurs before running the main test of opening a window, and closing it after the first draw event. In that case, the Matplotlib side does correctly close the Qt window, but the closing signal emitted through Qt seems to trigger the callback from the first figure. The first figure manager/canvas doesn't have any callbacks connected to it to print the CloseEvent, so the test fails.

Using this patch to run a copy of the test:

diff --git a/lib/matplotlib/backends/backend_qt.py b/lib/matplotlib/backends/backend_qt.py
index faeb0d6bb7..4cfa294c6e 100644
--- a/lib/matplotlib/backends/backend_qt.py
+++ b/lib/matplotlib/backends/backend_qt.py
@@ -501,6 +501,7 @@ class MainWindow(QtWidgets.QMainWindow):
     closing = QtCore.Signal()

     def closeEvent(self, event):
+        print('closeEvent', self, event, self.closing)
         self.closing.emit()
         super().closeEvent(event)

@@ -522,9 +523,11 @@ class FigureManagerQT(FigureManagerBase):
     def __init__(self, canvas, num):
         self.window = MainWindow()
         super().__init__(canvas, num)
-        self.window.closing.connect(
-            # The lambda prevents the event from being immediately gc'd.
-            lambda: CloseEvent("close_event", self.canvas)._process())
+        print('Creating new figure manager', self, canvas, num, self.window, self.window.closing)
+        def foo():
+            print('close event is happening!', self, self.canvas, self.window)
+            CloseEvent("close_event", self.canvas)._process()
+        self.window.closing.connect(foo)
         self.window.closing.connect(self._widgetclosed)

         if sys.platform != "darwin":
@@ -604,6 +607,7 @@ class FigureManagerQT(FigureManagerBase):
             self.window.raise_()

     def destroy(self, *args):
+        print('FigureManagerQT.destroy', self, self.window)
         # check for qApp first, as PySide deletes it in its atexit handler
         if QtWidgets.QApplication.instance() is None:
             return

outputs:

Creating new figure manager <matplotlib.backends.backend_qt.FigureManagerQT object at 0x7fc90fe38a60> <matplotlib.backends.backend_qtagg.FigureCanvasQTAgg(0x560b8d915020) at 0x7fc90fcc1f80> 1 <matplotlib.backends.backend_qt.MainWindow(0x560b8d91b750) at 0x7fc90fcc24c0> <PySide6.QtCore.SignalInstance closing() at 0x7fc90fffa1b0>
FigureManagerQT.destroy <matplotlib.backends.backend_qt.FigureManagerQT object at 0x7fc90fe38a60> <matplotlib.backends.backend_qt.MainWindow(0x560b8d91b750) at 0x7fc90fcc24c0>
closeEvent <matplotlib.backends.backend_qt.MainWindow(0x560b8d91b750) at 0x7fc90fcc24c0> <PySide6.QtCore.QEvent(QEvent::Close)> <PySide6.QtCore.SignalInstance closing() at 0x7fc90fffa1b0>
close event is happening! <matplotlib.backends.backend_qt.FigureManagerQT object at 0x7fc90fe38a60> <matplotlib.backends.backend_qtagg.FigureCanvasQTAgg(0x560b8d915020) at 0x7fc90fcc1f80> <matplotlib.backends.backend_qt.MainWindow(0x560b8d91b750) at 0x7fc90fcc24c0>

Creating new figure manager <matplotlib.backends.backend_qt.FigureManagerQT object at 0x7fc90d581900> <matplotlib.backends.backend_qtagg.FigureCanvasQTAgg(0x560b8d9b5ea0) at 0x7fc90d590340> 1 <matplotlib.backends.backend_qt.MainWindow(0x560b8d9e4010) at 0x7fc90d7dfd00> <PySide6.QtCore.SignalInstance closing() at 0x7fc90d715830>
FigureManagerQT.destroy <matplotlib.backends.backend_qt.FigureManagerQT object at 0x7fc90d581900> <matplotlib.backends.backend_qt.MainWindow(0x560b8d9e4010) at 0x7fc90d7dfd00>
closeEvent <matplotlib.backends.backend_qt.MainWindow(0x560b8d9e4010) at 0x7fc90d7dfd00> <PySide6.QtCore.QEvent(QEvent::Close)> <PySide6.QtCore.SignalInstance closing() at 0x7fc90d715830>
close event is happening! <matplotlib.backends.backend_qt.FigureManagerQT object at 0x7fc90fe38a60> <matplotlib.backends.backend_qtagg.FigureCanvasQTAgg(0x560b8d915020) at 0x7fc90fcc1f80> <matplotlib.backends.backend_qt.MainWindow(0x560b8d91b750) at 0x7fc90fcc24c0>

The second figure has a figure.canvas.manager of 0x7fc90d581900 and a figure.canvas.manager.window.closing instance of 0x7fc90d715830, and you can see that closeEvent receives it for that manager (so the correct window should be closed) and the same closing instance. But then when we get the callback for that signal, it's using the manager for the first window.

@QuLogic
Copy link
Member

QuLogic commented May 30, 2023

@ksunden
Copy link
Member

ksunden commented May 30, 2023

xref for pyqtgraph also having failures on pyside 6.5.1, seems at least overlapping root cause:

pyqtgraph/pyqtgraph#2734

@QuLogic
Copy link
Member

QuLogic commented May 30, 2023

Ah, interesting. We have two handlers for the closing signal, one a lambda and one a method, and based on that, the former is the one having trouble. I had considered merging them together, but for some reason never tried that out yesterday.

Merging them does seem to fix the problem, so I'll try to audit all the signal connections to ensure that's the only place it's needed.

@QuLogic
Copy link
Member

QuLogic commented May 30, 2023

There's another case of a handler with local capture in tool manager:

def handler():
self.trigger_tool(name)
if toggle:
button.setCheckable(True)
button.toggled.connect(handler)
else:
button.clicked.connect(handler)

I'm not sure if it's possible for us to avoid that one.

@ksunden ksunden changed the title [TST] Upcoming dependency test failures [TST] Qt/Pyside 6.5.1 dependency test failures May 30, 2023
@ksunden
Copy link
Member

ksunden commented May 30, 2023

(renamed to make it more discoverable as an issue since there is some conversation/notes here)

@QuLogic QuLogic added this to the v3.7.2 milestone Jul 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants