-
Notifications
You must be signed in to change notification settings - Fork 439
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
Fix Plotter and children for proper garbage collection #3037
Changes from all commits
0b64806
246351f
1f8f157
a373b46
f506690
dd476af
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -201,6 +201,8 @@ def __init__( | |
"""Initialize base plotter.""" | ||
super().__init__(**kwargs) # cooperative multiple inheritance | ||
log.debug('BasePlotter init start') | ||
self._initialized = False | ||
|
||
self._theme = pyvista.themes.DefaultTheme() | ||
if theme is None: | ||
# copy global theme to ensure local plot theme is fixed | ||
|
@@ -281,6 +283,8 @@ def __init__( | |
if self.theme.antialiasing: | ||
self.enable_anti_aliasing() | ||
|
||
self._initialized = True | ||
|
||
@property | ||
def theme(self): | ||
"""Return or set the theme used for this plotter. | ||
|
@@ -4720,13 +4724,12 @@ def _datasets(self): | |
|
||
def __del__(self): | ||
"""Delete the plotter.""" | ||
# We have to check here if it has the closed attribute as it | ||
# may not exist should the plotter have failed to initialize. | ||
if hasattr(self, '_closed'): | ||
# We have to check here if the plotter was only partially initialized | ||
if self._initialized: | ||
if not self._closed: | ||
self.close() | ||
self.deep_clean() | ||
if hasattr(self, 'renderers'): | ||
if self._initialized: | ||
del self.renderers | ||
Comment on lines
4725
to
4733
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We were getting errors inside Since errors can appear in many places during Again I'm not 100% sure this is the right call, or the right implementation (e.g. set |
||
|
||
def add_background_image(self, image_path, scale=1, auto_resize=True, as_global=True): | ||
|
@@ -5163,6 +5166,8 @@ def __init__( | |
lighting=lighting, | ||
theme=theme, | ||
) | ||
# reset partial initialization flag | ||
self._initialized = False | ||
|
||
log.debug('Plotter init start') | ||
|
||
|
@@ -5256,6 +5261,9 @@ def on_timer(iren, event_id): | |
if self.enable_depth_peeling(): | ||
for renderer in self.renderers: | ||
renderer.enable_depth_peeling() | ||
|
||
# some cleanup only necessary for fully initialized plotters | ||
self._initialized = True | ||
log.debug('Plotter init stop') | ||
|
||
def show( | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is how this all started. I naively added a
__del__
when fixing #2991, only to realise that this is never called. Had to skip this for coverage then, since you can't call__del__
on an object that refuses to die.