diff --git a/CLAUDE.md b/CLAUDE.md index d79a1005..c9e273d7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -133,8 +133,9 @@ must be reached through `qgis.PyQt`. GUI modules therefore never import `PyQt6` `dicogis/ui/qt_compat/`, which mirrors the `qgis.PyQt` layout: `from dicogis.ui.qt_compat.QtWidgets import QWidget`, `from dicogis.ui.qt_compat import uic`. The binding is picked once, at first -import, from `DICOGIS_QT_API` (`auto`: `qgis.PyQt` if QGIS is already loaded, -PyQt6 otherwise; `qgis`/`pyqt6` force it). Add a submodule there when a new Qt +import, from `DICOGIS_QT_API` (`auto`: `qgis.PyQt` if a `QgsApplication` is +running — QGIS merely imported isn't enough, pgserviceparser >= 3 imports it on +its own when installed —, PyQt6 otherwise; `qgis`/`pyqt6` force it). Add a submodule there when a new Qt module is needed. `tests/test_ui_qt_compat.py` fails on any direct binding import under `dicogis/ui`. Don't use Qt APIs newer than 6.8. diff --git a/Dockerfile b/Dockerfile index 6140b38f..8e21e49a 100644 --- a/Dockerfile +++ b/Dockerfile @@ -43,7 +43,7 @@ RUN python3 -m pip install --no-cache-dir \ notify-py==0.3.43 \ numpy==2.5.2 \ openpyxl==3.1.5 \ - pgserviceparser==2.5.0 \ + pgserviceparser==3.0.3 \ pygments==2.21.0 \ requests==2.34.2 \ rich==15.0.0 \ diff --git a/dicogis/ui/qt_compat/__init__.py b/dicogis/ui/qt_compat/__init__.py index 2f412988..ecce9e38 100644 --- a/dicogis/ui/qt_compat/__init__.py +++ b/dicogis/ui/qt_compat/__init__.py @@ -13,8 +13,11 @@ The binding is picked once, at first import, from the `DICOGIS_QT_API` environment variable: -- `auto` (default): `qgis.PyQt` when QGIS is already loaded in the process, - PyQt6 otherwise. Standalone runs never import QGIS, even if installed. +- `auto` (default): `qgis.PyQt` when running inside QGIS, i.e. a + `QgsApplication` is running, PyQt6 otherwise. QGIS merely being imported is + not enough: some libraries import `qgis.PyQt` on their own when QGIS is + installed (pgserviceparser >= 3 does), which loads `qgis.core` in a + standalone process too. - `qgis`: always `qgis.PyQt`. - `pyqt6`: always PyQt6. @@ -43,6 +46,24 @@ # ################################## +def _qgis_application_is_running() -> bool: + """Tell whether this process runs inside QGIS (desktop, plugin, qgis_process). + + Never imports QGIS itself: only looks at a `qgis.core` already loaded. + + Returns: + True if the running Qt application is a QgsApplication + """ + qgis_core = sys.modules.get("qgis.core") + qgs_application = getattr(qgis_core, "QgsApplication", None) + if qgs_application is None: + return False + try: + return isinstance(qgs_application.instance(), qgs_application) + except Exception: + return False + + def resolve_qt_api(requested: str | None = None) -> str: """Determine which Qt binding the GUI must use. @@ -67,7 +88,7 @@ def resolve_qt_api(requested: str | None = None) -> str: ) if requested == "auto": - return "qgis" if sys.modules.get("qgis") is not None else "pyqt6" + return "qgis" if _qgis_application_is_running() else "pyqt6" return requested diff --git a/pyproject.toml b/pyproject.toml index 9c804c75..ba9c5978 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -32,7 +32,7 @@ dependencies = [ "notify-py>=0.3.42,<0.4", "numpy>=2.0.2,<2.6", "openpyxl>=3.0,<3.2", - "pgserviceparser>=2.4,<3", + "pgserviceparser>=3.0.3,<4", "requests>=2.32.3,<3", "typer>=0.24.1,<1", ] diff --git a/tests/test_ui_qt_compat.py b/tests/test_ui_qt_compat.py index dfaa09d4..8c9cc1e4 100644 --- a/tests/test_ui_qt_compat.py +++ b/tests/test_ui_qt_compat.py @@ -22,6 +22,7 @@ import os import subprocess import sys +import types from pathlib import Path # 3rd party @@ -41,9 +42,17 @@ QT_BINDINGS = ("PyQt6", "PyQt5", "PySide6", "qgis") # a stand-in for QGIS's qgis.PyQt shim, re-exporting PyQt6 like the real one -# does, plus a marker telling which binding a dicogis module ended up with +# does, plus a marker telling which binding a dicogis module ended up with, and +# a qgis.core whose QgsApplication is running only once a test says so _FAKE_QGIS_PYQT = { "qgis/__init__.py": "", + "qgis/core.py": ( + "class QgsApplication:\n" + " running = None\n" + " @classmethod\n" + " def instance(cls):\n" + " return cls.running\n" + ), "qgis/PyQt/__init__.py": "", "qgis/PyQt/QtCore.py": "from PyQt6.QtCore import *\nFROM_FAKE_QGIS = True\n", "qgis/PyQt/QtGui.py": "from PyQt6.QtGui import *\n", @@ -51,6 +60,17 @@ "qgis/PyQt/uic.py": "from PyQt6.uic import *\n", } +# what each scenario runs before importing DicoGIS +_PRELOADS = { + "standalone": "", + # what pgserviceparser >= 3 does on import when QGIS is installed + "qgis_imported": "import qgis.PyQt\nimport qgis.core\n", + "qgis_running": ( + "import qgis.core\n" + "qgis.core.QgsApplication.running = qgis.core.QgsApplication()\n" + ), +} + _PROBE = ( "import sys\n" "{preload}" @@ -81,14 +101,14 @@ def fake_qgis(tmp_path: Path) -> Path: def _probe( - qt_api: str | None, fake_qgis_folder: Path, preload_qgis: bool + qt_api: str | None, fake_qgis_folder: Path, scenario: str ) -> subprocess.CompletedProcess: """Import the compat layer in a fresh interpreter and report what it picked. Args: qt_api: value of DICOGIS_QT_API, or None to leave it unset fake_qgis_folder: folder holding the fake qgis package, put on PYTHONPATH - preload_qgis: import qgis before DicoGIS, like a plugin running in QGIS + scenario: key of _PRELOADS, the QGIS state before DicoGIS is imported Returns: the finished subprocess, its stdout being " @@ -100,7 +120,7 @@ def _probe( env["PYTHONPATH"] = os.pathsep.join( [str(fake_qgis_folder), str(Path(__file__).parent.parent)] ) - code = _PROBE.format(preload="import qgis.PyQt\n" if preload_qgis else "") + code = _PROBE.format(preload=_PRELOADS[scenario]) # the command is this interpreter running a constant snippet: no untrusted input return subprocess.run( # noqa: S603 [sys.executable, "-c", code], @@ -116,34 +136,74 @@ def _probe( # ################################## +def _set_qgis_state(monkeypatch: pytest.MonkeyPatch, qgis_state: str) -> None: + """Fake the QGIS state of the process, in sys.modules. + + Args: + monkeypatch: pytest fixture + qgis_state: "absent", "imported" (qgis.core loaded, no QgsApplication, + as pgserviceparser >= 3 causes), "other_app" (a plain Qt application + runs) or "running" (inside QGIS) + """ + if qgis_state == "absent": + monkeypatch.delitem(sys.modules, "qgis.core", raising=False) + return + + class QgsApplication: + running = None + + @classmethod + def instance(cls) -> object: + return cls.running + + if qgis_state == "running": + QgsApplication.running = QgsApplication() + elif qgis_state == "other_app": + QgsApplication.running = object() + qgis_core = types.ModuleType("qgis.core") + qgis_core.QgsApplication = QgsApplication + monkeypatch.setitem(sys.modules, "qgis.core", qgis_core) + + @pytest.mark.parametrize( - ("requested", "qgis_loaded", "expected"), + ("requested", "qgis_state", "expected"), [ - ("auto", False, "pyqt6"), - ("auto", True, "qgis"), - (" AUTO ", False, "pyqt6"), - ("qgis", False, "qgis"), - ("pyqt6", True, "pyqt6"), - ("PyQt6", False, "pyqt6"), + ("auto", "absent", "pyqt6"), + ("auto", "imported", "pyqt6"), + ("auto", "other_app", "pyqt6"), + ("auto", "running", "qgis"), + (" AUTO ", "absent", "pyqt6"), + ("qgis", "absent", "qgis"), + ("pyqt6", "running", "pyqt6"), + ("PyQt6", "absent", "pyqt6"), ], ) def test_resolve_qt_api( - monkeypatch: pytest.MonkeyPatch, requested: str, qgis_loaded: bool, expected: str + monkeypatch: pytest.MonkeyPatch, requested: str, qgis_state: str, expected: str ) -> None: - if qgis_loaded: - monkeypatch.setitem(sys.modules, "qgis", object()) - else: - monkeypatch.delitem(sys.modules, "qgis", raising=False) + _set_qgis_state(monkeypatch, qgis_state) assert resolve_qt_api(requested) == expected +def test_resolve_qt_api_survives_a_failing_qgis_check( + monkeypatch: pytest.MonkeyPatch, +) -> None: + qgis_core = types.ModuleType("qgis.core") + qgis_core.QgsApplication = type( + "QgsApplication", (), {"instance": staticmethod(lambda: 1 / 0)} + ) + monkeypatch.setitem(sys.modules, "qgis.core", qgis_core) + + assert resolve_qt_api("auto") == "pyqt6" + + def test_resolve_qt_api_reads_the_environment(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setenv(QT_API_ENV_VAR, "qgis") assert resolve_qt_api() == "qgis" monkeypatch.setenv(QT_API_ENV_VAR, "") - monkeypatch.delitem(sys.modules, "qgis", raising=False) + _set_qgis_state(monkeypatch, "absent") assert resolve_qt_api() == "pyqt6" @@ -153,20 +213,22 @@ def test_resolve_qt_api_rejects_unknown_value() -> None: @pytest.mark.parametrize( - ("qt_api", "preload_qgis", "expected"), + ("qt_api", "scenario", "expected"), [ - (None, False, "pyqt6 False True True"), - (None, True, "qgis True True True"), - ("qgis", False, "qgis True True True"), - ("pyqt6", True, "pyqt6 False True True"), + (None, "standalone", "pyqt6 False True True"), + # QGIS installed and imported by a library, but not running: standalone + (None, "qgis_imported", "pyqt6 False True True"), + (None, "qgis_running", "qgis True True True"), + ("qgis", "standalone", "qgis True True True"), + ("pyqt6", "qgis_running", "pyqt6 False True True"), ], ) def test_submodules_follow_the_selected_binding( - fake_qgis: Path, qt_api: str | None, preload_qgis: bool, expected: str + fake_qgis: Path, qt_api: str | None, scenario: str, expected: str ) -> None: pytest.importorskip("PyQt6.QtWidgets", reason="PyQt6 (gui extra) required") - result = _probe(qt_api, fake_qgis, preload_qgis) + result = _probe(qt_api, fake_qgis, scenario) assert result.returncode == 0, result.stderr assert result.stdout.strip() == expected