From a792becb8231cae5982922c11a4d2d0cb868ceba Mon Sep 17 00:00:00 2001 From: Eric Larson Date: Thu, 5 Jan 2023 16:26:40 -0500 Subject: [PATCH] BUG: Safe app window close (#258) --- .ci/azure-pipelines.yml | 39 +++++++++++++++---------------------- .ci/macos-install-python.sh | 32 ------------------------------ .github/workflows/ci.yml | 19 +++++++++++++++--- pytest.ini | 1 + pyvistaqt/plotting.py | 5 ++--- tests/test_plotting.py | 34 +++++++++++++++++++------------- 6 files changed, 56 insertions(+), 74 deletions(-) delete mode 100755 .ci/macos-install-python.sh diff --git a/.ci/azure-pipelines.yml b/.ci/azure-pipelines.yml index 63ecd52c..bfa79ab4 100644 --- a/.ci/azure-pipelines.yml +++ b/.ci/azure-pipelines.yml @@ -50,7 +50,7 @@ stages: steps: - task: UsePythonVersion@0 inputs: - versionSpec: '3.7' + versionSpec: '3.10' - script: | pip install codespell pydocstyle[toml] make doctest @@ -72,10 +72,8 @@ stages: strategy: matrix: - Python37: - python.version: '3.7' - Python38: - python.version: '3.8' + Python310: + python.version: '3.10' steps: - task: UsePythonVersion@0 @@ -96,7 +94,7 @@ stages: - script: | sudo apt-get install python3-tk pip install -r requirements_test.txt - pip install PyQt5==5.11.3 + pip install PyQt5==5.12.* python -c "import pyvista; print(pyvista.Report())" which python pip list @@ -110,7 +108,6 @@ stages: - script: | # this must be right after the core API bash <(curl -s https://codecov.io/bash) displayName: 'Upload coverage to codecov.io' - condition: eq(variables['python.version'], '3.7') - script: | pytest -v --doctest-modules pyvistaqt @@ -121,7 +118,7 @@ stages: python setup.py sdist twine upload --skip-existing dist/pyvistaqt* displayName: 'Upload to PyPi' - condition: and(eq(variables['python.version'], '3.7'), contains(variables['Build.SourceBranch'], 'refs/tags/')) + condition: and(eq(variables['python.version'], '3.10'), contains(variables['Build.SourceBranch'], 'refs/tags/')) env: TWINE_USERNAME: $(twine.username) TWINE_PASSWORD: $(twine.password) @@ -149,7 +146,7 @@ stages: conda config --add channels conda-forge conda env create --quiet -n pyvistaqt-env --file environment.yml conda activate pyvistaqt-env - pip install PyQt5==5.11.3 + pip install PyQt5==5.12.* pip install -e . conda list which python @@ -179,10 +176,8 @@ stages: strategy: maxParallel: 4 matrix: - Python37-64bit: - PYTHON_VERSION: '3.7' - Python38-64bit: - PYTHON_VERSION: '3.8' + Python310-64bit: + PYTHON_VERSION: '3.10' steps: - task: UsePythonVersion@0 inputs: @@ -217,17 +212,15 @@ stages: python.architecture: 'x64' strategy: matrix: - Python37: - python.version: '3.7' - Python38: - python.version: '3.8' + Python310: + python.version: '3.10' pool: vmImage: 'macOS-10.15' steps: - - script: | - .ci/macos-install-python.sh '$(python.version)' - python -c "import sys; print(sys.version)" - displayName: Install Python + - task: UsePythonVersion@0 + inputs: + versionSpec: '$(python.version)' + displayName: 'Get Python' - script: | python -m pip install PyQt5>=5.11 @@ -257,8 +250,8 @@ stages: steps: - task: UsePythonVersion@0 inputs: - versionSpec: 3.7 - displayName: 'Use Python 3.7' + versionSpec: '3.10' + displayName: 'Use Python 3.10' - script: | git config --global user.name ${GH_NAME} diff --git a/.ci/macos-install-python.sh b/.ci/macos-install-python.sh deleted file mode 100755 index 3519afff..00000000 --- a/.ci/macos-install-python.sh +++ /dev/null @@ -1,32 +0,0 @@ -#!/usr/bin/env bash - -PYTHON_VERSION="$1" - -case $PYTHON_VERSION in -3.7) - FULL_VERSION=3.7.7 - ;; -3.8) - FULL_VERSION=3.8.3 - ;; -esac - -INSTALLER_NAME=python-$FULL_VERSION-macosx10.9.pkg -URL=https://www.python.org/ftp/python/$FULL_VERSION/$INSTALLER_NAME - -PY_PREFIX=/Library/Frameworks/Python.framework/Versions - -set -e -x - -curl $URL > $INSTALLER_NAME - -sudo installer -pkg $INSTALLER_NAME -target / - -sudo rm /usr/local/bin/python -sudo ln -s /usr/local/bin/python$PYTHON_VERSION /usr/local/bin/python - -which python -python --version -python -m ensurepip -python -m pip install --upgrade pip -python -m pip install setuptools twine wheel diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fe6b41ae..1c4c3c2e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,4 +1,7 @@ name: 'CI' +concurrency: + group: ${{ github.workflow }}-${{ github.event.number }}-${{ github.event.ref }} + cancel-in-progress: true on: push: branches: @@ -34,9 +37,8 @@ jobs: name: 'Setup python' - run: | python -m pip install --upgrade pip wheel - pip install --progress-bar off https://github.com/pyvista/pyvista/zipball/main - pip install --progress-bar off https://github.com/spyder-ide/qtpy/zipball/master pip install --upgrade -r requirements_test.txt + pip install --upgrade --progress-bar off https://github.com/pyvista/pyvista/zipball/main https://github.com/spyder-ide/qtpy/zipball/master name: 'Install dependencies with pip' - run: pip install ${{ matrix.qt }} name: 'Install Qt binding' @@ -67,7 +69,18 @@ jobs: fail-fast: false matrix: os: ['ubuntu-latest', 'macos-latest', 'windows-latest'] - pyvista: ['0.32', '0.33', '0.34'] + pyvista: ['0'] # All OSes on latest release + include: # and Linux for older ones + - os: 'ubuntu-latest' + pyvista: '0.32' + - os: 'ubuntu-latest' + pyvista: '0.33' + - os: 'ubuntu-latest' + pyvista: '0.34' + - os: 'ubuntu-latest' + pyvista: '0.35' + - os: 'ubuntu-latest' + pyvista: '0.36' defaults: run: shell: bash diff --git a/pytest.ini b/pytest.ini index 50997d7b..52f3c7f5 100644 --- a/pytest.ini +++ b/pytest.ini @@ -13,3 +13,4 @@ filterwarnings = ignore:.*`np.object` is a deprecated alias.*:DeprecationWarning ignore:.*`np.long` is a deprecated alias:DeprecationWarning ignore:.*Converting `np\.character` to a dtype is deprecated.*:DeprecationWarning + ignore:.*ImportDenier.*not found.*:ImportWarning diff --git a/pyvistaqt/plotting.py b/pyvistaqt/plotting.py index 4f688e7e..878f9608 100644 --- a/pyvistaqt/plotting.py +++ b/pyvistaqt/plotting.py @@ -254,7 +254,7 @@ def __init__( self.render_timer = QTimer(parent=parent) if float(auto_update) > 0.0: # Can be False as well # Spawn a thread that updates the render window. - # Sometimes directly modifiying object data doesn't trigger + # Sometimes directly modifying object data doesn't trigger # Modified() and upstream objects won't be updated. This # ensures the render window stays updated without consuming too # many resources. @@ -737,8 +737,7 @@ def window_size(self, window_size: QSize) -> None: def __del__(self) -> None: # pragma: no cover """Delete the qt plotter.""" - if not self._closed: - self.app_window.close() + self.close() def add_callback( self, func: Callable, interval: int = 1000, count: Optional[int] = None diff --git a/tests/test_plotting.py b/tests/test_plotting.py index d099b8a2..d111c147 100644 --- a/tests/test_plotting.py +++ b/tests/test_plotting.py @@ -23,6 +23,9 @@ from pyvistaqt.utils import _setup_application, _create_menu_bar, _check_type +WANT_AFTER = 0 if Version(pyvista.__version__) >= Version('0.37') else 1 + + class TstWindow(MainWindow): def __init__(self, parent=None, show=True, off_screen=True): MainWindow.__init__(self, parent) @@ -318,8 +321,7 @@ def test_qt_interactor(qtbot, plotting): assert not hasattr(vtk_widget, "iren") assert vtk_widget._closed - # check that BasePlotter.__init__() is called only once - assert len(_ALL_PLOTTERS) == 1 + assert len(_ALL_PLOTTERS) == WANT_AFTER @pytest.mark.parametrize('show_plotter', [ @@ -581,17 +583,20 @@ def test_background_plotting_toolbar(qtbot, plotting): plotter.close() +@pytest.mark.skipif( + platform.system() == 'Windows', reason='Segfaults on Windows') def test_background_plotting_menu_bar(qtbot, plotting): with pytest.raises(TypeError, match='menu_bar'): - p = BackgroundPlotter(off_screen=False, menu_bar="foo") - p.close() + BackgroundPlotter(off_screen=False, menu_bar="foo") - plotter = BackgroundPlotter(off_screen=False, menu_bar=False) + plotter = BackgroundPlotter( + off_screen=False, menu_bar=False, update_app_icon=False) assert plotter.main_menu is None assert plotter._menu_close_action is None plotter.close() - plotter = BackgroundPlotter(off_screen=False) # menu_bar=True + # menu_bar=True + plotter = BackgroundPlotter(off_screen=False, update_app_icon=False) assert_hasattr(plotter, "app_window", MainWindow) assert_hasattr(plotter, "main_menu", QMenuBar) @@ -607,9 +612,13 @@ def test_background_plotting_menu_bar(qtbot, plotting): window.show() # EDL action - assert not hasattr(plotter.renderer, 'edl_pass') + if hasattr(plotter.renderer, '_render_passes'): + obj, attr = plotter.renderer._render_passes, '_edl_pass' + else: + obj, attr = plotter.renderer, 'edl_pass' + assert getattr(obj, attr, None) is None plotter._edl_action.trigger() - assert hasattr(plotter.renderer, 'edl_pass') + assert getattr(obj, attr, None) is not None # and now test reset plotter._edl_action.trigger() @@ -626,13 +635,13 @@ def test_background_plotting_menu_bar(qtbot, plotting): assert plotter._last_update_time == -np.inf -def test_drop_event(tmpdir): +def test_drop_event(tmpdir, qtbot): output_dir = str(tmpdir.mkdir("tmpdir")) filename = str(os.path.join(output_dir, "tmp.vtk")) mesh = pyvista.Cone() mesh.save(filename) assert os.path.isfile(filename) - plotter = BackgroundPlotter() + plotter = BackgroundPlotter(update_app_icon=False, show=True) point = QPointF(0, 0) data = QMimeData() data.setUrls([QUrl(filename)]) @@ -668,7 +677,7 @@ def test_drag_event(tmpdir): plotter.close() -def test_gesture_event(): +def test_gesture_event(qtbot): plotter = BackgroundPlotter() gestures = [QPinchGesture()] event = QGestureEvent(gestures) @@ -834,8 +843,7 @@ def test_background_plotting_close(qtbot, close_event, empty_scene, plotting): assert not hasattr(window.vtk_widget, "iren") assert plotter._closed - # check that BasePlotter.__init__() is called only once - assert len(_ALL_PLOTTERS) == 1 + assert len(_ALL_PLOTTERS) == WANT_AFTER def test_multiplotter(qtbot, plotting):