PR: Add missing imports in QtOpenGL

[renefritze]

No description provided.

[dalthviz]

Thank you for helping with this @renefritze ! Could it be possible for you to add a test for this module? Otherwise this LGTM 👍

Let us know!

[renefritze]

Thank you for helping with this @renefritze ! Could it be possible for you to add a test for this module? Otherwise this LGTM +1

Let us know!

Sure thing. Happy to help. Fix was basically shorter than any bug report could be 😉

[renefritze]

Ofc now there's a test failure after I just added all the stuff from the Pyside2 docs to check.
The failures seem to be different across jobs and setups. And this for example contradicts the docs, I guess?

[dalthviz]

Checking, seems like a couple of things are happening:

For PyQt5 the mayority of classes related with OpenGL are located in QtGui. For compatibility I think we should add them at QtOpenGL.py with something like this under the PYQT5 validation:

from PyQt5.QtGui import QOpenGLBuffer, QOpenGLFramebufferObject, QOpenGLFramebufferObjectFormat, QOpenGLShader, QOpenGLShaderProgram

In the case of PyQt6 there are more modules by default on QtOpenGL but still some are in QtGui. Something like this under the PYQT6 validation should help:

from PyQt6.QtGui import QOpenGLContext

Something similar happens with PySide6. Something like this under the PYSIDE6 validation should help:

from PySide6.QtGui import QOpenGLContext

Beside that seems like for for PySide2 the classes don't have as prefix QOpenGL* but QGL*. So for compatibility I think we need to do some aliasing with something like this under the PYSIDE2 validation:

QOpenGLBuffer = QGLBuffer
QOpenGLContext = QGLContext
QOpenGLFormat = QGLFormat
QOpenGLFramebufferObject = QGLFramebufferObject
QOpenGLFramebufferObjectFormat = QGLFramebufferObjectFormat
QOpenGLShader = QGLShader
QOpenGLShaderProgram = QGLShaderProgram

Seems like currently the test are failing for PySide2 because of an uppercase b for QGLFramebufferObject but I think we should test for the QOpenGL* variant of the classes.

Let us know if you need any help with this @renefritze !

[dalthviz]

Pinging @ccordoba12 and @CAM-Gerlach just in case

[renefritze]

I'm not sure this is what you had in mind, @dalthviz, but aside from an unrelated error at least the tests are now green.

[pijyoi]

Beside that seems like for for PySide2 the classes don't have as prefix QOpenGL* but QGL*. So for compatibility I think we need to do some aliasing with something like this under the PYSIDE2 validation:

QOpenGLBuffer = QGLBuffer

I don't think QOpenGL* and QGL* are interchangeable. The QGL* classes are Qt4, provided in Qt5 for backwards compatibility. The QOpenGL* classes are available in both Qt5 and Qt6.

In [5]: from PySide2 import QtGui

In [6]: [x for x in dir(QtGui) if x.startswith('QOpenGL')]
Out[6]:
['QOpenGLBuffer',
 'QOpenGLContext',
 'QOpenGLContextGroup',
 'QOpenGLDebugLogger',
 'QOpenGLDebugMessage',
 'QOpenGLExtraFunctions',
 'QOpenGLFramebufferObject',
 'QOpenGLFramebufferObjectFormat',
 'QOpenGLFunctions',
 'QOpenGLPixelTransferOptions',
 'QOpenGLShader',
 'QOpenGLShaderProgram',
 'QOpenGLTexture',
 'QOpenGLTextureBlitter',
 'QOpenGLTimeMonitor',
 'QOpenGLTimerQuery',
 'QOpenGLVersionProfile',
 'QOpenGLVertexArrayObject',
 'QOpenGLWindow']

[dalthviz]

I don't think QOpenGL* and QGL* are interchangeable. The QGL* classes are Qt4, provided in Qt5 for backwards compatibility. The QOpenGL* classes are available in both Qt5 and Qt6.

Thank you @pijyoi for the feedback! Reading a little bit more the qt docs I think you are totally right!

I would say that then what we need to do is to add some imports inside qtpy/QtGui.py for PYQT6 and PYSIDE6 importing QtOpenGL so something like :

from PyQt6.QtOpenGL import *

from PySide6.QtOpenGL import *

On the other hand, inside qtpy/QtOpenGL.py to keep things simetric we could under PYSIDE2 add the missing QtGui imports (as was done for PYQT5). So something like:

from PySide2.QtGui import QOpenGLBuffer, QOpenGLFramebufferObject, QOpenGLFramebufferObjectFormat, QOpenGLShader, QOpenGLShaderProgram

And don't do the aliasing for QOpenGL* = QGL* and remove the QOpenGlContext import being done inside PYQT6

For the test, I would say that we need to only check for the QOpenGL classes available in Qt5 and Qt6

Pinging @ccordoba12 and @CAM-Gerlach just in case

Also, thank you so much @renefritze for all the help with this and sorry for the incorrect initial guidance for the changes!

[CAM-Gerlach]

For what its worth, given your expertise is much greater in this area, seems sensible to me!

[renefritze]

Just to make sure I understood what's to be tested correctly.

def get(mod):
    ret = []
    for sub in ('QtGui', 'QtOpenGL'):
        ret.extend([f for f in dir(getattr(mod, sub)) if f.startswith('QOpenGL')])
    return set(ret)

import PySide2, PyQt5, PySide6, PyQt6
sets = [get(PySide2), get(PySide6), get(PyQt5), get(PyQt6)]
common_opengl_classes = sets[0].intersection(*sets[1:])

I've added a test for everything in common_opengl_classes and then adjusted what needs to be pulled into qtpy.QOpenGL accordingly

[dalthviz]

Thank you so much again @renefritze for your help and patience !

Yep the common_opengl_classes is what I think needs to be tested. Also, I left a suggestion to fix the failing tests (there is a missing a change in an import which is using PySide6 instead of PyQt6 in qtpy/QtOpenGL.py), otherwise this LGTM 👍

[renefritze]

You're very welcome @dalthviz. I hope this can make its way into a bugfix release soon then.