Use real references to Graphics, proper garbage collection in ipython and jupyter#546
Use real references to Graphics, proper garbage collection in ipython and jupyter#546
Graphics, proper garbage collection in ipython and jupyter#546Conversation
There was a problem hiding this comment.
No more weakreferences to Graphics! 🥳
🥳 ❤️
fastplotlib/layouts/_plot_area.py
Outdated
| for ref in references: | ||
| if ip.user_ns[ref] is graphic: | ||
| # we found the graphic, remove from ipython | ||
| ip.del_var(ref) | ||
| break |
There was a problem hiding this comment.
How likely is it that the object is in user_ns? If it's only occasionally, can wrap this in:
if graphic in ip.user_ns.values():
...I guess it's possible that the graphic is present more than once, under a different name?
Alt solution that might be faster because it avoids copying the keys and looking up the values:
keys_to_delete = []
for key, ob in ip.user_ns.items():
if ob is graphic:
keys_to_delete.append(key)
for key in keys_to_delete:
ip.del_var(key)There was a problem hiding this comment.
How likely is it that the object is in
user_ns? If it's only occasionally, can wrap this in:if graphic in ip.user_ns.values(): ...
in operators don't work if any item of the iterable is a numpy array, for example:
a = np.random.rand(10)
l = [a, "bah", 123]
# will raise a ValueError
123 in lI've resorted to this, couldn't think of a more performance solution. Time taken scales with the number of items in the namespace.
if IS_IPYTHON:
# remove any references that ipython might have made
ip = get_ipython()
# check both namespaces
for namespace in [ip.user_ns, ip.user_ns_hidden]:
# find the reference
for ref, obj in namespace.items():
if graphic is obj:
# we found the reference, remove from ipython
ip.del_var(ref)
breakThere was a problem hiding this comment.
No problems with deleting items from an object being iterated over? I guess it depends on IPython internals whether that'd be a problem.
There was a problem hiding this comment.
LGTM aside from the minor suggestions that Almar had
I think this will be super useful for users to get the actual object instead of a weakref back (less confusing when interacting with objects etc.). I agree that for now we just leave weakref to the world object...we can always change later.
|
ready for another look, sorry for the delay been sick 🤧 |
|
Hope you get better soon! |
|
@clewis7 ready to go! |
closes #472, closes #161
also closes #393
No more weakreferences to
Graphics! 🥳Figured out how to properly garbage collect references proliferated by ipython in jupyter with some help from @lauraporta, thanks! 😄
The
pygfx.WorldObjectreference inGraphicis still aweakref.proxy, I thought let's keep it like that for now since we don't expect users to directly interact with the rendering engine objects much.In
PlotArea.delete_graphic()this is how proliferated references can be removed:fastplotlib/fastplotlib/layouts/_plot_area.py
Lines 642 to 651 in 675dfaa