Skip to content

Conversation

@sol87
Copy link

@sol87 sol87 commented Oct 21, 2016

If there are custom widgets in ui files, we must register them before loader.load() in pyside. So I make a new function for that.

@mottosso
Copy link
Owner

Thanks @sol87! Could you make an example or test for this functionality as well? Have a look in the tests.py file for how you could write one.

@mottosso
Copy link
Owner

And also take a gander at the error message, looks like something went out the window with PySide2 for both Python 2 and 3.

See the developer guide for how you could run tests locally, of keep committing to your repository to test here.

@fredrikaverpil
Copy link
Collaborator

fredrikaverpil commented Oct 21, 2016

Hi @sol87 and thanks for this!

Regarding adding custom widgets support, I did implement that into load_ui back in an old PR which we collectively decided to close.

You can view the full load_ui implementation I was proposing here: https://github.com/fredrikaverpil/Qt.py/blob/3daabe95bff0e09454595899543a3ab1d75c4e70/Qt.py

However, this was not merged. We came to the conclusion that this would add functionality to Qt.py which wasn't maintained by PySide/PyQt/Qt but instead had to be maintained by us. For end-users this would mean relying on features which were actually not developed in co-operation with neither the PySide development team or the PyQt development team. And the same goes for adding the "base_instance" argument, which I personally needed at the time.

We actually defined some of our guidelines based on that discussion:

Henceforth, Qt.py is about linking together commonalities between PySide2 and other bindings, and documenting differences, so as to foster a mindful developer of cross-compatible software.

This is in contrast to wrapping all functionality that differs PySide2 with our own ad-hoc implementations, which we believe will cause more problems than it solves long-term.

This should've been added to the development guidelines, which @mottosso already mentioned, but now when I glance through it, I can't see it being mentioned so we should probably fix that.
@mottosso I can see "No implementations = No bugs." but that's a little vague?

Okay... back to my story. Since I personally set out to add a base instance argument to load_ui, I decided it would be useful to at least add examples of how load_ui could be extended to support this (although not making load_ui) support it out of the box, and therefore steering users in a more compatible direction in terms of development. These examples are part of Qt.py testing and reside here: https://github.com/mottosso/Qt.py/tree/master/examples/load_ui

I propose that instead of adding custom widgets support to Qt.py directly we add one another example, where it is shown how such functionality can be added to load_ui. In the end, this would just mean that for the majority of users, load_ui is a simple function doing simple remapping - and for those who need more complex functionality, examples are provided on how to extend load_ui.

Please let me know what you think.

@sol87
Copy link
Author

sol87 commented Oct 21, 2016

Sure. I will make a example file soon.

@sol87
Copy link
Author

sol87 commented Oct 21, 2016

Hi @fredrikaverpil . I have checked your hack. It seems cool. I believe you make this decision based on good reasons :)

@mottosso
Copy link
Owner

Well put, @fredrikaverpil, and thanks for understanding @sol87!

mottosso added a commit to abstractfactory/Qt.py that referenced this pull request Oct 30, 2016
@mottosso mottosso mentioned this pull request Oct 30, 2016
@mottosso mottosso closed this Nov 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants