Skip to content

Add support for additional arguments to load_ui #80

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
bpabel opened this issue Jul 27, 2016 · 17 comments
Closed

Add support for additional arguments to load_ui #80

bpabel opened this issue Jul 27, 2016 · 17 comments
Labels

Comments

@bpabel
Copy link

bpabel commented Jul 27, 2016

Per the python mailing list discussion, add support for the 2nd argument to uic.loadUi

Here is an example using PyQt4

http://nullege.com/codes/search/PyQt4.uic.loadUi

Here is an example of an implementation for PySide that accepts the same arguments

https://gist.github.com/cpbotha/1b42a20c8f3eb9bb7cb8

@mottosso
Copy link
Owner

Thanks @bpabel.

Per the python mailing list discussion

Would it be possible to link to this discussion?

Here is an example of an implementation for PySide that accepts the same arguments

Excellent, would you happen to know whether the same implementation works for PySide2? If you put a pull-request in, we could have it run through tests on each platform, Python version and Qt distribution. That should get the conversation started.

As I'm not a user of loadUi myself, it would also be lovely to have one or more usage examples so that we implement it in a way that makes sense to you.

@fredrikaverpil
Copy link
Collaborator

fredrikaverpil commented Jul 27, 2016

Would it be possible to link to this discussion?

This was from the closed 3DPRO mailing list. I asked Abel to post an issue :)
@mottosso Did you get my invite?

Excellent, would you happen to know whether the same implementation works for PySide2?

I've made a gist where this works in PySide2:
https://gist.github.com/fredrikaverpil/0a89b174c9d29bc585f11d250adf2a7c

I think Abel's request makes a lot of sense. I omitted additional arguments to the load_ui convenience function but the gist seems to show a clear way to support that. We should do it!

@mottosso
Copy link
Owner

Would it be possible to post a minimal example ui file, along with how it is used today, and how you would like it to be used with this feature?

@fredrikaverpil
Copy link
Collaborator

fredrikaverpil commented Jul 28, 2016

Would it be possible to post a minimal example ui file, along with how it is used today, and how you would like it to be used with this feature?

I could do a PR, but I won't have time for it until mid August 😢 as I'm on vacation with my family 😄

However, here's an example UI file which we could use for this example. Today, we can access the widgets from the ui file like this:

import sys
from Qt import QtWidgets
from Qt import load_ui


class Hello(QtWidgets.QWidget):
    def __init__(self):
        super(Hello, self).__init__()
        self.ui = load_ui('main_window.ui')

        print(self.ui.boilerPushButton)  # a pushButton widget from main_window.ui

app = QtWidgets.QApplication(sys.argv)
window = Hello()
window.ui.show()
sys.exit(app.exec_())

However, we can't load the UI widgets directly into self and make the pushButton available like this: self.boilerPushButton. So let's say you load three UI files and want all of their widgets to be accessed like self.widgetName. Today this is possible in PyQt (by providing a second argument to uic.loadUi which load_ui wraps) but not in PySide (not without pysideuic). But with @bpabel's gist, this seems doable with a custom QUiLoader class (really nice since it doesn't require the pysideuic module which isn't always available, e.g. missing in Nuke).

Here's a full example of me loading two UIs with the current behaviour of load_ui and it shows how you need to load each UI into separate objects:
https://github.com/fredrikaverpil/pyvfx-boilerplate/blob/master/boilerplate.py#L164-L165

This is what we want to be able to do with an optional second argument to load_ui. So instead of the example above, we could do this:

class Hello(QtWidgets.QWidget):
    def __init__(self):
        super(Hello, self).__init__()
        self.ui = load_ui('main_window.ui', self)  # Load widgets into self

        print(self.boilerPushButton)  # the button is now available in self

Of course, you could also load widgets into something else than just self. The point is you want control on where your UI file's widget ends up being accessible from.

@mottosso if you like, we can assign this to me, but I won't have time for it until a couple of weeks away.

It would basically entail updating pyside_load_ui, pyside2_load_ui, pyqt4_load_ui and pyqt5_load_ui to support this optional argument.

@fredrikaverpil
Copy link
Collaborator

fredrikaverpil commented Jul 28, 2016

Sorry, I edited my prevous post a lot to clarify things a bit with better examples. Please re-read it if you already read it.

@fredrikaverpil
Copy link
Collaborator

If we were to write tests for this, we'd need to store a .ui file in the repository. How do you feel about that, @mottosso ?

We could create a main_window.ui file in a data dir?

@mottosso
Copy link
Owner

We could, but I'd rather keep it contained in the test module.

If you hand me a .ui snippet, I can show you how I mean.

@fredrikaverpil
Copy link
Collaborator

If you hand me a .ui snippet, I can show you how I mean.

Will this do?

<?xml version="1.0" encoding="UTF-8"?>
<ui version="4.0">
 <class>MainWindow</class>
 <widget class="QMainWindow" name="MainWindow">
  <property name="windowTitle">
   <string>MainWindow</string>
  </property>
  <widget class="QWidget" name="centralwidget">
   <layout class="QGridLayout" name="gridLayout">
    <item row="0" column="0">
     <widget class="QPushButton" name="pushButton">
      <property name="text">
       <string>PushButton</string>
      </property>
     </widget>
    </item>
   </layout>
  </widget>
  <widget class="QMenuBar" name="menubar">
   <property name="geometry">
    <rect>
     <x>0</x>
     <y>0</y>
     <width>125</width>
     <height>22</height>
    </rect>
   </property>
  </widget>
  <widget class="QStatusBar" name="statusbar"/>
 </widget>
 <resources/>
 <connections/>
