diff --git a/docs/source/api/layouts/subplot.rst b/docs/source/api/layouts/subplot.rst index 61f5da307..dc77a725a 100644 --- a/docs/source/api/layouts/subplot.rst +++ b/docs/source/api/layouts/subplot.rst @@ -55,7 +55,6 @@ Methods Subplot.clear Subplot.delete_graphic Subplot.get_rect - Subplot.get_refcounts Subplot.insert_graphic Subplot.map_screen_to_world Subplot.remove_animation diff --git a/examples/notebooks/test_gc.ipynb b/examples/notebooks/test_gc.ipynb index 57d7bb576..6a0130d8e 100644 --- a/examples/notebooks/test_gc.ipynb +++ b/examples/notebooks/test_gc.ipynb @@ -7,6 +7,7 @@ "metadata": {}, "outputs": [], "source": [ + "import weakref\n", "import fastplotlib as fpl\n", "import numpy as np\n", "import pytest" @@ -23,7 +24,7 @@ " for i in range(len(plot_objects)):\n", " with pytest.raises(ReferenceError) as failure:\n", " plot_objects[i]\n", - " pytest.fail(f\"GC failed for object: {objects[i]}\")" + " pytest.fail(f\"GC failed for object: {plot_objects[i]} of type: {plot_objects[i].__class__.__name__}\")" ] }, { @@ -49,7 +50,15 @@ "\n", "line_collection_data = [points_data[:, 1].copy() for i in range(10)]\n", "\n", - "img_data = np.random.rand(2_000, 2_000)" + "img_data = np.random.rand(1_000, 1_000)" + ] + }, + { + "cell_type": "markdown", + "id": "2a8a92e1-70bc-41b5-9ad8-b86dab6e74eb", + "metadata": {}, + "source": [ + "# Make references to each graphic" ] }, { @@ -76,42 +85,98 @@ "linear_region_sel_img = image.add_linear_region_selector(name=\"image_linear_region_sel\")" ] }, + { + "cell_type": "markdown", + "id": "d691c3c6-0d82-4aa8-90e9-165efffda369", + "metadata": {}, + "source": [ + "# Add event handlers" + ] + }, { "cell_type": "code", "execution_count": null, - "id": "bb2083c1-f6b7-417c-86b8-9980819917db", + "id": "64198fd0-edd4-4ba1-8082-a65d57b83881", "metadata": {}, "outputs": [], "source": [ "def feature_changed_handler(ev):\n", - " pass\n", - "\n", - "\n", + " pass" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "id": "4a86c37b-41ce-4b50-af43-ef61d36b7d81", + "metadata": {}, + "outputs": [], + "source": [ "objects = list()\n", + "weakrefs = list() # used to make sure the real objs are garbage collected\n", "for subplot in fig:\n", - " objects += subplot.objects\n", - "\n", + " for obj in subplot.objects:\n", + " objects.append(obj)\n", + " weakrefs.append(weakref.proxy(obj))\n", "\n", "for g in objects:\n", " for feature in g._features:\n", - " # if isinstance(g, fpl.LineCollection):?\n", - " # continue # skip collections for now\n", - " \n", - " g.add_event_handler(feature_changed_handler, feature)\n", - "\n", + " g.add_event_handler(feature_changed_handler, feature)" + ] + }, + { + "cell_type": "markdown", + "id": "ecd09bc8-f051-4ffd-93d3-63c262064bb4", + "metadata": {}, + "source": [ + "# Show figure" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "id": "11cf43c0-94fa-4e75-a85d-04a3f5c97729", + "metadata": {}, + "outputs": [], + "source": [ "fig.show()" ] }, + { + "cell_type": "markdown", + "id": "ad58698e-1a21-466d-b640-78500cfcb229", + "metadata": {}, + "source": [ + "# Clear fig and user-created objects list" + ] + }, { "cell_type": "code", "execution_count": null, - "id": "ba9fffeb-45bd-4a0c-a941-e7c7e68f2e55", + "id": "5849b8b3-8765-4e37-868f-6be0d127bdee", "metadata": {}, "outputs": [], "source": [ "fig.clear()" ] }, + { + "cell_type": "code", + "execution_count": null, + "id": "8ea2206b-2522-40c2-beba-c3a377990219", + "metadata": {}, + "outputs": [], + "source": [ + "objects.clear()" + ] + }, + { + "cell_type": "markdown", + "id": "a7686046-65b6-4eb4-832a-7ca72c7f9bad", + "metadata": {}, + "source": [ + "# test gc" + ] + }, { "cell_type": "code", "execution_count": null, @@ -119,7 +184,15 @@ "metadata": {}, "outputs": [], "source": [ - "test_references(objects)" + "test_references(weakrefs)" + ] + }, + { + "cell_type": "markdown", + "id": "4f927111-61c5-468e-8c90-b7b5338606ba", + "metadata": {}, + "source": [ + "# test for ImageWidget" ] }, { @@ -152,11 +225,11 @@ { "cell_type": "code", "execution_count": null, - "id": "38557b63-997f-433a-b744-e562e30be6ae", + "id": "7e855043-91c1-4f6c-bed3-b69cf4a87f84", "metadata": {}, "outputs": [], "source": [ - "old_graphics = iw.managed_graphics\n", + "old_graphics = [weakref.proxy(g) for g in iw.managed_graphics]\n", "\n", "new_movies = [np.random.rand(100, 200, 200) for i in range(6)]\n", "\n", @@ -176,7 +249,7 @@ { "cell_type": "code", "execution_count": null, - "id": "712bb6ea-7244-4e03-8dfa-9419daa34915", + "id": "ad3d2a24-88b3-4071-a49c-49667d5a7813", "metadata": {}, "outputs": [], "source": [] diff --git a/fastplotlib/graphics/_base.py b/fastplotlib/graphics/_base.py index cab941894..01482ae6e 100644 --- a/fastplotlib/graphics/_base.py +++ b/fastplotlib/graphics/_base.py @@ -42,7 +42,7 @@ class Graphic: - _features = {} + _features: set[str] = {} def __init_subclass__(cls, **kwargs): # set the type of the graphic in lower case like "image", "line_collection", etc. @@ -177,7 +177,7 @@ def block_events(self, value: bool): def world_object(self) -> pygfx.WorldObject: """Associated pygfx WorldObject. Always returns a proxy, real object cannot be accessed directly.""" # We use weakref to simplify garbage collection - return weakref.proxy(WORLD_OBJECTS[self._fpl_address]) + return weakref.proxy(WORLD_OBJECTS[hex(id(self))]) def _set_world_object(self, wo: pygfx.WorldObject): WORLD_OBJECTS[self._fpl_address] = wo @@ -348,24 +348,17 @@ def __repr__(self): else: return rval - def __eq__(self, other): - # This is necessary because we use Graphics as weakref proxies - if not isinstance(other, Graphic): - raise TypeError("`==` operator is only valid between two Graphics") - - if self._fpl_address == other._fpl_address: - return True - - return False - - def _fpl_cleanup(self): + def _fpl_prepare_del(self): """ Cleans up the graphic in preparation for __del__(), such as removing event handlers from plot renderer, feature event handlers, etc. Optionally implemented in subclasses """ - # remove event handlers + # signal that a deletion has been requested + self.deleted = True + + # clear event handlers self.clear_event_handlers() # clear any attached event handlers and animation functions @@ -394,13 +387,10 @@ def _fpl_cleanup(self): self.world_object._event_handlers.clear() - for n in self._features: - fea = getattr(self, f"_{n}") - fea.clear_event_handlers() - def __del__(self): - self.deleted = True - del WORLD_OBJECTS[self._fpl_address] + # remove world object if created + # world object does not exist if an exception was raised during __init__ which is why this check exists + WORLD_OBJECTS.pop(hex(id(self)), None) def rotate(self, alpha: float, axis: Literal["x", "y", "z"] = "y"): """Rotate the Graphic with respect to the world. diff --git a/fastplotlib/graphics/_collection_base.py b/fastplotlib/graphics/_collection_base.py index 2805c684d..36f83ec7a 100644 --- a/fastplotlib/graphics/_collection_base.py +++ b/fastplotlib/graphics/_collection_base.py @@ -1,12 +1,9 @@ +from contextlib import suppress from typing import Any -import weakref import numpy as np -from ._base import HexStr, Graphic - -# Dict that holds all collection graphics in one python instance -COLLECTION_GRAPHICS: dict[HexStr, Graphic] = dict() +from ._base import Graphic class CollectionProperties: @@ -193,25 +190,17 @@ def __init__(self, name: str = None, metadata: Any = None, **kwargs): super().__init__(name=name, metadata=metadata, **kwargs) # list of mem locations of the graphics - self._graphics: list[str] = list() + self._graphics: list[Graphic] = list() self._graphics_changed: bool = True - self._graphics_array: np.ndarray[Graphic] = None self._iter = None @property def graphics(self) -> np.ndarray[Graphic]: - """The Graphics within this collection. Always returns a proxy to the Graphics.""" - if self._graphics_changed: - proxies = [ - weakref.proxy(COLLECTION_GRAPHICS[addr]) for addr in self._graphics - ] - self._graphics_array = np.array(proxies) - self._graphics_array.flags["WRITEABLE"] = False - self._graphics_changed = False + """The Graphics within this collection.""" - return self._graphics_array + return np.asarray(self._graphics) def add_graphic(self, graphic: Graphic): """ @@ -231,10 +220,7 @@ def add_graphic(self, graphic: Graphic): f"you are trying to add a {graphic.__class__.__name__}." ) - addr = graphic._fpl_address - COLLECTION_GRAPHICS[addr] = graphic - - self._graphics.append(addr) + self._graphics.append(graphic) self.world_object.add(graphic.world_object) @@ -254,7 +240,7 @@ def remove_graphic(self, graphic: Graphic): """ - self._graphics.remove(graphic._fpl_address) + self._graphics.remove(graphic) self.world_object.remove(graphic.world_object) @@ -313,7 +299,7 @@ def _fpl_add_plot_area_hook(self, plot_area): for g in self: g._fpl_add_plot_area_hook(plot_area) - def _fpl_cleanup(self): + def _fpl_prepare_del(self): """ Cleans up the graphic in preparation for __del__(), such as removing event handlers from plot renderer, feature event handlers, etc. @@ -324,20 +310,21 @@ def _fpl_cleanup(self): self.world_object._event_handlers.clear() for g in self: - g._fpl_cleanup() + g._fpl_prepare_del() def __getitem__(self, key) -> CollectionIndexer: if np.issubdtype(type(key), np.integer): - addr = self._graphics[key] - return weakref.proxy(COLLECTION_GRAPHICS[addr]) + return self.graphics[key] return self._indexer(selection=self.graphics[key], features=self._features) def __del__(self): + # detach children self.world_object.clear() - for addr in self._graphics: - del COLLECTION_GRAPHICS[addr] + for g in self.graphics: + g._fpl_prepare_del() + del g super().__del__() @@ -350,9 +337,8 @@ def __iter__(self): def __next__(self) -> Graphic: index = next(self._iter) - addr = self._graphics[index] - return weakref.proxy(COLLECTION_GRAPHICS[addr]) + return self._graphics[index] def __repr__(self): rval = super().__repr__() diff --git a/fastplotlib/graphics/image.py b/fastplotlib/graphics/image.py index 5805804c7..866ed2486 100644 --- a/fastplotlib/graphics/image.py +++ b/fastplotlib/graphics/image.py @@ -1,5 +1,4 @@ from typing import * -import weakref import pygfx @@ -307,7 +306,7 @@ def add_linear_selector( size=size, center=center, axis=axis, - parent=weakref.proxy(self), + parent=self, **kwargs, ) @@ -316,7 +315,7 @@ def add_linear_selector( # place selector above this graphic selector.offset = selector.offset + (0.0, 0.0, self.offset[-1] + 1) - return weakref.proxy(selector) + return selector def add_linear_region_selector( self, @@ -384,7 +383,7 @@ def add_linear_region_selector( center=center, axis=axis, fill_color=fill_color, - parent=weakref.proxy(self), + parent=self, **kwargs, ) @@ -393,4 +392,4 @@ def add_linear_region_selector( # place above this graphic selector.offset = selector.offset + (0.0, 0.0, self.offset[-1] + 1) - return weakref.proxy(selector) + return selector diff --git a/fastplotlib/graphics/line.py b/fastplotlib/graphics/line.py index d0a8cc336..f352dfde5 100644 --- a/fastplotlib/graphics/line.py +++ b/fastplotlib/graphics/line.py @@ -1,5 +1,4 @@ from typing import * -import weakref import numpy as np @@ -147,7 +146,7 @@ def add_linear_selector( size=size, center=center, axis=axis, - parent=weakref.proxy(self), + parent=self, **kwargs, ) @@ -156,7 +155,7 @@ def add_linear_selector( # place selector above this graphic selector.offset = selector.offset + (0.0, 0.0, self.offset[-1] + 1) - return weakref.proxy(selector) + return selector def add_linear_region_selector( self, @@ -204,7 +203,7 @@ def add_linear_region_selector( size=size, center=center, axis=axis, - parent=weakref.proxy(self), + parent=self, **kwargs, ) @@ -215,7 +214,7 @@ def add_linear_region_selector( # PlotArea manages this for garbage collection etc. just like all other Graphics # so we should only work with a proxy on the user-end - return weakref.proxy(selector) + return selector # TODO: this method is a bit of a mess, can refactor later def _get_linear_selector_init_args( diff --git a/fastplotlib/graphics/line_collection.py b/fastplotlib/graphics/line_collection.py index 01faa9164..666d441e4 100644 --- a/fastplotlib/graphics/line_collection.py +++ b/fastplotlib/graphics/line_collection.py @@ -1,5 +1,4 @@ from typing import * -import weakref import numpy as np @@ -378,7 +377,7 @@ def add_linear_selector( size=size, center=center, axis=axis, - parent=weakref.proxy(self), + parent=self, **kwargs, ) @@ -387,7 +386,7 @@ def add_linear_selector( # place selector above this graphic selector.offset = selector.offset + (0.0, 0.0, self.offset[-1] + 1) - return weakref.proxy(selector) + return selector def add_linear_region_selector( self, @@ -435,7 +434,7 @@ def add_linear_region_selector( size=size, center=center, axis=axis, - parent=weakref.proxy(self), + parent=self, **kwargs, ) @@ -446,7 +445,7 @@ def add_linear_region_selector( # PlotArea manages this for garbage collection etc. just like all other Graphics # so we should only work with a proxy on the user-end - return weakref.proxy(selector) + return selector def _get_linear_selector_init_args(self, axis, padding): # use bbox to get size and center diff --git a/fastplotlib/graphics/selectors/_base_selector.py b/fastplotlib/graphics/selectors/_base_selector.py index 0fc48058d..ab7bda049 100644 --- a/fastplotlib/graphics/selectors/_base_selector.py +++ b/fastplotlib/graphics/selectors/_base_selector.py @@ -364,10 +364,10 @@ def _key_up(self, ev): self._move_info = None - def _fpl_cleanup(self): + def _fpl_prepare_del(self): if hasattr(self, "_pfunc_fill"): self._plot_area.renderer.remove_event_handler( self._pfunc_fill, "pointer_down" ) del self._pfunc_fill - super()._fpl_cleanup() + super()._fpl_prepare_del() diff --git a/fastplotlib/graphics/selectors/_linear.py b/fastplotlib/graphics/selectors/_linear.py index 22ba96a28..e7b0032f9 100644 --- a/fastplotlib/graphics/selectors/_linear.py +++ b/fastplotlib/graphics/selectors/_linear.py @@ -390,8 +390,8 @@ def _move_graphic(self, delta: np.ndarray): else: self.selection = self.selection + delta[1] - def _fpl_cleanup(self): + def _fpl_prepare_del(self): for widget in self._handled_widgets: widget.unobserve(self._ipywidget_callback, "value") - super()._fpl_cleanup() + super()._fpl_prepare_del() diff --git a/fastplotlib/layouts/_graphic_methods_mixin.py b/fastplotlib/layouts/_graphic_methods_mixin.py index 387549ade..ea553f119 100644 --- a/fastplotlib/layouts/_graphic_methods_mixin.py +++ b/fastplotlib/layouts/_graphic_methods_mixin.py @@ -3,7 +3,6 @@ from typing import * import numpy -import weakref from ..graphics import * from ..graphics._base import Graphic @@ -25,8 +24,7 @@ def _create_graphic(self, graphic_class, *args, **kwargs) -> Graphic: graphic = graphic_class(*args, **kwargs) self.add_graphic(graphic, center=center) - # only return a proxy to the real graphic - return weakref.proxy(graphic) + return graphic def add_image( self, diff --git a/fastplotlib/layouts/_plot_area.py b/fastplotlib/layouts/_plot_area.py index d8e0adebc..36d9c4019 100644 --- a/fastplotlib/layouts/_plot_area.py +++ b/fastplotlib/layouts/_plot_area.py @@ -1,7 +1,5 @@ from inspect import getfullargspec -from sys import getrefcount -from typing import TypeAlias, Literal, Union -import weakref +from typing import Literal, Union from warnings import warn import numpy as np @@ -12,85 +10,19 @@ from ._utils import create_controller from ..graphics._base import Graphic -from ..graphics._collection_base import GraphicCollection from ..graphics.selectors._base_selector import BaseSelector from ..legends import Legend -HexStr: TypeAlias = str - - -class References: - """ - This is the only place where the real graphic objects are stored. Everywhere else gets a proxy. - """ - - _graphics: dict[HexStr, Graphic] = dict() - _selectors: dict[HexStr, BaseSelector] = dict() - _legends: dict[HexStr, Legend] = dict() - - def add(self, graphic: Graphic | BaseSelector | Legend): - """Adds the real graphic to the dict""" - addr = graphic._fpl_address - - if isinstance(graphic, BaseSelector): - self._selectors[addr] = graphic - - elif isinstance(graphic, Legend): - self._legends[addr] = graphic - - elif isinstance(graphic, Graphic): - self._graphics[addr] = graphic - - else: - raise TypeError("Can only add Graphic, Selector or Legend types") - - def remove(self, address): - if address in self._graphics.keys(): - del self._graphics[address] - elif address in self._selectors.keys(): - del self._selectors[address] - elif address in self._legends.keys(): - del self._legends[address] - else: - raise KeyError(f"graphic with address not found: {address}") - - def get_proxies(self, refs: list[HexStr]) -> tuple[weakref.proxy]: - proxies = list() - for key in refs: - if key in self._graphics.keys(): - proxies.append(weakref.proxy(self._graphics[key])) - - elif key in self._selectors.keys(): - proxies.append(weakref.proxy(self._selectors[key])) - - elif key in self._legends.keys(): - proxies.append(weakref.proxy(self._legends[key])) - - else: - raise KeyError(f"graphic object with address not found: {key}") - - return tuple(proxies) - - def get_refcounts(self) -> dict[HexStr:int]: - counts = dict() - - for item in (self._graphics, self._selectors, self._legends): - for k in item.keys(): - counts[(k, item[k].name, item[k].__class__.__name__)] = getrefcount( - item[k] - ) - - return counts - - -REFERENCES = References() +try: + ip = get_ipython() +except NameError: + IS_IPYTHON = False +else: + IS_IPYTHON = True class PlotArea: - def get_refcounts(self): - return REFERENCES.get_refcounts() - def __init__( self, parent: Union["PlotArea", "Figure"], @@ -158,14 +90,14 @@ def __init__( # list of hex id strings for all graphics managed by this PlotArea # the real Graphic instances are managed by REFERENCES - self._graphics: list[HexStr] = list() + self._graphics: list[Graphic] = list() # selectors are in their own list so they can be excluded from scene bbox calculations # managed similar to GRAPHICS for garbage collection etc. - self._selectors: list[HexStr] = list() + self._selectors: list[BaseSelector] = list() # legends, managed just like other graphics as explained above - self._legends: list[HexStr] = list() + self._legends: list[Legend] = list() self._name = name @@ -276,18 +208,18 @@ def controller(self, new_controller: str | pygfx.Controller): @property def graphics(self) -> tuple[Graphic, ...]: - """Graphics in the plot area. Always returns a proxy to the Graphic instances.""" - return REFERENCES.get_proxies(self._graphics) + """Graphics in the plot area.""" + return tuple(self._graphics) @property def selectors(self) -> tuple[BaseSelector, ...]: - """Selectors in the plot area. Always returns a proxy to the Graphic instances.""" - return REFERENCES.get_proxies(self._selectors) + """Selectors in the plot area.""" + return tuple(self._selectors) @property def legends(self) -> tuple[Legend, ...]: """Legends in the plot area.""" - return REFERENCES.get_proxies(self._legends) + return tuple(self._legends) @property def objects(self) -> tuple[Graphic | BaseSelector | Legend, ...]: @@ -531,32 +463,25 @@ def _add_or_insert_graphic( if graphic.name is not None: # skip for those that have no name self._check_graphic_name_exists(graphic.name) - addr = graphic._fpl_address - if isinstance(graphic, BaseSelector): - addr_list = self._selectors + obj_list = self._selectors elif isinstance(graphic, Legend): - addr_list = self._legends + obj_list = self._legends elif isinstance(graphic, Graphic): - addr_list = self._graphics + obj_list = self._graphics else: raise TypeError("graphic must be of type Graphic | BaseSelector | Legend") if action == "insert": - addr_list.insert(index, addr) + obj_list.insert(index, graphic) elif action == "add": - addr_list.append(addr) + obj_list.append(graphic) else: raise ValueError("valid actions are 'insert' | 'add'") - REFERENCES.add(graphic) - - # now that it's in the dict, just use the weakref - graphic = weakref.proxy(graphic) - # add world object to scene self.scene.add(graphic.world_object) @@ -696,29 +621,36 @@ def delete_graphic(self, graphic: Graphic): The graphic to delete """ - # TODO: proper gc of selectors, RAM is freed for regular graphics but not selectors - # TODO: references to selectors must be lingering somewhere - # TODO: update March 2024, I think selectors are gc properly, should check - # get memory address - address = graphic._fpl_address - if graphic not in self: raise KeyError(f"Graphic not found in plot area: {graphic}") - # check which type it is - for l in [self._graphics, self._selectors, self._legends]: - if address in l: - l.remove(address) - break + if isinstance(graphic, BaseSelector): + self._selectors.remove(graphic) + elif isinstance(graphic, Legend): + self._legends.remove(graphic) + + elif isinstance(graphic, Graphic): + self._graphics.remove(graphic) # remove from scene if necessary if graphic.world_object in self.scene.children: self.scene.remove(graphic.world_object) # cleanup - graphic._fpl_cleanup() - - REFERENCES.remove(address) + graphic._fpl_prepare_del() + + 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) + break def clear(self): """ diff --git a/fastplotlib/widgets/histogram_lut.py b/fastplotlib/widgets/histogram_lut.py index 02c21aa38..872cf4319 100644 --- a/fastplotlib/widgets/histogram_lut.py +++ b/fastplotlib/widgets/histogram_lut.py @@ -311,8 +311,8 @@ def disconnect_image_graphic(self): del self._image_graphic # self._image_graphic = None - def _fpl_cleanup(self): - self._linear_region_selector._fpl_cleanup() - self._histogram_line._fpl_cleanup() + def _fpl_prepare_del(self): + self._linear_region_selector._fpl_prepare_del() + self._histogram_line._fpl_prepare_del() del self._histogram_line del self._linear_region_selector diff --git a/scripts/generate_add_graphic_methods.py b/scripts/generate_add_graphic_methods.py index 3f45d9007..d69185521 100644 --- a/scripts/generate_add_graphic_methods.py +++ b/scripts/generate_add_graphic_methods.py @@ -29,8 +29,7 @@ def generate_add_graphics_methods(): f.write("# This is an auto-generated file and should not be modified directly\n\n") f.write("from typing import *\n\n") - f.write("import numpy\n") - f.write("import weakref\n\n") + f.write("import numpy\n\n") f.write("from ..graphics import *\n") f.write("from ..graphics._base import Graphic\n\n") @@ -49,8 +48,7 @@ def generate_add_graphics_methods(): f.write(" self._check_graphic_name_exists(kwargs['name'])\n\n") f.write(" graphic = graphic_class(*args, **kwargs)\n") f.write(" self.add_graphic(graphic, center=center)\n\n") - f.write(" # only return a proxy to the real graphic\n") - f.write(" return weakref.proxy(graphic)\n\n") + f.write(" return graphic\n\n") for m in modules: class_name = m