PR: Add QT_VERSION environment variable#478
Conversation
dalthviz
left a comment
dalthviz
left a comment
There was a problem hiding this comment.
Thank you @Bzero for your work here! Checked the CI failures and I think those were unrelated to the work being done here (trigger a new run and seems like checks are passing now). Left some initial comments but from a quick review I would say the changes make sense 👍
| def test_qt_version(): | ||
| """ | ||
| If QT_VERSION is specified, check that one of the correct Qt wrappers was used | ||
| """ | ||
|
|
||
| QT_VERSION = os.environ.get("QT_VERSION", "").lower() | ||
|
|
||
| if QT_VERSION == "qt5": | ||
| assert_qt5() | ||
| elif QT_VERSION == "qt6": | ||
| assert_qt6() |
There was a problem hiding this comment.
Not totally sure but, I think we are not setting the QT_VERSION env var over the CI so for the moment this test is passing without doing any assert?
There was a problem hiding this comment.
Yes, I don't think QT_VERSION is set in CI, ideally both possibilities should be checked. Do you happen to know how this works for the similar test test_qt_api?
There was a problem hiding this comment.
I think currently the test_qt_api is going through the else branch 😅 Checking the script that does the test setup (test.sh) I'm not seeing us setting up QT_API 🤔 Maybe we should try to check a way to setup QT_API (and QT_VERSION) for the tests (I think there is a USE_QT_API that can be set for test but I think we are not using it here either). What do you think @ccordoba12 ?
There was a problem hiding this comment.
I also think that's a good idea but I don't understand very well how to set up those env vars in our CIs.
Co-authored-by: Daniel Althviz Moré <[email protected]>
dalthviz
changed the title
QT_VERSION environment variableQT_VERSION environment variable
4 participants
This PR adds the option to specify the Qt version qtpy will use via the
QT_VERSIONenvironment variable and should resolve #476.If the
QT_VERSTIONbut not theQT_APIenvironment variable is set, theAPIis selected based onQT_VERSTION(and follows the usual orderPyQt5->PySide2->PyQt6->PySide6if a binding is not installed). If both environment variables are set,QT_APItakes precedence. If the version cannot be satisfied or the environment variables are incompatible warnings are triggered.Please have an extra eye on the test when reviewing as I am not perfectly sure it will behave correctly in the CI environment.