</ui>

@mottosso
Copy link
Owner

Here's what I had in mind.

def test_pyside_load_ui():
    """load_ui works well"""

    ui = """\
<?xml version="1.0" encoding="UTF-8"?>
<ui version="4.0">
 <class>MainWindow</class>
 <widget class="QMainWindow" name="MainWindow">
  <property name="windowTitle">
   <string>MainWindow</string>
  </property>
  <widget class="QWidget" name="centralwidget">
   <layout class="QGridLayout" name="gridLayout">
    <item row="0" column="0">
     <widget class="QPushButton" name="pushButton">
      <property name="text">
       <string>PushButton</string>
      </property>
     </widget>
    </item>
   </layout>
  </widget>
  <widget class="QMenuBar" name="menubar">
   <property name="geometry">
    <rect>
     <x>0</x>
     <y>0</y>
     <width>125</width>
     <height>22</height>
    </rect>
   </property>
  </widget>
  <widget class="QStatusBar" name="statusbar"/>
 </widget>
 <resources/>
 <connections/>
</ui>"""

    with tempfile.NamedTemporaryFile(suffix=".ui") as f:
        f.write(ui)
        f.seek(0)

        with pyside():
            import Qt
            widget = Qt.load_ui(f.name)

Ideally then, the .ui file would only contain parts we are actually testing. In this case, perhaps QStatusBar and explicit widths are less important in the interest of readability.

@fredrikaverpil
Copy link
Collaborator

Ah, yes. That's an excellent idea.

@mottosso
Copy link
Owner

mottosso commented Jul 28, 2016

It might not seem like a big deal today. From experience with maintaining external files with tests, I find that this works fine initially, but then as the project is updated, tests change, and the external files will need to branch out to cover both in order to be backwards compatible.

With this approach, even though it makes each test bigger and more repetitive, tests and data change together and should be easier to maintain long-term.

@fredrikaverpil
Copy link
Collaborator

Yep. Agreed.

@fredrikaverpil
Copy link
Collaborator

Hm, so I'm wondering about one thing. It looks like we could add a custom class to the _pyside and _pyside2 functions. First, it's quite unconventional to define a class inside a function, but it will inherit QtUiTools.QUiLoader from PySide/PySide2 and will throw an error if Qt.py is using PyQt4/PyQt5.

Right now, I'm just putting the class definition in both these functions. Any idea on how we could structure this more nicely?

@fredrikaverpil
Copy link
Collaborator

Never mind, I think I found a solution.

@mottosso
Copy link
Owner

@fredrikaverpil If you can, please post an example or two of the intended usage before looking at implementing this.

What I'm looking for is to have tests in place before the implementation starts. TDD style.

@fredrikaverpil
Copy link
Collaborator

fredrikaverpil commented Jul 28, 2016

Yes, I'm doing the tests now. But as I had to rebuild the docker container I now am having issues with nosetests complaining about not being able to connect to an X server:

...
...
Raise ImportError if sip API v1 was already set (Python 2.x only) ... ok
load_ui works well ... ERROR
nosetests: cannot connect to X server

Even if I remove my load_ui test, I get the X server error. It seems nosetests broke after I re-built the container from scratch. There are no detailed error messages since nosetests errors out with this X server notice before it can get there.

Do you see this on your end too if you rebuild your container?

Here's my test:

def test_pyside_load_ui():
    """load_ui works well"""

    ui = """\
<?xml version="1.0" encoding="UTF-8"?>
<ui version="4.0">
 <class>MainWindow</class>
 <widget class="QMainWindow" name="MainWindow">
  <property name="windowTitle">
   <string>MainWindow</string>
  </property>
  <widget class="QWidget" name="centralwidget">
   <layout class="QGridLayout" name="gridLayout">
    <item row="0" column="0">
     <widget class="QPushButton" name="pushButton">
      <property name="text">
       <string>PushButton</string>
      </property>
     </widget>
    </item>
   </layout>
  </widget>
  <widget class="QMenuBar" name="menubar">
   <property name="geometry">
    <rect>
     <x>0</x>
     <y>0</y>
     <width>125</width>
     <height>22</height>
    </rect>
   </property>
  </widget>
  <widget class="QStatusBar" name="statusbar"/>
 </widget>
 <resources/>
 <connections/>
</ui>"""

    with tempfile.NamedTemporaryFile(suffix=".ui") as f:
        f.write(ui)
        f.seek(0)

        with pyside():
            from Qt import QtWidgets, load_ui

            class MainWindow(QtWidgets.QMainWindow):
                def __init__(self, parent=None):
                    QtWidgets.QMainWindow.__init__(self, parent)
                    load_ui(f.name, self)
                    assert self.pushButton

            app = QtWidgets.QApplication(sys.argv)
            window = MainWindow()
            # window.show()
            # app.exec_()

I intend to remove QStatusBar, QMenuBar etc as soon as I get this example working.

@mottosso
Copy link
Owner

mottosso commented Sep 7, 2016

See #81

@mottosso mottosso closed this as completed Sep 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants