Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG: Multiple PyQt/PySide Versions Not Supported #437

Open
adam-grant-hendry opened this issue Jun 27, 2022 · 9 comments
Open

BUG: Multiple PyQt/PySide Versions Not Supported #437

adam-grant-hendry opened this issue Jun 27, 2022 · 9 comments

Comments

@adam-grant-hendry
Copy link

adam-grant-hendry commented Jun 27, 2022

Bug

I ran into this issue when trying to fix a bug with pyvistaqt. It appears, for some reason, when all of

  • PyQt5
  • PySide2
  • PyQt6
  • PySide6

are installed, pytest-qt fails to create a QApplication instance, causing tests to silently fail.

NOTE: QT_API is not set to anything on my system when this occurs.

System Information

OS: Windows 10, x64-bit
Python: 3.8.10 x64-bit (CPython)
pytest-qt: 4.1.0

Steps to Reproduce

  1. Clone the pyvistaqt repo and cd into the folder
git clone https://github.com/pyvista/pyvistaqt.git
cd pyvistaqt
  1. Create and activate a virtual environment
py -m venv .venv && .venv\Scripts\Activate.ps1
  1. Install dependencies
py -m pip install --upgrade pip
py -m pip install -r requirements_test.txt
py -m pip install -r requirements_docs.txt
  1. First, install only PySide6
py -m pip install pyside6
  1. Sanity Check: Run tests to verify everything is working correctly (All Pass)
pytest
  1. Now install PyQt5, PySide2, and PyQt6
py -m pip install pyqt5 pyside2 pyqt6
  1. Rerun pytest and tests will fail silently
PS> pytest
=================================== test session starts =======================================
platform win32 -- Python 3.8.10, pytest-7.1.2, pluggy-1.0.0
PySide6 6.3.1 -- Qt runtime 6.3.1 -- Qt compiled 6.3.1
rootdir: %USERPROFILE%\Code\external\pyvistaqt-demo\pyvistaqt, configfile: pytest.ini
plugins: cov-3.0.0, memprof-0.2.0, mypy-plugins-1.9.3, mypy-testing-0.0.11, qt-4.1.0, sphinx-0.4.0
collected 2 items

tests\test_plotting.py
  1. Re-run pytest in verbose mode while printing output to stdout/stderr
PS> pytest -v -s

==================================== test session starts ======================================
platform win32 -- Python 3.8.10, pytest-7.1.2, pluggy-1.0.0 -- %USERPROFILE%\code\external\pyvistaqt-demo\pyvistaqt\.venv\scripts\python.exe
cachedir: .pytest_cache
PySide6 6.3.1 -- Qt runtime 6.3.1 -- Qt compiled 6.3.1
rootdir: %USERPROFILE%\Code\external\pyvistaqt-demo\pyvistaqt, configfile: pytest.ini
plugins: cov-3.0.0, memprof-0.2.0, mypy-plugins-1.9.3, mypy-testing-0.0.11, qt-4.1.0, sphinx-0.4.0
collected 2 items

tests/test_plotting.py::test_create_menu_bar QWidget: Must construct a QApplication before a QWidget
  1. Add an assert to the offending test for details
def test_create_menu_bar(qtbot):
    assert QtWidgets.QApplication.instance() is not None
    menu_bar = _create_menu_bar(parent=None)
    qtbot.addWidget(menu_bar)
  1. Rerun test in verbose mode while outputing to stdout/stderr
PS> pytest -v -s
==================================== test session starts ======================================
platform win32 -- Python 3.8.10, pytest-7.1.2, pluggy-1.0.0 -- %USERPROFILE%\code\external\pyvistaqt-demo\pyvistaqt\.venv\scripts\python.exe
cachedir: .pytest_cache
PySide6 6.3.1 -- Qt runtime 6.3.1 -- Qt compiled 6.3.1
rootdir: %USERPROFILE%\Code\external\pyvistaqt-demo\pyvistaqt, configfile: pytest.ini
plugins: cov-3.0.0, memprof-0.2.0, mypy-plugins-1.9.3, mypy-testing-0.0.11, qt-4.1.0, sphinx-0.4.0
collected 2 items

