Skip to content

Add @QtCore.Slot() decorations to NavigationToolbar2QT #27215

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Nov 18, 2023
Merged

Add @QtCore.Slot() decorations to NavigationToolbar2QT #27215

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8b81c13
Add `@QtCore.Slot()` decorations
bersbersbers Oct 27, 2023
498b5b1
Avoid `lambda` expression for `flake8`
bersbersbers Oct 27, 2023
40c6756
Maintain `__name__` for `PyQt`
bersbersbers Oct 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion lib/matplotlib/backends/backend_qt.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -642,8 +642,13 @@ def __init__(self, canvas, parent=None, coordinates=True):
if text is None:
self.addSeparator()
else:
slot = getattr(self, callback)
# https://bugreports.qt.io/browse/PYSIDE-2512
slot = functools.wraps(slot)(functools.partial(slot))
slot = QtCore.Slot()(slot)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this indeed portable this way, can we just decorate the methods directly? Even if we have to add a bunch of methods like

@QtCore.Slot
def foo(self):
    return super().foo()

that seems better than doing the multi-layered meta-programming here!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, can do. A considerable amount of meta-programming is still done in the loop just around the snippet, so maybe a test is needed to make sure that all functions in self.toolitems have been decorated...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, that's interesting. I tried adding

    @QtCore.Slot()
    def home(self, *args):
        return super().home(*args)

    @QtCore.Slot()
    def back(self, *args):
        return super().back(*args)

    @QtCore.Slot()
    def forward(self, *args):
        return super().forward(*args)

    @QtCore.Slot()
    def pan(self, *args):
        return super().pan(*args)

    @QtCore.Slot()
    def zoom(self, *args):
        return super().zoom(*args)

    @QtCore.Slot()
    def configure_subplots(self, *args):
        return super().configure_subplots(*args)

    @QtCore.Slot()
    def edit_parameters(self, *args):
        return super().edit_parameters(*args)

    @QtCore.Slot()
    def save_figure(self, *args):
        return super().save_figure(*args)

and this works for home, back and forward - but not for the others. Irrespective of the position inside NavigationToolbar2QT, and irrespective of the order of the functions. This is with PySide6-Essentials v6.6.0.

I put this code into https://github.com/bersbersbers/matplotlib-27214/commit/a3d97f8eb2b40a46fe2d4fc4cf60f9d631fd3cf2 if anyone wants to try it themselves.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My guess is multiple inheritance is related, but do not have a good theory as to why (other than general suspicion that MI always causes problems....).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the metaprogramming approach works then let's just stick to it.


a = self.addAction(self._icon(image_file + '.png'),
text, getattr(self, callback))
text, slot)
self._actions[callback] = a
if callback in ['zoom', 'pan']:
a.setCheckable(True)
Expand Down