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

MAINT: Add GC tests #66

Merged
merged 36 commits into from
Jan 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
f4a4af8
MAINT: Add GC tests
larsoner Oct 22, 2020
2705025
FIX: Missed one
larsoner Oct 22, 2020
c5e1cdd
FIX: Missed more
larsoner Oct 22, 2020
6cb8afe
FIX: More
larsoner Oct 22, 2020
fc01eaf
FIX: Revert deep
larsoner Oct 23, 2020
1bba575
FIX: Revert Azure addition
larsoner Oct 23, 2020
6341359
FIX: Try again?
larsoner Oct 23, 2020
82cebae
MAINT: Reorg
larsoner Oct 23, 2020
06f3fe9
FIX: Old VTK
larsoner Oct 23, 2020
d5a7101
FIX: Simplify
larsoner Oct 23, 2020
c59bf2e
FIX: Missing
larsoner Oct 23, 2020
9f8bb88
FIX: Pre
larsoner Oct 23, 2020
515627c
STY: Flake
larsoner Oct 23, 2020
f999e8c
FIX: Black
larsoner Oct 23, 2020
795e642
FIX: Sty
larsoner Oct 23, 2020
76acb42
STY: More
larsoner Oct 23, 2020
85eee8d
FIX: Just use master
larsoner Oct 23, 2020
269378a
Merge remote-tracking branch 'upstream/main' into mem
larsoner Jan 6, 2023
936e7da
FIX: Hopefully
larsoner Jan 6, 2023
dbab0de
FIX: Placeholder
larsoner Jan 6, 2023
7a44256
FIX: Rebase
larsoner Jan 6, 2023
c009a71
FIX: Order
larsoner Jan 6, 2023
39ca333
FIX: More
larsoner Jan 6, 2023
7cdde11
FIX: Flake
larsoner Jan 6, 2023
78d119c
FIX: Always bad
larsoner Jan 6, 2023
a52db70
FIX: Fixture
larsoner Jan 6, 2023
3339845
FIX: Use name
larsoner Jan 6, 2023
f363aae
FIX: Dynamic
larsoner Jan 6, 2023
c6e3b28
FIX: Pack
larsoner Jan 6, 2023
dcb59c9
FIX: Better
larsoner Jan 6, 2023
166b3a7
FIX: Revert
larsoner Jan 6, 2023
d33afea
FIX: Line
larsoner Jan 6, 2023
7b5baa9
STY: Flake
larsoner Jan 6, 2023
908330b
FIX: Del
larsoner Jan 6, 2023
a634517
FIX: Show
larsoner Jan 6, 2023
0cd0112
FIX: Skip
larsoner Jan 6, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions pytest.ini
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
[pytest]
junit_family=legacy
addopts =
--durations=10 -ra --cov-report= --tb=short
filterwarnings =
error
ignore::ResourceWarning
Expand Down
36 changes: 29 additions & 7 deletions pyvistaqt/editor.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"""This module contains the Qt scene editor."""

import weakref
from typing import List

from pyvista import Renderer
Expand Down Expand Up @@ -28,6 +29,7 @@ def __init__(self, parent: MainWindow, renderers: List[Renderer]) -> None:
"""Initialize the Editor."""
super().__init__(parent=parent)
self.renderers = renderers
del renderers

self.tree_widget = QTreeWidget()
self.tree_widget.setHeaderHidden(True)
Expand Down Expand Up @@ -78,19 +80,25 @@ def toggle(self) -> None:
def _get_renderer_widget(renderer: Renderer) -> QWidget:
widget = QWidget()
layout = QVBoxLayout()
axes = QCheckBox("Axes")
if hasattr(renderer, "axes_widget"):
axes.setChecked(renderer.axes_widget.GetEnabled())
else:
axes.setChecked(False)

renderer_ref = weakref.ref(renderer)
del renderer

# axes
def _axes_callback(state: bool) -> None:
renderer = renderer_ref()
if renderer is None: # pragma: no cover
return
if state:
renderer.show_axes()
else:
renderer.hide_axes()

axes = QCheckBox("Axes")
if hasattr(renderer, "axes_widget"):
axes.setChecked(renderer.axes_widget.GetEnabled())
else:
axes.setChecked(False)
axes.toggled.connect(_axes_callback)
layout.addWidget(axes)

Expand All @@ -105,9 +113,16 @@ def _get_actor_widget(actor: vtkActor) -> QWidget:
prop = actor.GetProperty()

# visibility
set_vis_ref = weakref.ref(actor.SetVisibility)

def _set_vis(visibility: bool) -> None: # pragma: no cover
set_vis = set_vis_ref()
if set_vis is not None:
set_vis(visibility)

visibility = QCheckBox("Visibility")
visibility.setChecked(actor.GetVisibility())
visibility.toggled.connect(actor.SetVisibility)
visibility.toggled.connect(_set_vis)
layout.addWidget(visibility)