tests/test_plotting.py::test_create_menu_bar FAILED
tests/test_qt.py::test_no_qt_binding PASSED

========================================= FAILURES ======================================
____________________________________________________________ test_create_menu_bar _________________________________________________________

qtbot = <pytestqt.qtbot.QtBot object at 0x00000237D940E160>

    def test_create_menu_bar(qtbot):
>       assert QtWidgets.QApplication.instance() is not None
E       AssertionError: assert None is not None
E        +  where None = <built-in function instance>()
E        +    where <built-in function instance> = <class 'PyQt5.QtWidgets.QApplication'>.instance
E        +      where <class 'PyQt5.QtWidgets.QApplication'> = QtWidgets.QApplication

tests\test_plotting.py:79: AssertionError
================================== memory consumption estimates ==============================
tests::test_plotting.py::test_create_menu_bar  - 40.0 KB
================================== short test summary info ====================================
FAILED tests/test_plotting.py::test_create_menu_bar - AssertionError: assert None is not None
================================== 1 failed, 1 passed in 0.79s ===================================

It appears there is a disconnect between the python Qt library registered by pytest:

PySide6 6.3.1 -- Qt runtime 6.3.1 -- Qt compiled 6.3.1

and the one picked up by pytest-qt

>       assert QtWidgets.QApplication.instance() is not None
E       AssertionError: assert None is not None
E        +  where None = <built-in function instance>()
E        +    where <built-in function instance> = <class 'PyQt5.QtWidgets.QApplication'>.instance
E        +      where <class 'PyQt5.QtWidgets.QApplication'> = QtWidgets.QApplication

Expected Behavior

pytest-qt can be run with multiple versions of python Qt libraries installed simultaneously for testing purposes.

Actual Behavior

pytest-qt has a disconnect between the python Qt binding registered with pytest and the one it uses.

@nicoddemus
Copy link
Member

Thanks @adam-grant-hendry!

That's strange, I did a quick look at the qt_compat code and nothing jumps to the eyes.

Just to confirm, if you set QT_API explicitly, things work for you?

@adam-grant-hendry
Copy link
Author

My solution was to uninstall the other bindings and only use PySide6, but let me try setting QT_API and seeing if that works.

@adam-grant-hendry
Copy link
Author

I can confirm, after step 8 in the above steps to reproduce (pytest fails), setting QT_API to pyside6:

PS> $env:QT_API = "pyside6"
PS> $env:QT_API
pyside6

causes the tests to work for PySide6, but the others do not. Setting QT_API to any of "pyqt5", "pyside2", or "pyqt6" results in the same failure.

@adam-grant-hendry
Copy link
Author

This is odd because the docs state:

Works with either PySide6, PySide2, PyQt6 or PyQt5, picking whichever is available on the system, giving preference to the first one installed in this order:

  • PySide6
  • PySide2
  • PyQt6
  • PyQt5

@adam-grant-hendry
Copy link
Author

@nicoddemus Are you able to reproduce my results?

@The-Compiler
Copy link
Member

The-Compiler commented Jul 2, 2022

This happens because pyvistaqt uses QtPy which uses PyQt5 if QT_API was not set, but pytest-qt defaults to PySide6 without PYTEST_QT_API (or qt_api in the pytest config). Thus, you end up with a PySide6 QApplication but a PyQt5 widget being created by the code.

Doesn't seem like a bug to me, just a misconfiguration on your (or the project's) side. Maybe #412 would help against this kind of thing, because in qt_compat.set_qt_api, the only already imported wrapper is PyQt5.

@adam-grant-hendry
Copy link
Author

adam-grant-hendry commented Jul 2, 2022

Doesn't seem like a bug to me, just a misconfiguration on your (or the project's) side.

Yes, you're right. What confused me was QtPy doesn't read qt_api in the pytest config, only pytest-qt does. Hence, one has to do (in a pyproject.toml):

