Bridge header resize calls

[hdd]

With this patch I'm trying to leverage the difference in setting the headers between Qt4 and Qt5 bindings.

Added some tests, but can't make to have them running locally.
Testing them on Travis on this PR.

[hdd]

weird enough , locally seems to be working.
I'll check why fails on the tests....

[hdd]

still some issues while testing on maya 2017, looking into this.

 PySide2.QtGui.QHeaderView.setResizeMode = __setResizeMode
AttributeError: 'module' object has no attribute 'QHeaderView'

[mottosso]

I'm surprised this works. I wouldn't think that you would be able to monkey patch these C++ classes and expect the Python functionality to get passed on to their corresponding instances via sip/shiboken.

I think the more robust method would be to simply subalss it, and implement the overlapping functionality there. We can hide the original class and replace it with ours. Should be backwards compatible and all.

[mottosso]

But yes, good functionality! This is definitely the right direction for the project.

[hdd]

Same here, surprised but glad it did work.
I'll check for the class inheritance approach.

[hdd]

I think we should be warning the users when they use wrapped functions (either backward or forward compatible). This though should be going into a bigger logging ticket.

[hdd]

not sure why tests are failing.... (all of them).
could it be an issue into the docker container somehow ?

[hdd]

found.

[hdd]

It seems there are still issues with this.

'PySide2.QtWidgets.QHeaderView' object has no attribute 'setResizeMode'

This seems to be happening when trying to access QHeaderView from a widget:
eg:

        horizontalHeader = self.myTable.horizontalHeader()
        horizontalHeader.setResizeMode(QtWidgets.QHeaderView.Fixed)

Ironically, patching the method only seems to be working better,
I'll run more tests and double check.

[hdd]

So it seems monkey patching methods is more robust than patching the class.
I'll roll back to the previous solution for now.

[mottosso]

With #101 in mind, could we have a think about how to work around this caveat, and how to instead document it in CAVEATS.md?

[hdd]

new tests added for pyside2 and pyqt5

[mottosso]

I think we're saying, make this part of CAVEATS.md, rather than implement this in source.

What do you think?

[hdd]

Not sure it does make sense, but I fear is that I'm mostly confused at this point on what's the aim of this project.

Whether you are building a documentation on how to leverage the differences, or you are providing something which does.

To me (as you can infer) is the latter, something which help the user re using old code, and slowly moving to the new one.

I think before spending more time and energy on it, worth clarifying what's the final aim.

[mottosso]

Sorry about that.

The goal of this project is to enable the developer to write for PySide2, and have this run on any other binding as well.

Where interface has been moved, Qt.py moves with it. Where implementation differs, workarounds are offered via CAVEATS.md

Let me know if that makes sense, and I'll try and highlight this in the README.

[hdd]

How can a user use this if on older code (or newer) if we don't provide automatic bridges around the differences ? Stacking up try/except code is not efficient and not maintainable, that's why, in my opinion, this should be handled from within the module itself (as in #96 and #99 ).
So there's just one point of failure, which can be easily tested and fixed, rather than many all over the code.

Having caveats in the docs, is helpful for understanding the differences, but without actually providing some leverage I don't see this module useful. (at least for what I need it for)

[mottosso]

Sorry to hear it doesn't live up to your expectations. :(

if we don't provide automatic bridges around the differences

The majority of difference should be taken care of via the remappings, such as this one for PyQt4.

• https://github.com/mottosso/Qt.py/blob/master/Qt.py#L66

Those cover differences in interface.

For differences in implementation, such as the one this PR is about, I believe we are adding to debugging and development time for both us and users. That's the motivation at least, and why I shy away from re-implementing these things.

I believe working across bindings will require some level of know-how about differences. I believe we get better result accepting, understanding and communicating these differences as opposed to forcing our own ad-hoc solutions that may very well break in other, still unknown corner cases.

Let me know what you think.

[hdd]

This:

 https://github.com/mottosso/Qt.py/blob/master/Qt.py#L66

leverage just the import of Qt in a consistent way for all the binding (which is great), but is already altering the behaviour of the other (older) modules. (so all consistently expose and remap to QtWidgets)

To me all boils down to the usual two tickets #96 and #99.
Without these two , there's no actual way to use this module , which seems to dictates already the direction should be taking.

I'm not saying though, it should be only code, is better if we have also documentation explaining these patches and the actual differences. If properly done, these informations should be presented in the docs, through the api documentation. (sphinx-autodoc)

I agree though, that the users should be aware of what they are using and doing, that's where the #109 comes from. Allowing (if needed) the users to understand what they are actually using.

As you pointed out in some commits, testing is paramount to ensure less as possible issues gets found while using this, so we should be checking and double checking any bridge put in place.

[mottosso]

but is already altering the behaviour of the other (older) modules.

No, this isn't true.

It is adding to other bindings, that is different from altering. The difference being that everything already in, say, PyQt4 will continue to work as it did before Qt.py. This isn't true in #96 on the other hand, see below.

We've tried capturing these decisions in the README, under rules.

To your pull requests, #99 does add value to the project; it enables developers to write for PySide2, while still having this code work with older bindings. (At least I think it does? It should).

#96 on the other hand potentially adds bugs to every binding. This does more than add, this physically alters all bindings, so when Qt.py is used in conjunction with a plain binding, like PySide, then those will also be affected.

[mottosso]

Let me take a step back and tell you what I'm worried about.

I'm worried that we will see bugs coming from (1) Qt, plus bugs coming from (2) a binding (each of them, at the same time!), plus an additional series of bugs coming from (3) Qt.py. That's three layers of bugs.

Just look at the Qt and PyQt mailing lists. They are chocked full with support tickets. If we do it our way, then there will be no bugs from Qt.py.

I get that you have a particular use in mind, I'm saying there is another way to achieve the same goal. I believe this way will mean less work for you in the long run, not more.

[hdd]

Not up for debates, but you are remapping QtGui, QtCore for some and this , seems quite a big change :

Qt.py/Qt.py

Line 114 in 097bbdb

PySide.QtCore.QSortFilterProxyModel = PySide.QtGui.QSortFilterProxyModel

, remapping from QtGui to QtCore.

I think the final call has to come from you, as you are the main maintainer of the project.

I simply can't use this project without these fixes (as it simply doesn't work), if there's no plan to have this behaviour in place (and it's ok by me), I simply might end up forking.

[mottosso]

seems quite a big change

QSortFilterProxyModel doesn't exist in PySide.QtCore. It's an addition, not an alteration.

I simply can't use this project without these fixes

That's fine, there are others who can. If it's between you or them, including myself, I'm sorry but there's only one way it can go.

Thanks for your input!