if prop is not None:
Expand All @@ -116,7 +131,14 @@ def _get_actor_widget(actor: vtkActor) -> QWidget:
opacity = QDoubleSpinBox()
opacity.setMaximum(1.0)
opacity.setValue(prop.GetOpacity())
opacity.valueChanged.connect(prop.SetOpacity)
set_opacity_ref = weakref.ref(prop.SetOpacity)

def _set_opacity(opacity: float) -> None: # pragma: no cover
set_opacity = set_opacity_ref()
if set_opacity is not None:
set_opacity(opacity)

opacity.valueChanged.connect(_set_opacity)
tmp_layout.addWidget(QLabel("Opacity"))
tmp_layout.addWidget(opacity)
layout.addLayout(tmp_layout)
Expand Down
26 changes: 25 additions & 1 deletion pyvistaqt/plotting.py
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,10 @@ def _render(self, *args: Any, **kwargs: Any) -> BasePlotter.render:
@conditional_decorator(threaded, platform.system() == "Darwin")
def render(self) -> None:
"""Override the ``render`` method to handle threading issues."""
return self.render_signal.emit()
try:
return self.render_signal.emit()
except RuntimeError: # wrapped C/C++ object has been deleted
return None

@wraps(BasePlotter.enable)
def enable(self) -> None:
Expand Down Expand Up @@ -400,6 +403,16 @@ def close(self) -> None:
self.render_timer.stop()
BasePlotter.close(self)
QVTKRenderWindowInteractor.close(self)
# Qt LeaveEvent requires _Iren so we use _FakeIren instead of None
# to resolve the ref to vtkGenericRenderWindowInteractor
self._Iren = ( # pylint: disable=invalid-name,attribute-defined-outside-init
_FakeEventHandler()
)
for key in ("_RenderWindow", "renderer"):
try:
setattr(self, key, None)
except AttributeError:
pass


class BackgroundPlotter(QtInteractor):
Expand Down Expand Up @@ -1013,3 +1026,14 @@ def __getitem__(self, idx: Tuple[int, int]) -> Optional[BackgroundPlotter]:
row, col = idx
self._plotter = self._plotters[row * self._ncols + col]
return self._plotter


class _FakeEventHandler:
# pylint: disable=too-few-public-methods

def _noop(self, *args: tuple, **kwargs: dict) -> None:
pass

SetDPI = EnterEvent = MouseMoveEvent = LeaveEvent = SetSize = _noop
SetEventInformation = ConfigureEvent = SetEventInformationFlipY = _noop
KeyReleaseEvent = CharEvent = _noop
80 changes: 80 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,34 @@
import gc
import importlib
import inspect
import sys

import pytest
import pyvista
from pyvista.plotting import system_supports_plotting
import pyvistaqt

NO_PLOTTING = not system_supports_plotting()


def pytest_configure(config):
"""Configure pytest options."""
# Fixtures
for fixture in ('check_gc',):
config.addinivalue_line('usefixtures', fixture)
# Markers
for marker in ('allow_bad_gc', 'allow_bad_gc_pyside'):
config.addinivalue_line('markers', marker)


# Adapted from PyVista
def _is_vtk(obj):
try:
return obj.__class__.__name__.startswith('vtk')
except Exception: # old Python sometimes no __class__.__name__
return False


def _check_qt_installed():
try:
from qtpy import QtCore # noqa
Expand All @@ -17,6 +38,65 @@ def _check_qt_installed():
return True


@pytest.fixture(autouse=True)
def check_gc(request):
"""Ensure that all VTK objects are garbage-collected by Python."""
if 'test_ipython' in request.node.name: # XXX this keeps a ref
yield
return
try:
from qtpy import API_NAME
except Exception:
API_NAME = ''
marks = set(mark.name for mark in request.node.iter_markers())
if 'allow_bad_gc' in marks:
yield
return
if 'allow_bad_gc_pyside' in marks and API_NAME.lower().startswith('pyside'):
yield
return
gc.collect()
before = set(id(o) for o in gc.get_objects() if _is_vtk(o))
yield
pyvista.close_all()
gc.collect()
after = [
o
for o in gc.get_objects()
if _is_vtk(o) and id(o) not in before
]
msg = 'Not all objects GCed:\n'
for obj in after:
cn = obj.__class__.__name__
cf = inspect.currentframe()
referrers = [
v for v in gc.get_referrers(obj)
if v is not after and v is not cf
]
del cf
for ri, referrer in enumerate(referrers):
if isinstance(referrer, dict):
for k, v in referrer.items():
if k is obj:
referrers[ri] = 'dict: d key'
del k, v
break
elif v is obj:
referrers[ri] = f'dict: d[{k!r}]'
#raise RuntimeError(referrers[ri])
del k, v
break
del k, v
else:
referrers[ri] = f'dict: len={len(referrer)}'
else:
referrers[ri] = repr(referrer)
del ri, referrer
msg += f'{cn}: {referrers}\n'
del cn, referrers
assert len(after) == 0, msg