[tool.pytest.ini_options]
qt_api = "pyside6"
env = [
    "D:QT_API=pyside6"
]

or

[tool.pytest.ini_options]
env = [
    "D:PYTEST_QT_API=pyside6",
    "D:QT_API=pyside6"
]

The commonality between the naming of the arguments made me think I didn't also have to specify the QT_API environment variable.

Maybe #412 would help against this kind of thing.

Perhaps yes. Definitely a good first step. However, can I request the following also be implemented?:

  1. Rename the lowercase qt_api to pytest_qt_api to avoid confusion

  2. If qtpy is installed and the environment variable QT_API is set, use that (i.e. set PYTEST_QT_API to the value specified in QT_API).

    • Would require updating the user documentation to note that pytest-qt uses the qtpy QT_API environment variable if set and both PYTEST_QT_API/pytest_qt_api and QT_API should not be set simultaneously when qtpy is installed

    (ASIDE: Wouldn't make sense to permit PYTEST_QT_API to override QT_API since pytest-qt will always fail if both are set to different values and using both to set the same value is rather redundant.)

    • e.g.

      qtpy_api = os.environ.get("QT_API")
      pytestqt_api = os.environ.get("PYTEST_QT_API")
      
      if 'qtpy' in sys.modules:
         if qtpy_api is not None and pytestqt_api is not None:
             msg  = f"PYTEST_QT_API and QT_API cannot both be set when qtpy is installed."
             raise pytest.UsageError(msg)

However it is implemented, a nice pytest.UsageError would be preferable over silently failing. In addition, it would be nice to only have to rely on one environment variable in the event qtpy is installed (rather than setting multiple environment variables to the same thing).

@The-Compiler
Copy link
Member

  1. Rename the lowercase qt_api to pytest_qt_api to avoid confusion

It's a pytest setting in the pytest config file - so that seems both redundant, and a very big churn for projects using pytest-qt already, for questionable benefit.

  1. If qtpy is installed and the environment variable QT_API is set, use that (i.e. set PYTEST_QT_API to the value specified in QT_API).

Hm, maybe. Though I'm a bit hesitant to have more and more magic in the backend selection. Also it probably wouldn't be useful after #412 anyways.

  • Would require updating the user documentation to note that pytest-qt uses the qtpy QT_API environment variable if set and both PYTEST_QT_API/pytest_qt_api and QT_API should not be set simultaneously when qtpy is installed

(ASIDE: Wouldn't make sense to permit PYTEST_QT_API to override QT_API since pytest-qt will always fail if both are set to different values and using both to set the same value is rather redundant.)

Note this wouldn't solve the problem you had (which was due to the autodetection). Maybe it could be a warning, but I don't think it should be an outright error. We don't know how people actually use pytest-qt and qtpy in the wild - just because it's imported it doesn't mean that all tests necessarily use it, for example.

All in all, I feel like we're introducing lots and lots of additional complexity and 5 different ways to select the backend to use, and I'm not sure that's a good idea.

@adam-grant-hendry
Copy link
Author

It's a pytest setting in the pytest config file - so that seems both redundant

Several other test packages rely on Qt, not just pytestqt. The fact that QT_API exists for qtpy is an indicator: what package does the setting qt_api belong to? Just pytest-qt? What if other packages come along that rely on the Qt binding being used?

a very big churn for projects using pytest-qt already, for questionable benefit.

Is it really? They would just update their config variable with a prefix pytest_. Explicit is better than implicit.

Also it probably wouldn't be useful after #412 anyways.

#412 won't solve this problem. It's still preferring PySide6 first when QtPy perfers PyQt5 first. There will still be a conflict when you have multiple Qt bindings installed as part of a test suite (as I did)

All in all, I feel like we're introducing lots and lots of additional complexity and 5 different ways to select the backend to use, and I'm not sure that's a good idea.

It's really not that big of a change; there aren't 5 ways to select the backend. You could even make it as simple as if QT_API is set, just use that, regardless of whether or not qtpy is actually installed.

I'm happy to add to #412 if you like.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants