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 :)

[mottosso]

Thanks @dgovil, like Fredrik mentioned it is something we've been through in the past.

Having said that, a few things have changed since then.

1. --convert was introduced to convert from a pyside2-uic generated file to a cross-compatible file.
2. wrapInstance is being added to bridge sip and shiboken.

And by now I had expected plentiful of additional requests for wrapping various functionality but it almost looks like loadUi is the last piece of the puzzle.

So if Fredrik is ok with it, I'd be open to reconsider it, given we first:

1. Specify exactly what we expect (and not expect) of it (see User Stories)
2. Limit expectation to what can realistically be achieved identically across all bindings
3. Assign a maintainer to it (as I'm no user of it myself).

If you put a PR together @dgovil we could continue the discussion there.

[fredrikaverpil]

So if Fredrik is ok with it

👍

[fredrikaverpil]

@nchaverou back on topic; I think you're best off using the baseinstance2.py example. Will that work for you?

[glm-nchaverou]

@fredrikaverpil : yep that's what i'm doing right now.
The only thing with the baseinstance2.py example is that I need a special branching in my imports to get the pysideuic module loaded correctly (which I don't really like). But I guess I can't really avoid it

 if mayaApi >= 201700:
    import pyside2uic as pysideuic
else:
    import pysideuic

[fredrikaverpil]

A third option is to not use the base instance argument and just use QtCompat.loadUi. This is what I do and would recommend everyone do for greatest compatibility when working with multiple Qt bindings.

Then you'll have to do something like this:

my_ui = QtCompat.loadUi(my_ui_file)
my_ui.my_widget  # this is how you would then access a widget loaded from the ui file

This is a compromise which I don't want. But when working with multiple bindings this is an okay compromise, I think, compared to the headaches of dealing with base instance (like you are experiencing now).

[mottosso]

Out of curiosity, is there anything you can't achieve with the approach @fredrikaverpil just suggested?

For example, here's the initial example using this approach, the difference being that main window parameters are defined in Python as opposed to Qt Designer, such as Window Size.

import os
import tempfile

from Qt import QtWidgets, QtCompat


ui = """\
<?xml version="1.0" encoding="UTF-8"?>
<ui version="4.0">
 <class>Form</class>
 <widget class="QWidget" name="Form">
  <property name="geometry">
   <rect>
    <x>0</x>
    <y>0</y>
    <width>400</width>
    <height>300</height>
   </rect>
  </property>
  <property name="windowTitle">
   <string>Form</string>
  </property>
  <layout class="QHBoxLayout" name="horizontalLayout">
   <item>
    <layout class="QVBoxLayout" name="verticalLayout">
     <item>
      <widget class="QPushButton" name="pushButton">
       <property name="text">
        <string>My Button</string>
       </property>
      </widget>
     </item>
    </layout>
   </item>
  </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)

        layout = QtWidgets.QVBoxLayout(self)
        layout.addWidget(QtCompat.loadUi(f.name))

        os.remove(f.name)

        self.resize(300, 300)

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


window = MainWindow()
window.show()

[fredrikaverpil]

I'm guessing that the reason people want to use the base_instance is that they want their ui-loaded widgets to appear exactly next to their "hand-coded" widgets and they want all widgets on self.

Meaning; the following would have all widgets defined on self (class taken from your example above):

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.layout = QtWidgets.QVBoxLayout()  # Creates widget "layout" on self
        QtCompat.loadUi(f.name, base_instance=self)  # Load "pushButton" widget onto self (if base_instance was an argument)

        os.remove(f.name)

        self.resize(300, 300)

        print(self.layout)  # this would work
        print(self.pushButton)  # this would work

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

I can see an issue if you're porting old code, but not really if you're designing your own stuff and from scratch and deliberately don't need your widgets stored on self in favor for simpler code (with multiple Qt bindings).

[dgovil]

I've asked for peoples opinions at work, and here's what I get:

• baseinstance is useful because it loads the ui file directly onto the current instance, so its more (in code) elegant.
• The implementation here allows for the PyQt style auto signal handling
• If you dont use baseinstance and instead just add the ui as a child of the instance, then your code can significantly change, but also you don't inherit the top level properties like window size etc from the ui file.

It's definitely not a problem for new code. But quite a few studios it seem are using Qt.py to port old code, which is where I think the value is.

[mottosso]

Thanks guys that's really useful info.

If you put a PR together @dgovil I'd be happy to start talking implementation details.

[csmotion]

Howdy all, I have a multi-window application that I'd like to close with a custom closeEvent, but have not had any luck implementing the above methods.

What is the current preferred method of overriding closeEvent for a MainWindow(QtWidgets.QWidget) class implementing QtCompat.loadUi?