Skip to content
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

API that's open to developers (binding_helper) needs more documented #2

Closed
2 tasks done
130s opened this issue Feb 28, 2013 · 7 comments
Closed
2 tasks done
Assignees

Comments

@130s
Copy link
Member

130s commented Feb 28, 2013

A few missing documents:

  • python_qt_binding.loadUi should obligate the 3rd arg, since PySide binding requires the 3rd arg in order to incorporate custom widget classes, whereas PyQt doesn't (Related discussion)
  • Some public variables (QT_BINDING, QT_BINDING_MODULES, QT_BINDING_VERSION) missing doc
@DorianScholz
Copy link
Member

I think changing the python_qt_binding loadui implementation for PySide in a way so it does not need the 3rd parameter would be a better solution.
This would allow existing PyQt code to run directly and not require loadui calls for ui files without custom widgets to explicitly pass an empty dict as 3rd arg.
This should be possible by changing the check for a class_name in custom_widgets to an import of that class_name inside a try except block:
https://github.com/ros-visualization/python_qt_binding/blob/groovy-devel/src/python_qt_binding/binding_helper.py#L204
I don't have the time to try this out and test it properly, but you're welcome to do this.

@130s
Copy link
Member Author

130s commented Mar 1, 2013

Comment by @DorianScholz in answers.ros.org:

This was certainly true for PySide 1.0.6, but has anyone checked if it changed in more recent versions?

I wanted to give it a try but before even reaching the point, I haven't been able to load .ui file in pyside...(I posted a question here...if interested I appreciate your answer).

@130s
Copy link
Member Author

130s commented Mar 2, 2013

Having given up making test class, instead I tested the current python_qt_binding using existing rqt plugin (should be enough for this purpose). I found it's still not working with pyside 1.1.1 on Quantal.

Changing this line in rqt_bag from:

        loadUi(ui_file, self, {'BagGraphicsView': BagGraphicsView})

to

        loadUi(ui_file, self)

yields:

"QFormBuilder was unable to create a custom widget of the class 'BagGraphicsView'; defaulting to base class 'QGraphicsView'." 

This indicates that the loading mthods of pyqt & pyside is not yet compatible out of the box.

@130s
Copy link
Member Author

130s commented Mar 2, 2013

@DorianScholz I couldn't exactly visualize your suggestion. Could you write a simple pseudo code?

@DorianScholz
Copy link
Member

@130s Your approach was correct, this is how it should be called: loadUi(ui_file, self)
But for this to work the PySide version of loadUi needs to be extended to automatically import the module "BagGraphicsView" and map the widget class "BagGraphicsView" to it.

For this to work, it needs to take a similar approach to the PyQt version, which uses the header file entry from the ui-file to find the appropriate module to load. But I'm not sure whether the PySide/Qt API of QUiLoader makes the "header file" entry of the ui-file available. Figuring this out and if possible implementing this automatic module importing would be the right approach here, but too time consuming for me at the moment...

@130s
Copy link
Member Author

130s commented Mar 15, 2013

@DorianScholz I see. I can also try when I get to have time.
Meanwhile, I added api doc (192c138 and later) so that users won't forget to add 3rd arg for pyside.

@dirk-thomas
Copy link
Contributor

Commit 7e8d7cc documents the loadUi function.

Commit a6ef126 documents the available variables (e.g. QT_BINDING, QT_BINDING_VERSION, QT_BINDING_MODULES)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants