PR: Add missing imports in QtOpenGL#307
Conversation
dalthviz
left a comment
There was a problem hiding this comment.
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!
Sure thing. Happy to help. Fix was basically shorter than any bug report could be 😉 |
|
Ofc now there's a test failure after I just added all the stuff from the Pyside2 docs to check. |
|
Checking, seems like a couple of things are happening: For from PyQt5.QtGui import QOpenGLBuffer, QOpenGLFramebufferObject, QOpenGLFramebufferObjectFormat, QOpenGLShader, QOpenGLShaderProgramIn the case of from PyQt6.QtGui import QOpenGLContextSomething similar happens with from PySide6.QtGui import QOpenGLContextBeside that seems like for for QOpenGLBuffer = QGLBuffer
QOpenGLContext = QGLContext
QOpenGLFormat = QGLFormat
QOpenGLFramebufferObject = QGLFramebufferObject
QOpenGLFramebufferObjectFormat = QGLFramebufferObjectFormat
QOpenGLShader = QGLShader
QOpenGLShaderProgram = QGLShaderProgram
Seems like currently the test are failing for Let us know if you need any help with this @renefritze ! |
|
Pinging @ccordoba12 and @CAM-Gerlach just in case |
deceed0 to
7ce7668
Compare
|
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. |
I don't think 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'] |
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 On the other hand, inside And don't do the aliasing for 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! |
|
For what its worth, given your expertise is much greater in this area, seems sensible to me! |
7ce7668 to
76a89ba
Compare
|
Just to make sure I understood what's to be tested correctly. I've added a test for everything in |
There was a problem hiding this comment.
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 👍
Co-authored-by: Daniel Althviz Moré <d.althviz10@uniandes.edu.co>
|
You're very welcome @dalthviz. I hope this can make its way into a bugfix release soon then. |
No description provided.