BUG: Safe app window close (#258)

.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}

.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

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

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

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):