QDialog, loadUi and closeEvent

[nchaverou]

Hey there,

Wanted to move some legacy tools to maya 2017 using Qt.py
The only tool left I couldn't get to work properly used Nathan Ortiz loadUIType

Thus, I took a look to this page: https://github.com/mottosso/Qt.py/tree/master/examples/loadUi

• baseinstance2.py implements it the same way than we do (but uses pysideuic) but works and my closeEvent function is called properly when the Dialog is closed
• baseinstance1.py is pretty elegant, uses QtCompat.loadUi and looks like what i'd like to do. But when using it, my QDialog closeEvent is not called anymore

Attached is a minimal module with its ui file showing the issue
Am I missing something or is there a limitation to the loadUi function ?

skinWrangler.zip

[mottosso]

Here's a full reproducible of baseinstance1.py with the problem you are having.

import os
import sys
import tempfile

from Qt import QtWidgets, QtCompat


def setup_ui(uifile, base_instance=None):
    """Load a Qt Designer .ui file and returns an instance of the user interface

    Args:
        uifile (str): Absolute path to .ui file
        base_instance (QWidget): The widget into which UI widgets are loaded

    Returns:
        QWidget: the base instance

    """
    ui = QtCompat.loadUi(uifile)  # Qt.py mapped function
    if not base_instance:
        return ui
    else:
        for member in dir(ui):
            if not member.startswith('__') and \
               member is not 'staticMetaObject':
                setattr(base_instance, member, getattr(ui, member))
        return ui

ui = """\
<?xml version="1.0" encoding="UTF-8"?>
<ui version="4.0">
 <class>Dialog</class>
 <widget class="QDialog" name="Dialog">
  <property name="geometry">
   <rect>
    <x>0</x>
    <y>0</y>
    <width>300</width>
    <height>300</height>
   </rect>
  </property>
  <property name="windowTitle">
   <string>Dialog</string>
  </property>
  <layout class="QVBoxLayout" name="verticalLayout">
   <property name="spacing">
    <number>2</number>
   </property>
   <property name="margin">
    <number>3</number>
   </property>
  </layout>
 </widget>
 <resources/>
 <connections/>
</ui>

"""


class MainWindow(QtWidgets.QWidget):
    def __init__(self, parent=None):
        QtWidgets.QWidget.__init__(self, parent)
        
        with tempfile.NamedTemporaryFile("w+b", delete=False) as f:
            f.write(ui)

        self.base_instance = setup_ui(f.name, self)
        os.remove(f.name)

    def closeEvent(self, event):
        print("Called closeEvent()")

window = MainWindow()
window.show()
window.close()

Removing the setup_ui() line works as expected, with it the closeEvent isn't called.

[mottosso]

Surrounding it with print-statements reveals that the call to setup_ui erases the overridden function.

...
class MainWindow(QtWidgets.QWidget):
    def __init__(self, parent=None):
        QtWidgets.QWidget.__init__(self, parent)

        with tempfile.NamedTemporaryFile("w+b", delete=False) as f:
            f.write(ui)

        print(self.closeEvent)
        self.base_instance = setup_ui(f.name, self)
        print(self.closeEvent)

        os.remove(f.name)

    def closeEvent(self, event):
        print("Called closeEvent()")

Output

<bound method MainWindow.closeEvent of <__main__.MainWindow object at 0x000002C4C0AA9408>>
<built-in method closeEvent of PySide.QtGui.QDialog object at 0x000002C4C0B86C88>

[mottosso]

Handing over to @fredrikaverpil, seeing anything odd here?

[fredrikaverpil]

Thanks @mottosso for putting together the reproducible code!

When looping over the members of the UI file in setup_ui, the closeEvent (among many other events) is being replaced with the one from QDialog in this case. To prevent this you can modify the if condition to exclude closeEvent (or any member ending with "Event" like in the example below).

I can update the docs to make a note of this.

def setup_ui(uifile, base_instance=None):
    """Load a Qt Designer .ui file and returns an instance of the user interface

    Args:
        uifile (str): Absolute path to .ui file
        base_instance (QWidget): The widget into which UI widgets are loaded

    Returns:
        QWidget: the base instance

    """
    ui = QtCompat.loadUi(uifile)  # Qt.py mapped function
    if not base_instance:
        return ui
    else:
        for member in dir(ui):
            if not member.startswith('__') and \
                    member is not 'staticMetaObject' and \
                    not member.endswith('Event'):
                setattr(base_instance, member, getattr(ui, member))

        return ui

@nchaverou will this work for you?

[mottosso]

Maybe there is a way to only override non-overridden methods in the setup_ui function?

[fredrikaverpil]

Yeah... you could do something like this to check whether the member was overridden in the class instance and if so skip it:

import inspect

def has_method(obj, name):
    """Returns True if method is defined directly in instance"""
    v = vars(obj.__class__)
    return name in v and inspect.isroutine(v[name])

[fredrikaverpil]

Let me whip up a working example.

[fredrikaverpil]

Actually, I just noticed I'm barking up the completely wrong tree as my suggestion didn't fix the original issue. I can only get "Called closeEvent()" to print if I use a base instance which is not self. Hmmm...

[fredrikaverpil]

...which in itself is kind of weird as I get this when I print the method before and after setup_ui:

<bound method MainWindow.closeEvent of <__main__.MainWindow object at 0x15e2a7950>>
<bound method MainWindow.closeEvent of <__main__.MainWindow object at 0x15e2a7950>>

[fredrikaverpil]

This is what I have. But unfortunately, I don't know why we can't get the closeEvent to print unless you use base_instance=self.my_ui.

import inspect
import os
import sys
import tempfile

