Skip to content

PR: Skip import of QOpenGLTime* on architectures where not available#333

Merged
dalthviz merged 3 commits intospyder-ide:masterfrom
juliangilbey:conditionally-skip-opengl-time
Apr 8, 2022
Merged

PR: Skip import of QOpenGLTime* on architectures where not available#333
dalthviz merged 3 commits intospyder-ide:masterfrom
juliangilbey:conditionally-skip-opengl-time

Conversation

@juliangilbey
Copy link
Copy Markdown
Contributor

QOpenGLTimeMonitor and QOpenGLTimerQuery do not exist on some architectures such as armhf; the PyQt5 and PySide2 code has a check for whether these should be included in the package. Since they are imported unconditionally, importing qtpy.QtOpenGL fails on those architectures. This patch imports those two classes within a try/except block to allow for this to gracefully fail, and comments out the corresponding assertions in the test suite.

@dalthviz dalthviz changed the title Skip import of QOpenGLTime* on architectures where not available PR: Skip import of QOpenGLTime* on architectures where not available Apr 8, 2022
@dalthviz dalthviz added this to the v2.1.0 milestone Apr 8, 2022
Copy link
Copy Markdown
Member

@dalthviz dalthviz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @juliangilbey thanks for checking into this and submitting a fix! I left some comments regarding the addition of some specific architectures examples for future reference. Otherwise this LGTM 👍

Copy link
Copy Markdown
Member

@dalthviz dalthviz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @juliangilbey !

@dalthviz dalthviz merged commit c762aac into spyder-ide:master Apr 8, 2022
@juliangilbey juliangilbey deleted the conditionally-skip-opengl-time branch April 8, 2022 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants