24

I am trying to connect slots with lambda functions, but it's not working the way I expect. In the code below, I succeed in connecting the first two buttons correctly. For the second two, which I connect in a loop, this goes wrong. Someone before me had the same question (Qt - Connect slot with argument using lambda), but this solution doesn't work for me. I've been staring at my screen for a half hour, but I can't figure out how my code is different.

class MainWindow(QtGui.QWidget):
    def __init__(self):
        super(QtGui.QWidget, self).__init__()

        main_layout = QtGui.QVBoxLayout(self)

        # Works:
        self.button_1 = QtGui.QPushButton('Button 1 manual', self)
        self.button_2 = QtGui.QPushButton('Button 2 manual', self)
        main_layout.addWidget(self.button_1)
        main_layout.addWidget(self.button_2)

        self.button_1.clicked.connect(lambda x:self.button_pushed(1))
        self.button_2.clicked.connect(lambda x:self.button_pushed(2))

        # Doesn't work:
        self.buttons = []
        for idx in [3, 4]:
            button = QtGui.QPushButton('Button {} auto'.format(idx), self)
            button.clicked.connect(lambda x=idx: self.button_pushed(x))
            self.buttons.append(button)
            main_layout.addWidget(button)


    def button_pushed(self, num):
        print 'Pushed button {}'.format(num)

Pressing the first two buttons yields 'Pushed button 1' and 'Pushed button 2', the other two both yield 'Pushed button False', although I expected 3 and 4.

I also haven't understood the lambda mechanism completely. What exactly gets connected? A pointer to a function that is generated by lambda (with the parameter substituted in) or is the lambda function evaluated whenever the signal fires?

Community
  • 1
  • 1
zeus300
  • 1,017
  • 2
  • 12
  • 30

4 Answers4

62

The QPushButton.clicked signal emits an argument that indicates the state of the button. When you connect to your lambda slot, the optional argument you assign idx to is being overwritten by the state of the button.

Instead, make your connection as 

button.clicked.connect(lambda state, x=idx: self.button_pushed(x))

This way the button state is ignored and the correct value is passed to your method.

three_pineapples
  • 11,579
  • 5
  • 38
  • 75
  • 1
    What word 'state' really does? – ioaniatr Aug 19 '18 at 16:54
  • 3
    @ioaniatr When you connect to the `clicked` signal, that signal has a signature that is described [here](http://doc.qt.io/qt-5/qabstractbutton.html#clicked). As you can see, when the signal is emitted, an argument is provided that contains the state of the button (whether or not the button is checked). The variable in my code can be called whatever you want, but it must be there so that Qt doesn't override the next argument (`x=idx`) with the checked state. – three_pineapples Aug 21 '18 at 07:27
  • When I try connecting a QAction from QMenu.addAction() I get an exception: `missing 1 required positional argument: 'checked'` when triggering my menu items. `action.triggered.connect(lambda checked, service=service: printService(service))` what am I doing wrong? – grego Feb 04 '22 at 23:16
  • @grego, if you've not solve this yet. I'd suggest to use functools.partial: ```action.triggered.connect(functools.partial(printService, service))``` – papirosnik Feb 19 '22 at 20:25
  • 1
    @grego. You are probably using PySide instead of PyQt. In this case you have to adjust your code slightly: `action.triggered.connect(lambda checked=None, service=service: printService(service))` since PySide obviously does not send `checked` if the signature of the receiver does not match. – johnson Jun 27 '22 at 08:36
  • If you've got more than one argument, you can assign a list to the x in lambda and change your input vars in your function: `button.clicked.connect(lambda state, x=[a, b, c]: self.button_pushed(x))` – fsg Apr 11 '23 at 08:56
  • Why is the x even necessary? Why not simply `button.clicked.connect(lambda : self.button_pushed(idx))` ? – Marshall Eubanks May 18 '23 at 20:35
  • 1
    @MarshallEubanks It's necessary due to late binding in Python. In your example, the value of `idx` used will be that of the last loop iteration for all signals connected in the loop (including in previous iterations). The way to avoid that is to set it as a default value for an unused keyword argument of the function/lambda. See https://stackoverflow.com/q/3431676/1994235 for a slightly longer explanation – three_pineapples May 29 '23 at 08:03
24

Beware! As soon as you connect your signal to a lambda slot with a reference to self, your widget will not be garbage-collected! That's because lambda creates a closure with yet another uncollectable reference to the widget.

Thus, self.someUIwidget.someSignal.connect(lambda p: self.someMethod(p)) is very evil :)

eyllanesc
  • 235,170
  • 19
  • 170
  • 241