from Qt import QtWidgets, QtCompat


def has_method(obj, name):
    """Returns True if method is defined directly in instance"""
    v = vars(obj.__class__)
    return name in v and inspect.isroutine(v[name])


def setup_ui(uifile, base_instance=None, obj=None):
    """Load a Qt Designer .ui file and returns an instance of the user interface

    Args:
        uifile (str): Absolute path to .ui file
        base_instance (QWidget): The widget into which UI widgets are loaded

    Returns:
        QWidget: the base instance

    """
    ui = QtCompat.loadUi(uifile)  # Qt.py mapped function
    overridden_methods = []


    if obj is not None:
        for member in dir(ui):
            if has_method(obj, member):
                overridden_methods.append(member)

    # print(overridden_methods)
    
    if not base_instance:
        return ui
    else:
        for member in dir(ui):
            if not member.startswith('__') and \
                    member is not 'staticMetaObject' and \
                    member not in overridden_methods:
                setattr(base_instance, member, getattr(ui, member))
            # else:
            #     print('Skip ' + member)


        return ui

ui = """\
<?xml version="1.0" encoding="UTF-8"?>
<ui version="4.0">
 <class>Dialog</class>
 <widget class="QDialog" name="Dialog">
  <property name="geometry">
   <rect>
    <x>0</x>
    <y>0</y>
    <width>300</width>
    <height>300</height>
   </rect>
  </property>
  <property name="windowTitle">
   <string>Dialog</string>
  </property>
  <layout class="QVBoxLayout" name="verticalLayout">
   <property name="spacing">
    <number>2</number>
   </property>
   <property name="margin">
    <number>3</number>
   </property>
  </layout>
 </widget>
 <resources/>
 <connections/>
</ui>

"""


class MainWindow(QtWidgets.QWidget):
    def __init__(self, parent=None):
        QtWidgets.QWidget.__init__(self, parent)
        
        with tempfile.NamedTemporaryFile("w+b", delete=False) as f:
            f.write(ui)

        print(self.closeEvent)
        # self.my_ui = QtWidgets.QWidget()
        # self.base_instance = setup_ui(f.name, base_instance=self.my_ui, obj=self)
        self.base_instance = setup_ui(f.name, base_instance=self, obj=self)
        print(self.closeEvent)
        os.remove(f.name)

    def closeEvent(self, event):
        print("Called closeEvent()")

window = MainWindow()
window.show()
window.close()

<bound method MainWindow.closeEvent of <__main__.MainWindow object at 0x15e2d3758>>
<bound method MainWindow.closeEvent of <__main__.MainWindow object at 0x15e2d3758>>

[dgovil]

Hey guys, so semi related, but at work I'm getting a lot of requests for QtComp.loadUi to support the baseinstance parameter so it matches the uic behavior.

I have an implementation we're trying based on this gist I found here. It's not the prettiest but it seems to be working.
It's based off of this: https://gist.githubusercontent.com/cpbotha/1b42a20c8f3eb9bb7cb8/raw/a4e9b5086d719be98d8bdc0ca189270a7874867d/pyside_dynamic.py

I'm getting permission to contribute the changes back but essentially:

• I removed support for the customWidgets parameter and for the workingDirectory since the uic signature doesn't support it.
• I only define the class if the binding is PySide/PySide2 and their loadUi method runs the function in that gist.

Anyway hopefully I can contribute the changes back, but it's the minimal most change I could make to Qt.py while keeping all the bindings working identically.

[fredrikaverpil]

@dgovil yeah, I created Qt.py support for loadUi (previously known as load_ui), has been using it extensively and I made an effort to add base instance too but we ended up deciding it was a dependency to maintain outside of the Qt.py project: #81 (comment)

Just wanted to mention it because there was substantial amount of work (as you can see from that PR) as well as discussing back and forth. In the end, it still seems right to me that we ultimately didn't support this in Qt.py as this would be difficult to maintain.

This is why the two examples exists which makes it easy to customize QtCompat.loadUi to accept the base instance: https://github.com/mottosso/Qt.py/blob/master/examples/loadUi/README.md

In this thread, we're discussing why the baseinstance1.py example causes some issues but it's worth noting that the baseinstance2.py does not cause these same issues and does accept the base_instance argument.

My recommendation is to not attempt to further extend the QtCompat.loadUi and instead look to baseinstance1.py and baseinstance2.py implementation examples.

[dgovil]

Fair enough, its definitely good to want to keep the separation of custom functionality separate. I'll maintain the extensions to loadUi in our fork then rather than try and mainline it.

[fredrikaverpil]

I forgot to mention I very much appreciate the fact that you want to contribute back and I know @mottosso does too. ❤️

Let's hear what @mottosso has to say as well. Perhaps we should consider including the base instance examples into the Python wheel (and then perhaps just baseinstance2.py since we now are facing some issues with baseinstance1.py)?
Would that be helpful?
One drawback of doing that is that some users just download the Qt.py file and use that as-is - and the other obvious one is then we basically does maintain this helper function...

[dgovil]

Oh I don't really have a stake in the functionality of loadUi myself so I wouldn't want to try and make the argument either way. Personally I don't ever use ui files (handcoded UI's all the way!) . I only implemented it internally because we're starting our long port to Maya 2017 and a lot of TDs here use loadUi.

I agree with you guys that introducing custom functionality sort of puts the project on the hook for dealing with issues that may arise from the custom implementations and being the basis for a lot of production tools, that can start ballooning.

On the flip side, I see it being a common request from a lot of studio TDs who use loadUi with the baseinstance parameter.

And I'm glad you guys are open to discussion and contributions :)