Remove support for Qt4 (PyQt4 and PySide bindings)

[dalthviz]

Hi, an initial attempt to remove Qt4 (PyQt4 and PySide bindings) support. Not totally sure if more changes are needed and also not sure about some changes I did to the Raspberry instructions so let me know if something else needs to be done

Closes #1788

[brisvag]

The original issue seems to imply that by removing pyqt4

It would really simplify LOTs of the code.

I'm assuming there's a bunch of other places that now could drop qt4-specific tricks, even if qt4 is not explicitly imported?

Right now, I'm not sure this is worth it, given how little and relatively "obvious" (if/else) these changes are. The non-example, non-test diffs are minimal, and touch files that require basically zero maintenance, by the looks of the logs.

Is there some other benefit I'm not seeing?

[djhoese]

I'm assuming there's a bunch of other places that now could drop qt4-specific tricks, even if qt4 is not explicitly imported?

Very likely. We had one case in the other PR @dalthviz made where there were hasattr checks for specific attributes that only Qt 5 and 6 have and 4 doesn't. This could/can probably get cleaned up, but I'd also be OK finding those things over time. Some of those types of cases could also be cleaned up as we drop really old versions of Qt5 where bugs made things not available or convenient Enums (or other types) weren't added until later versions.

Right now, I'm not sure this is worth it, given how little and relatively "obvious" (if/else) these changes are. The non-example, non-test diffs are minimal, and touch files that require basically zero maintenance, by the looks of the logs.

Is there some other benefit I'm not seeing?

I disagree. I'm not sure we can argue about maintenance burden or parts of vispy that require "zero maintenance" when we barely maintain the library as it is. Removing anything old, especially when someone outside the core maintainer group is contributing it, is welcome in my opinion. I also don't know if we can know all the benefits of dropping this support and dependency until the future when people (such as @dalthviz's last PR) want to contribute something to the Qt backend and we have to say "well, could you actually make this 3 times longer to handle this old backend that hopefully no one uses anymore".

Now, I will acknowledge the very likely scenario of these changes:

1. Breaking existing non-Qt4 functionality.
2. Breaking some napari build process that assumes Qt4 support exists.
3. Breaking a workflow for Qt4 users.

However, if it wasn't for https://www.qt.io/blog/qt-5.15-extended-support-for-subscription-license-holders even Qt5 would be out of support for the Qt creators. So if we were really strict we'd drop Qt5 support soon too...but I still use Qt5 in my application so please no.

Qt4 is old and not supported by upstream. The code is old. I see no reason not to merge this when it is already completed and tests pass. If someone who contributes to VisPy and helps support the library depends on Qt4 support then we can revert these changes.

[dalthviz]

I'm assuming there's a bunch of other places that now could drop qt4-specific tricks, even if qt4 is not explicitly imported?

Completly true. As it is, this PR only achieves partially #1788

Is there some other benefit I'm not seeing?

From this PR, as it is, not much more than staying that Qt4 will not be supported anymore. In turn, staying that Qt4 is unsupported could in the future simplify the logic needed to fix bugs or the implementation of any other logic that could relay on newer Qt features that are not present in Qt4 like for example high DPI support (only available for Qt >= 5.6)

Although @djhoese seems okay with the PR as it is, if i'm understanding correctly, @brisvag do you think that a throughout review and elimination of code handling Qt4 needs to be done and be part of this PR to be worthy, right? Or maybe the whole idea of removing Qt4 support is not worthy for the moment?

[brisvag]

To be clear, I have personally zero interest in keeping support for qt4, and agree with all your assessments above @djhoese :P

My point was simply that, since I haven't seen recently a PR that struggled or that had significant extra work because of support for qt4, this seems a bit unnecessary. But it sounds like there was a PR like this and I missed it?

Either way, no strong opinions here; I just felt that this PR as is (without any specific followup plans) only achieves the negatives and none of the positives, so it would be a pity to just merge this, break those 2 workflows that use qt4, and move on like before.

[djhoese]

For reference it was this PR and the related issue(s): #2540

And ok, so the main concern is that removing imports and the other changes here don't "fix" all the if statements in the Qt code that were working around Qt4, right?

@dalthviz if you could identify any of these and clear them up (in the _qt.py and qt.py modules) that would be nice. I don't disagree with @brisvag, but I also really like getting rid of unused imports 😜

[djhoese]

Awesome! Nice job!