Grigory Makeev
  • 391
  • 3
  • 5
  • 1
    I spent two days of commenting & uncommenting code to discover this turned out to be the "leak" fault I was investigating, then I came across your analysis which is exactly the conclusion I had finally reached. Off to change all the existing code now.... :( – JonBrave Jul 19 '18 at 14:09
  • For the record, also same problem if `lambda` passes `self` to some external function: `self.someUIwidget.someSignal.connect(lambda someNonSelfMethod(self))` – JonBrave Jul 19 '18 at 14:14
  • 2
    What is an alternative then? – pyjamas Dec 12 '18 at 20:56
  • 2
    @Esostack: depends what you want to achieve. If you just want to detect in the slot which object emitted the signal, you can use the `QObject.sender()` method. Additionally, you can store information in widget properties and access them in the slot function, see this example: https://stackoverflow.com/questions/49446832/pyside2-unable-to-get-sender-inside-the-slot/55118468 – Daniel K. Mar 12 '19 at 16:33
  • No alternative is needed. Using slots with closures is not evil. If you're concerned about object cleanup, just explicitly disconnect any signals connected to slots forming a closure over the object being deleted. – ekhumoro Oct 24 '19 at 14:42
  • When do you, @ekhumoro, think is the right time to disconnect all those signals? – Grigory Makeev Nov 18 '19 at 06:54
  • @GrigoryMakeev It only makes sense to worry about this if you are explicitly deleting and recreating the connected objects during runtime (i.e. before the normal python shutdown process). If you don't do that, the program will steadily leak memory, because the closures will keep all the objects alive. So simply disconnect the signals in the code that handles the object deletion/cleanup. If the objects ***aren't*** being deleted during runtime, there is no point in disconnecting anything, because the system will reclaim all the memory after the program has shut down. – ekhumoro Nov 18 '19 at 13:28
  • @Rhdr what is the conclusion? Does the QtCore.Qt.WA_DeleteOnClose option help the garbage collector to collect a complex widget with subwidgets with complex references and lambda-based slots with closures? – Grigory Makeev Jul 14 '20 at 10:08
  • 1
    @Rhdr anothe discussion worth mentioning was here https://stackoverflow.com/questions/33304203/pyqt-a-correct-way-to-connect-multiple-signals-to-the-same-function-in-pyqt-qs/33310118 – Grigory Makeev Jul 14 '20 at 10:46
  • @GrigoryMakeev I have only played with simple widgets using QtCore.Qt.WA_DeleteOnClose not nearly the complexity you describe (hence the reason I commented "Looks like WA_DeleteOnClose recovers memory..." on the link I shared above) – Rhdr Jul 14 '20 at 14:34
  • @GrigoryMakeev Lesiok on Qt forum: "Qt::WA_DeleteOnClose setting makes a call to the widget deleteLater" Link: https://www.qtcentre.org/threads/56560-Difference-between-WA_DeleteOnClose-true-and-deleteLater() – Rhdr Jul 16 '20 at 06:58
0

I'm not honestly sure what's going wrong with your use of lambda here either. I think it's because idx (your loop index when you set up the auto buttons) is going out of scope and doesn't contain the proper value anymore.

But I don't think you need to do it this way. It looks like the only reason you're using lambda is so that you can pass an argument to button_pushed(), identifying which button it was. There's a function sender() that can be called in the button_pushed() slot which identifies which button originated the signal.

Here's an example which I think does more or less what you were shooting for:

from PyQt5.QtGui import *
from PyQt5.QtCore import *
from PyQt5.QtWidgets import *

import sys

class MainWindow(QWidget):
    def __init__(self):
        super(QWidget, self).__init__()

        main_layout = QVBoxLayout(self)

        self.buttons = []

        # Works:
        self.button_1 = QPushButton('Button 1 manual', self)
        main_layout.addWidget(self.button_1)
        self.buttons.append(self.button_1)
        self.button_1.clicked.connect(self.button_pushed)

        self.button_2 = QPushButton('Button 2 manual', self)
        main_layout.addWidget(self.button_2)
        self.buttons.append(self.button_2)
        self.button_2.clicked.connect(self.button_pushed)

        # Doesn't work:
        for idx in [3, 4]:
            button = QPushButton('Button {} auto'.format(idx), self)
            button.clicked.connect(self.button_pushed)
            self.buttons.append(button)
            main_layout.addWidget(button)


    def button_pushed(self):
        print('Pushed button {}'.format(self.buttons.index(self.sender())+1))


app = QApplication(sys.argv)
w = MainWindow()
w.show()
sys.exit(app.exec_())
jfsturtz
  • 397
  • 4
  • 13
  • (Obviously, it would be trivial to create all the buttons 'auto' (in the loop), rather than two of them outside the loop and two of them inside the loop, if that's your eventual goal ... – jfsturtz Mar 05 '16 at 22:01
  • Thanks for the suggestion, but the three_pineapples solution above is exactly what I was looking for and explains what's going on. And yes, there is no reason to not have all four buttons generated in the loop other than showing an example of how the behavior is different outside the loop. – zeus300 Mar 06 '16 at 00:51
  • I'm also glad to learn what the problem was with the way you were doing it. – jfsturtz Mar 06 '16 at 01:47
-1

It's very simple. Check the working code and not working code. You have a syntax error.

Working code:

self.button_1.clicked.connect(lambda x:self.button_pushed(1))

Doesn't work:

button.clicked.connect(lambda x=idx: self.button_pushed(x))

Fix:

button.clicked.connect(lambda x: self.button_pushed(idx))

for lambda you are defining an "x" function and explaining the function to the program as "self.button_pushed(idx)" in order to use arguments with your function in this case the argument is (idx). Just try and let me know if it works.

The issue he has is that he is trying to get different outputs from a for loop creation. Unfortunately, it assigns the last value to any variable named button, so it gives 4 as a result. First two are working because they are not created in a for loop but created individually.

And the names of the button variables are different in the working ones as button_1 and button_2. In the for loop all buttons created will have the name button which results in the same function.

The solution for what he is trying to do is below and it works like a charm.

from sys import *
from PyQt5.QtCore import *
from PyQt5.QtGui import *
from PyQt5.QtWidgets import *

buttons = []

def newWin():
    window = QWidget()
    window.setWindowTitle("Lambda Loop")
    window.setFixedWidth(1000)
    window.move(175, 10)
    window.setStyleSheet("background: #161219;")
    grid = QGridLayout()
    return window, grid

def newButton(text :str, margin_left, margin_right, x):
    button = QPushButton(text)
    button.setCursor(QCursor(Qt.PointingHandCursor))
    button.setFixedWidth(485)
    button.setStyleSheet(
        "*{border: 4px solid '#BC006C';" +
        "margin-left: " + str(margin_left) + "px;" +
        "margin-right: " + str(margin_right) + "px;" +
        "color: 'white';" +
        "font-family: 'Comic Sans MS';" +
        "font-size: 16px;" +
        "border-radius: 25px;" +
        "padding: 15px 0px;" +
        "margin-top: 20px;}" +
        "*:hover {background: '#BC006C'}"
    )

    def pushed():
        val = x
        text = QLabel(str(val))
        text.setAlignment(Qt.AlignRight)
        text.setStyleSheet(
            "font-size: 35px;" +
            "color: 'white';" +
            "padding: 15px 15px 15px 25px;" +
            "margin: 50px;" +
            "background: '#64A314';" +
            "border: 1px solid '#64A314';" +
            "border-radius: 0px;"
        )
        grid.addWidget(text, 1, 0)
    button.clicked.connect(pushed)
    return button

app = QApplication(argv)
window, grid = newWin()

def frame1(grid):
    for each in [3, 4]:
        button = newButton('Button {}'.format(each), 150, 150, each)
        buttons.append(button)
        pass
    b_idx = 0
    for each in buttons:
        grid.addWidget(each, 0, b_idx, 1, 2)
        b_idx += 1

frame1(grid)

window.setLayout(grid)

window.show()

exit(app.exec())

I put all in one place so that all can see. Tell what you want to do is easier than speculations. (You can also add new variables in the list in Frame function and it will create more buttons for you with different values and functions.)

Huseyin Sozen
  • 9
  • 4
  • 1
    Avoid arguing about the votes. Votes are private and need not be justified. If you receive a UV, would you make the same claims? Well no, the same rule applies to DVs. If you put irrelevant information in your post then that is considered noise. If your post is good then for a long time then you will get more UV than DV, if it is not then you will get the opposite. – eyllanesc Sep 23 '21 at 03:02
  • On the other hand, I do not contribute better in your answer since it is a less good version of the accepted answer since for example your solution fails in the for-loops unlike the accepted answer that does cover that case. – eyllanesc Sep 23 '21 at 03:05
  • Apparently you voted negatively. I'm not arguing, but it's not fair to vote people, especially newly registered people like myself for a true statement I've made. Try the code first. – Huseyin Sozen Sep 23 '21 at 03:05
  • 1. I have not voted, and if I had, I would not say so either. That is the right of any user. 2. The votes are given by the post, not by the user as all the SO rules indicate. Here we do not rate the posts by who creates them but by their quality of the post itself. Here it does not matter if you are new or an old user, if you are a beginner or an expert but because of the contribution of the post. Please read [answer] and review the [tour] – eyllanesc Sep 23 '21 at 03:08