@pytest.fixture()
def plotting():
"""Require plotting."""
Expand Down
43 changes: 37 additions & 6 deletions tests/test_plotting.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import os
from packaging.version import Version
import platform
import weakref

import numpy as np
import pytest
Expand Down Expand Up @@ -197,6 +198,14 @@ def test_counter(qtbot):
assert counter.count == 0


# TODO: Fix gc on PySide6
@pytest.mark.parametrize('border', (True, False))
@pytest.mark.allow_bad_gc_pyside
def test_subplot_gc(border):
BackgroundPlotter(shape=(2, 1), update_app_icon=False, border=border)


@pytest.mark.allow_bad_gc_pyside
def test_editor(qtbot, plotting):
# test editor=False
plotter = BackgroundPlotter(editor=False, off_screen=False)
Expand Down Expand Up @@ -226,7 +235,8 @@ def test_editor(qtbot, plotting):

# add at least an actor
plotter.subplot(0, 0)
plotter.add_mesh(pyvista.Sphere())
pd = pyvista.Sphere()
actor = plotter.add_mesh(pd)
plotter.subplot(1, 0)
plotter.show_axes()

Expand Down Expand Up @@ -258,6 +268,7 @@ def test_editor(qtbot, plotting):

# hide the editor for coverage
editor.toggle()
plotter.remove_actor(actor)
plotter.close()


Expand Down Expand Up @@ -435,6 +446,7 @@ def _to_array(camera_position):
with pytest.raises(TypeError, match=match):
plotter_one.link_views_across_plotters(plotter_two, other_views=[0.0])


@pytest.mark.parametrize('show_plotter', [
True,
False,
Expand Down Expand Up @@ -583,6 +595,8 @@ def test_background_plotting_toolbar(qtbot, plotting):
plotter.close()


# TODO: _render_passes not GC'ed
@pytest.mark.allow_bad_gc_pyside
@pytest.mark.skipif(
platform.system() == 'Windows', reason='Segfaults on Windows')
def test_background_plotting_menu_bar(qtbot, plotting):
Expand Down Expand Up @@ -641,7 +655,9 @@ def test_drop_event(tmpdir, qtbot):
mesh = pyvista.Cone()
mesh.save(filename)
assert os.path.isfile(filename)
plotter = BackgroundPlotter(update_app_icon=False, show=True)
plotter = BackgroundPlotter(update_app_icon=False)
with qtbot.wait_exposed(plotter.app_window, timeout=10000):
plotter.app_window.show()
point = QPointF(0, 0)
data = QMimeData()
data.setUrls([QUrl(filename)])
Expand Down Expand Up @@ -678,7 +694,9 @@ def test_drag_event(tmpdir):


def test_gesture_event(qtbot):
plotter = BackgroundPlotter()
plotter = BackgroundPlotter(update_app_icon=False)
with qtbot.wait_exposed(plotter.app_window, timeout=10000):
plotter.app_window.show()
gestures = [QPinchGesture()]
event = QGestureEvent(gestures)
plotter.gesture_event(event)
Expand All @@ -688,10 +706,10 @@ def test_gesture_event(qtbot):
def test_background_plotting_add_callback(qtbot, monkeypatch, plotting):
class CallBack(object):
def __init__(self, sphere):
self.sphere = sphere
self.sphere = weakref.ref(sphere)

def __call__(self):
self.sphere.points *= 0.5
self.sphere().points[:] = self.sphere().points * 0.5

update_count = [0]
orig_update_app_icon = BackgroundPlotter.update_app_icon
Expand Down Expand Up @@ -766,10 +784,23 @@ def update_app_icon(slf):
assert not callback_timer.isActive() # window stops the callback


def allow_bad_gc_old_pyvista(func):
if Version(pyvista.__version__) < Version('0.37'):
return pytest.mark.allow_bad_gc(func)
else:
return func


# TODO: Need to fix this allow_bad_gc:
# - the actors are not cleaned up in the non-empty scene case
# - the q_key_press leaves a lingering vtkUnsignedCharArray referred to by
# a "managedbuffer" object
Comment on lines +794 to +797
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's open a follow-up issue for this.

@allow_bad_gc_old_pyvista
@pytest.mark.allow_bad_gc_pyside
@pytest.mark.parametrize('close_event', [
"plotter_close",
"window_close",
"q_key_press",
pytest.param("q_key_press", marks=pytest.mark.allow_bad_gc),
"menu_exit",
"del_finalizer",
])
Expand Down