GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Graphic features refactor by kushalkolar · Pull Request #511 · fastplotlib/fastplotlib

Conversation

@kushalkolar

@kushalkolar kushalkolar commented Jun 4, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

WIP

closes #389, closes #489, closes #499, closes #456, closes #369, closes #322, closes #316, closes #309, closes #283, closes #174, closes #147, closes #126

closes #507, closes #449, closes #123, closes #214

Remaining TODOs

  • Use tiling for ImageGraphic
  • refactor GraphicCollection
  • more GraphicCollection implementation:
    • getters work
    • similar for non-sliceable graphics and those with uniform buffer for colors, ex:
      • line_stack.name = [f"line-{i}" for i in range(len(line_stack))]
  • update TextGraphic with the feature changes.
  • All tests

Summary

Graphic Features

  • Changes how our Graphic Features are implemented, makes it easier for static type checking tools to understand them, slightly changes how they behave, see consider descriptor pattern for GraphicFeature. #389 for the original inspiration of this refactor, thanks @tlambert03 !!
  • Modifes how events handlers can be added to a graphic, events refactor #456
  • Makes the feature events more uniform across features, and simpler, Cleanup GraphicFeatures events pick_info  #174
  • UniformBuffer can be used for lines and scatters, cannot be shared between graphics but maybe we can implement this later.
  • Adds new GraphicFeatures, basically every graphic property which makes sense to be evented is a GraphicFeature, see Scope of graphic features #489. These features are common among all graphics:
    • Name: str, an optional name for the graphic
    • Offset: np.ndarray, [x, y, z], offset from the origin, NOT related to the data shown in the graphic, rename position_x, position_y, position_z to offsets #499
      • example: LineStack, lines are stacked along the y-axis w.r.t. to the rendering engine. This is done by incrementing the Offset. We DO NOT add to the actual data buffer that the graphic uses!
      • Rotation: np.ndarray, quaternion
      • Visible: bool
      • Deleted: bool - indicates if a a PlotArea has requested the deletion of a graphic. This is useful because you can add event handlers for "deleted", and this event handler can perform any necessary cleanup (disconnted events, etc.) so that the RAM for the graphic (i.e. the buffers) can be cleared.
  • vmin, vmax are now their own features, no longer properties of ImageCmap. It was just cleaner this way and makes more sense because RGBA images have no cmap, but they can have vmin and vmax.
  • Ownership of things is much cleaner. Graphic feature instances no longer keep the parent graphic as an attribute. Similarly, Graphics do not keep collection_index as an attribute.

Graphic Features that are sliceable, subclasses BufferManager

  • Makes it possible to share buffers across graphics 😄 Look into sharing storage buffers  #126
    • buffers can be attached/detached, not yet tested. Graphic.attach_feature("feature_name") and Graphic.detach_feature("feature_name")
  • All sorts of indexing works, including negative indexing, 16 test cases: https://github.com/fastplotlib/fastplotlib/blob/1d6adc6413e861b4a4ddc1f22fbe4c754b584a30/tests/utils.py
  • The slice parser is much much simpler now, no more 🍝
  • Only one color parser used for constructor and setter, also much simpler, less spaghetti
  • A bunch of parameterized tests for slicing and setting these buffers
  • BufferManager.__init__ manages whether the user's passed array should be isolated or not, the default is to create an isolated buffer. Makes much more sense to manage it here rather than in each SubGraphic.__init__. need to improve how we handle buffers #147
  • Everything just gets casted to float32, Choose dtype for isolated buffer #369

Selector tools

  • Deleted Sychronizer, this is something that a user should manage on their own, tailored to their usecase.
  • The logic for getting the selected, index, indices, and selected data is much simpler.
    • All selector graphics set their offset to match the parent graphic's offset. This greatly simplifies things, no more adding/subtracting offsets to map from world-space -> data space 😄
    • The geometry of the selector tool's underlying WorldObjects are used to get the position/region under the selection. Since the offsets match those of the parent Graphic, we don't need to do anything more.
    • the "selection" feature now returns value(s) in data space, NOT INDICES. This makes more sense since for example, you would want to set or get the bounds of a LinearRegionSelector in terms of actual (data xmin, data xmax). Likewise for LinearSelector.
  • Selectors behave a bit better, example: LinearRegionSelector behaves more intuitively when at/close to the limits. It is not actually possible to "smash" the selector right up against the limits and it actually touches the limit, whereas before if the delta was a bit larger it would be stuck a few pixels away from the limit.
  • The events are simplified (see below)

API comparision

Basic features (no buffer)

# before this PR, setting and getting a feature value
>>> line_graphic.thickness = 3.5
>>> line_graphic.thickness()
3.5

# this PR, getter & setter are symmetric when possible
>>> line_graphic.thickness = 3.5
>>> line_graphic.thickness
3.5

Sliceable features (manages a buffer)

# setting and getting the "entire value"
>>> line_graphic.colors = "r"
>>> line_graphic.colors
<__repr__ prints array>

# this PR, similar but value can be directly access if desired
>>> line_graphic.colors = "r"
>>> line_graphic.colors
<__repr__ prints array>

>> line_graphic.colors.value
<returns the actual array>

Sharing buffers 😄 !

# let's say you have a line with some vertex colors
>>> line1.colors
<returns array for red>

>>> line2 = subplot.add_line(data, colors=line1.colors)  # share buffer :D !

# modify colors
>>> line2.colors[50:] = "b"

# change of colors is reflected in BOTH graphics :D 

The implementation is very simple, for example in PositionsGraphic.__init__:

class PositionsGraphic(Graphic):
  def __init__(self, data, colors, ...):
    if isinstance(data, VertexPositions):
        self._data = data
    else:
        self._data = VertexPositions(data, isolated_buffer=isolated_buffer)

As we can see, it actually shares the BufferManager (i.e. feature) instance. This is simpler and makes it so changing the buffer causes events to be emitted even if it's from another graphic. In the above example, if an event handler is added to line1, i.e. line1.add_event_handler(<callable>, "colors"), then changing the colors of line2 would also trigger the event which I assume is what you would want since line1 colors have also changed.

Events

# before this PR
line_graphic.thickness.add_event_handler(<callable>)

# this PR
line_graphic.add_event_handler(<callable>, "thickness")

# can also use decorators :D

@line_graphic.add_event_handler("thickness")
def thickness_changed_handler(ev):
  pass

# exactly the same for features that subclass BufferManager
@line_graphic.add_event_handler("colors", "data")
def line_colors_or_data_changed(ev):
  if ev.type == "colors":
    print(
      f"colors changed for graphic: <{ev.graphic}> 
      F"with slice: {ev.info['key']} and new values: {ev.info['value']}"
  )
  if ev.type == "data":
    print(
      f"data changed for graphic: <{ev.graphic}> 
      F"with slice: {ev.info['key']} and new values: {ev.info['value']}"
  )

# rendering engine events added in exactly the same way
@image_graphic.add_event_handler("click")
def image_clicked(ev):
  ev.graphic # the graphic that caused the event

The event adding and handling is managed here:

fastplotlib/fastplotlib/graphics/_base.py

Lines 173 to 278 in 1d6adc6

@property
def event_handlers(self) -> list[tuple[str, callable, ...]]:
"""
Registered event handlers. Read-only use ``add_event_handler()``
and ``remove_event_handler()`` to manage callbacks
"""
return list(self._event_handlers.items())
def add_event_handler(self, *args):
"""
Register an event handler.
Parameters
----------
callback: callable, the first argument
Event handler, must accept a single event argument
*types: list of strings
A list of event types, ex: "click", "data", "colors", "pointer_down"
For the available renderer event types, see
https://jupyter-rfb.readthedocs.io/en/stable/events.html
All feature support events, i.e. ``graphic.features`` will give a set of
all features that are evented
Can also be used as a decorator.
Example
-------
.. code-block:: py
def my_handler(event):
print(event)
graphic.add_event_handler(my_handler, "pointer_up", "pointer_down")
Decorator usage example:
.. code-block:: py
@graphic.add_event_handler("click")
def my_handler(event):
print(event)
"""
decorating = not callable(args[0])
callback = None if decorating else args[0]
types = args if decorating else args[1:]
def decorator(_callback):
_callback_injector = partial(self._handle_event, _callback) # adds graphic instance as attribute
for t in types:
# add to our record
self._event_handlers[t].add(_callback)
if t in self.features:
# fpl feature event
feature = getattr(self, f"_{t}")
feature.add_event_handler(_callback_injector)
else:
# wrap pygfx event
self.world_object._event_handlers[t].add(_callback_injector)
# keep track of the partial too
self._event_handler_wrappers[t].add((_callback, _callback_injector))
return _callback
if decorating:
return decorator
return decorator(callback)
def _handle_event(self, callback, event: pygfx.Event):
"""Wrap pygfx event to add graphic to pick_info"""
event.graphic = self
if event.type in self.features:
# for feature events
event._target = self.world_object
with log_exception(f"Error during handling {event.type} event"):
callback(event)
def remove_event_handler(self, callback, *types):
# remove from our record first
for t in types:
for wrapper_map in self._event_handler_wrappers[t]:
# TODO: not sure if we can handle this mapping in a better way
if wrapper_map[0] == callback:
wrapper = wrapper_map[1]
self._event_handler_wrappers[t].remove(wrapper_map)
break
else:
raise KeyError(f"event type: {t} with callback: {callback} is not registered")
self._event_handlers[t].remove(callback)
# remove callback wrapper from world object if pygfx event
if t in PYGFX_EVENTS:
print("pygfx event")
print(wrapper)
self.world_object.remove_event_handler(wrapper, t)
else:
feature = getattr(self, f"_{t}")
feature.remove_event_handler(wrapper)

Before this PR, the event info was a mess that we probably don't need to dig into 😆 . Now every FeatureEvent inherits from pygfx.Event and has the following attributes:

attribute type description
type str name of the event, ex: "colors"
graphic Graphic graphic instance that the event is from
info dict event info dictionary (see below)
target pygfx.WorldObject pygfx rendering engine object for the graphic
time_stamp float time when the event occured, in ms

The "info" dict for all "simple" features has one key, "value", which is the user passed new value. Example if line_graphic.name is changed, then the info dict will be: {"value": "new_name_str"}

BufferManager example info dicts:

colors (vertex colors, line or scatter)

dict key value type value description
key int | slice | np.ndarray[int | bool] | tuple[slice, ...] key at which colors were indexed/sliced
value np.ndarray new color values for points that were changed, shape is [n_points_changed, RGBA]
user_value str | np.ndarray | tuple[float] | list[float] | list[str] user input value that was parsed into the RGBA array

data (points data, line or scatter)

dict key value type value description
key int | slice | np.ndarray[int | bool] | tuple[slice, ...] key at which vertex positions data were indexed/sliced
value np.ndarray | float | list[float] new data values for points that were changed, shape depends on the indices that were set

Selectors

Adding event handlers to a selector is identical to how they're added for "regular" graphics:

# one of the selectors will change the line colors when it moves
@selector.add_event_handler("selection")
def set_color_at_index(ev):
    # changes the color at the index where the slider is
    ix = ev.get_selected_index()
    g = ev.graphic.parent
    g.colors[ix] = "green"

Selector feature event structure

LinearSelector "selection"

additional event attributes:

attribute type description
get_selected_index callable returns indices under the selector

info dict:

dict key value type value description
value np.ndarray new x or y value of selection

LinearRegioonSelector

additional event attributes:

attribute type description
get_selected_indices callable returns indices under the selector
get_selected_data callable returns data under the selector

info dict:

dict key value type value description
value np.ndarray new [min, max] of selection

This is how it's implemented:

fastplotlib/fastplotlib/graphics/_features/_selection_features.py

Lines 57 to 76 in 1d6adc6

def set_value(self, selector, value: float):
# clip value between limits
value = np.clip(value, self._limits[0], self._limits[1])
# set position
if self._axis == "x":
dim = 0
elif self._axis == "y":
dim = 1
for edge in selector._edges:
edge.geometry.positions.data[:, dim] = value
edge.geometry.positions.update_range()
self._value = value
event = FeatureEvent("selection", {"value": value})
event.get_selected_index = selector.get_selected_index
self._call_event_handlers(event)

selector is the LinearSelector (in this case) or LinearRegionSelector that the feature manages, and it has the get_selected_index, or get_selected_indices and get_selected_data methods for LinearRegionSelector

Graphics

Some major refactor to graphics not mentioned above:

  • line and scatter graphics now inherit from a common PositionsGraphic that handles things that are common between lines and scatters (data, colors, etc.)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
kushalkolar added 30 commits May 17, 2024 23:33
@kushalkolar
just a start
60e7f60
@kushalkolar
pushing to continue on my desktop
6eec7d5
@kushalkolar
progress on buffer manager cleanup_key
0249125
@kushalkolar
more progress, still lots to do
9d891ee
@kushalkolar
slicing working with PointsDataFeature, negative slices too, still ma…
9635bc0
…jor WIP
@kushalkolar
comitting stuff
9be017c
@kushalkolar
_update_range() is pretty good now
0491f8c
@kushalkolar
comment
97b7303
@kushalkolar
if statement
4585be2
@kushalkolar
simply colors setting
261a5a9
@kushalkolar
type annotation
5d40af8
@kushalkolar
simpler slice parsing
697d50e
@kushalkolar
remove cleanup_key :D :D :D git status!
08fd17d
@kushalkolar
all fancy and negative indexing working :D
a45651d
@kushalkolar
start tests
9612d60
@kushalkolar
exception message
dda8396
@kushalkolar
more on tests
e22fe7c
@kushalkolar
refactor sizes, not tested yet
9b4082f
@kushalkolar
start parameterizing buffer tests
2a1869c
@kushalkolar
better buffer tests
29a0a76
@kushalkolar
more variants
054c836
@kushalkolar
add array fancy indexing to same parameterization
f50614f
@kushalkolar
parameterize tuple tests
9ed2bf6
@kushalkolar
remove repr
5e1471e
@kushalkolar
test offset and size
f59fb19
@kushalkolar
test offset and size
4d25daa
@kushalkolar
test create colors
aa1949b
@kushalkolar
also test with direct truth indices in colors
c74d7b9
@kushalkolar
points tests, works
cecb438
@kushalkolar
remove imports
0a20008
@kushalkolar
kushalkolar marked this pull request as ready for review June 11, 2024 07:55
@kushalkolar

kushalkolar commented Jun 11, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

@almarklein if you have time it would be great to get your thoughts, I've summarized what this does here since it's a massive diff:

The majority of this PR implements this: #389 (comment)

I decided to go for explict properties instead of descriptors because it shows up better for typing and docstring popups.

API comparision

Basic features (no buffer)

# before this PR, setting and getting a feature value
>>> line_graphic.thickness = 3.5
>>> line_graphic.thickness()
3.5

# this PR, getter & setter are symmetric when possible
>>> line_graphic.thickness = 3.5
>>> line_graphic.thickness
3.5

Sliceable features (manages a buffer)

# setting and getting the "entire value"
>>> line_graphic.colors = "r"
>>> line_graphic.colors
<__repr__ prints array>

# this PR, similar but value can be directly access if desired
>>> line_graphic.colors = "r"
>>> line_graphic.colors
<__repr__ prints array>

>> line_graphic.colors.value
<returns the actual array>

Sharing buffers 😄 !

# let's say you have a line with some vertex colors
>>> line1.colors
<returns array for red>

>>> line2 = subplot.add_line(data, colors=line1.colors)  # share buffer :D !

# modify colors
>>> line2.colors[50:] = "b"

# change of colors is reflected in BOTH graphics :D 

The implementation is very simple, for example in PositionsGraphic.__init__:

class PositionsGraphic(Graphic):
  def __init__(self, data, colors, ...):
    if isinstance(data, VertexPositions):
        self._data = data
    else:
        self._data = VertexPositions(data, isolated_buffer=isolated_buffer)

As we can see, it actually shares the BufferManager (i.e. feature) instance. This is simpler and makes it so changing the buffer causes events to be emitted even if it's from another graphic. In the above example, if an event handler is added to line1, i.e. line1.add_event_handler(<callable>, "colors"), then changing the colors of line2 would also trigger the event which I assume is what you would want since line1 colors have also changed.

Events

# before this PR
line_graphic.thickness.add_event_handler(<callable>)

# this PR
line_graphic.add_event_handler(<callable>, "thickness")

# can also use decorators :D

@line_graphic.add_event_handler("thickness")
def thickness_changed_handler(ev):
  pass

# exactly the same for features that subclass BufferManager
@line_graphic.add_event_handler("colors", "data")
def line_colors_or_data_changed(ev):
  if ev.type == "colors":
    print(
      f"colors changed for graphic: <{ev.graphic}> 
      F"with slice: {ev.info['key']} and new values: {ev.info['value']}"
  )
  if ev.type == "data":
    print(
      f"data changed for graphic: <{ev.graphic}> 
      F"with slice: {ev.info['key']} and new values: {ev.info['value']}"
  )

# rendering engine events added in exactly the same way
@image_graphic.add_event_handler("click")
def image_clicked(ev):
  ev.graphic # the graphic that caused the event

The event adding and handling is managed here, mostly copy-pasted from pygfx.

fastplotlib/fastplotlib/graphics/_base.py

Lines 209 to 332 in 47cecfa

def add_event_handler(self, *args):
"""
Register an event handler.
Parameters
----------
callback: callable, the first argument
Event handler, must accept a single event argument
*types: list of strings
A list of event types, ex: "click", "data", "colors", "pointer_down"
For the available renderer event types, see
https://jupyter-rfb.readthedocs.io/en/stable/events.html
All feature support events, i.e. ``graphic.features`` will give a set of
all features that are evented
Can also be used as a decorator.
Example
-------
.. code-block:: py
def my_handler(event):
print(event)
graphic.add_event_handler(my_handler, "pointer_up", "pointer_down")
Decorator usage example:
.. code-block:: py
@graphic.add_event_handler("click")
def my_handler(event):
print(event)
"""
decorating = not callable(args[0])
callback = None if decorating else args[0]
types = args if decorating else args[1:]
unsupported_events = [t for t in types if t not in self.events]
if len(unsupported_events) > 0:
raise TypeError(
f"unsupported events passed: {unsupported_events} for {self.__class__.__name__}\n"
f"`graphic.events` will return a tuple of supported events"
)
def decorator(_callback):
_callback_wrapper = partial(
self._handle_event, _callback
) # adds graphic instance as attribute and other things
for t in types:
# add to our record
self._event_handlers[t].add(_callback)
if t in self._features:
# fpl feature event
feature = getattr(self, f"_{t}")
feature.add_event_handler(_callback_wrapper)
else:
# wrap pygfx event
self.world_object._event_handlers[t].add(_callback_wrapper)
# keep track of the partial too
self._event_handler_wrappers[t].add((_callback, _callback_wrapper))
return _callback
if decorating:
return decorator
return decorator(callback)
def _handle_event(self, callback, event: pygfx.Event):
"""Wrap pygfx event to add graphic to pick_info"""
event.graphic = self
if self.block_events:
return
if event.type in self._features:
# for feature events
event._target = self.world_object
if isinstance(event, pygfx.PointerEvent):
# map from screen to world space and data space
world_xy = self._plot_area.map_screen_to_world(event)
# subtract offset to map to data
data_xy = world_xy - self.offset
# append attributes
event.x_world, event.y_world = world_xy[:2]
event.x_data, event.y_data = data_xy[:2]
with log_exception(f"Error during handling {event.type} event"):
callback(event)
def remove_event_handler(self, callback, *types):
# remove from our record first
for t in types:
for wrapper_map in self._event_handler_wrappers[t]:
# TODO: not sure if we can handle this mapping in a better way
if wrapper_map[0] == callback:
wrapper = wrapper_map[1]
self._event_handler_wrappers[t].remove(wrapper_map)
break
else:
raise KeyError(
f"event type: {t} with callback: {callback} is not registered"
)
self._event_handlers[t].remove(callback)
# remove callback wrapper from world object if pygfx event
if t in PYGFX_EVENTS:
print("pygfx event")
print(wrapper)
self.world_object.remove_event_handler(wrapper, t)
else:
feature = getattr(self, f"_{t}")
feature.remove_event_handler(wrapper)

We handle the events in a wrapper method which appends the Graphic instead that caused the event, and also adds data and world (x, y) for pointer events.

Before this PR, the event info for "graphic features" was a mess that we probably don't need to dig into 😆 . Now every FeatureEvent inherits from pygfx.Event and has the following attributes:

attribute type description
type str name of the event, ex: "colors"
graphic Graphic graphic instance that the event is from
info dict event info dictionary (see below)
target pygfx.WorldObject pygfx rendering engine object for the graphic
time_stamp float time when the event occured, in ms

The "info" dict for all "simple" features has one key: "value" - the user passed new value. Example if line_graphic.name is changed, then the info dict will be: {"value": "new_name_str"}. The info dict will have an additional "key" entry if it's from a buffer manager feature.

BufferManager example info dicts:

colors (vertex colors, line or scatter)

dict key value type value description
key int | slice | np.ndarray[int | bool] | tuple[slice, ...] key at which colors were indexed/sliced
value np.ndarray new color values for points that were changed, shape is [n_points_changed, RGBA]
user_value str | np.ndarray | tuple[float] | list[float] | list[str] user input value that was parsed into the RGBA array

data (points data, line or scatter)

dict key value type value description
key int | slice | np.ndarray[int | bool] | tuple[slice, ...] key at which vertex positions data were indexed/sliced
value np.ndarray | float | list[float] new data values for points that were changed, shape depends on the indices that were set

Selectors

Adding event handlers to a selector is identical to how they're added for "regular" graphics:

# one of the selectors will change the line colors when it moves
@selector.add_event_handler("selection")
def set_color_at_index(ev):
    # changes the color at the index where the slider is
    ix = ev.get_selected_index()
    g = ev.graphic.parent
    g.colors[ix] = "green"

Selector feature event structure

LinearSelector "selection"

additional event attributes:

attribute type description
get_selected_index callable returns indices under the selector

info dict:

dict key value type value description
value np.ndarray new x or y value of selection

LinearRegioonSelector

additional event attributes:

attribute type description
get_selected_indices callable returns indices under the selector
get_selected_data callable returns data under the selector

info dict:

dict key value type value description
value np.ndarray new [min, max] of selection

selector is the LinearSelector or LinearRegionSelector that the feature manages, and it has the get_selected_index, or get_selected_indices and get_selected_data methods for LinearRegionSelector

Graphics

Some major refactor to graphics not mentioned above:

  • line and scatter graphics now inherit from a common PositionsGraphic that handles things that are common between lines and scatters (data, colors, etc.)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

kushalkolar added 7 commits June 11, 2024 04:43
@kushalkolar
smaller hm data test for CI
3079fa6
@kushalkolar
exclude heatmap change data from tests, too large RAM usage probably
d719ad6
@kushalkolar
black
e63043c
@kushalkolar
change dtype to save ram usage for CI
06a489f
@kushalkolar
remove large square heatmap from screenshot tests
b524799
@kushalkolar
black
fc5160e
@kushalkolar
disable all but one hm test
2c6d0ca
clewis7
clewis7 requested changes Jun 11, 2024
View reviewed changes
Comment thread fastplotlib/graphics/_collection_base.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread fastplotlib/graphics/_collection_base.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread fastplotlib/graphics/_collection_base.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

@clewis7

clewis7 commented Jun 11, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Bug when adding an existing graphic to another plot

import fastplotlib as fpl
import numpy as np

data = np.random.rand(512, 512)

fig = fpl.Figure()

fig[0,0].add_image(data=data)

fig2 = fpl.Figure()

fig2[0,0].add_graphic(fig[0,0].graphics[0])

yields

TypeError                                 Traceback (most recent call last)
Cell In[3], line 3
      1 fig2 = fpl.Figure()
----> 3 fig2[0,0].add_graphic(fig[0,0].graphics[0])

File [~/repos/fastplotlib/fastplotlib/layouts/_plot_area.py:468](http://localhost:8888/home/caitlin/repos/fastplotlib/fastplotlib/layouts/_plot_area.py#line=467), in PlotArea.add_graphic(self, graphic, center)
    465     self.scene.add(graphic.world_object)
    466     return
--> 468 self._add_or_insert_graphic(graphic=graphic, center=center, action="add")
    470 if self.camera.fov == 0:
    471     # for orthographic positions stack objects along the z-axis
    472     # for perspective projections we assume the user wants full 3D control
    473     graphic.offset = (*graphic.offset[:-1], len(self))

File [~/repos/fastplotlib/fastplotlib/layouts/_plot_area.py:558](http://localhost:8888/home/caitlin/repos/fastplotlib/fastplotlib/layouts/_plot_area.py#line=557), in PlotArea._add_or_insert_graphic(self, graphic, center, action, index)
    555 REFERENCES.add(graphic)
    557 # now that it's in the dict, just use the weakref
--> 558 graphic = weakref.proxy(graphic)
    560 # add world object to scene
    561 self.scene.add(graphic.world_object)

TypeError: cannot create weak reference to 'weakref.ProxyType' object

also, even just trying to access a graphic

fig[0,0].graphics[0]
----> 1 fig[0,0].graphics[0]

File [~/repos/fastplotlib/fastplotlib/layouts/_plot_area.py:280](http://localhost:8888/home/caitlin/repos/fastplotlib/fastplotlib/layouts/_plot_area.py#line=279), in PlotArea.graphics(self)
    277 @property
    278 def graphics(self) -> tuple[Graphic, ...]:
    279     """Graphics in the plot area. Always returns a proxy to the Graphic instances."""
--> 280     return REFERENCES.get_proxies(self._graphics)

File [~/repos/fastplotlib/fastplotlib/layouts/_plot_area.py:62](http://localhost:8888/home/caitlin/repos/fastplotlib/fastplotlib/layouts/_plot_area.py#line=61), in References.get_proxies(self, refs)
     60 for key in refs:
     61     if key in self._graphics.keys():
---> 62         proxies.append(weakref.proxy(self._graphics[key]))
     64     elif key in self._selectors.keys():
     65         proxies.append(weakref.proxy(self._selectors[key]))

TypeError: cannot create weak reference to 'weakref.ProxyType' object

I think we are passing around the weakrefs everywhere and then we end up trying to weakref a weakref

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

kushalkolar added 6 commits June 11, 2024 16:27
@kushalkolar
fix gc
3fd7734
@kushalkolar
update screenshots
24b5d95
@kushalkolar
replace one more test image
25876bf
@kushalkolar
fix docs
eb83cfd
@kushalkolar
update image screenshots
8696d32
@kushalkolar
fix docs
70ca7c7
almarklein
almarklein reviewed Jun 12, 2024
View reviewed changes

@almarklein almarklein left a comment

Copy link
Copy Markdown
Collaborator

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.

@almarklein if you have time it would be great to get your thoughts, I've summarized what this does here since it's a massive diff

The description you posted sounds very good!

Sharing buffers

Hooray! Feels nice and simple.

Adding event handlers to a selector is identical to how they're added for "regular" graphics:

Nice, less stuff to learn for users.

I also glanced over the diff. Looks nice from a birds-eye view. Made a few minor comments/suggestions.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread fastplotlib/graphics/_base.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread fastplotlib/graphics/_base.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

@kushalkolar
docstrings, Graphic.events -> Graphic.supported_events
28e08b9
@kushalkolar

kushalkolar commented Jun 13, 2024

Copy link
Copy Markdown
Member Author

failing because camera stuff was recently merged in pygfx, I might as well update this PR with it 🫠

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@almarklein

almarklein commented Jun 13, 2024

Copy link
Copy Markdown
Collaborator

failing because camera stuff was recently merged in pygfx

You mean pygfx/pygfx#778? I was under the impression that that change was backwards compatible.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@kushalkolar

kushalkolar commented Jun 13, 2024

Copy link
Copy Markdown
Member Author

It's specific to how we allow changing the controller of an existing plot area: https://github.com/fastplotlib/fastplotlib/actions/runs/9492712816/job/26160279159?pr=511#step:9:895

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@clewis7 clewis7 mentioned this pull request Jun 16, 2024
Fpl demos flatironinstitute/ccn-software-fens-2024#11
Closed
5 tasks
@kushalkolar
update docs
d610690
clewis7
clewis7 approved these changes Jun 16, 2024
View reviewed changes

@clewis7 clewis7 left a comment

Copy link
Copy Markdown
Member

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.

YAYAY!!!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@kushalkolar

kushalkolar commented Jun 16, 2024

Copy link
Copy Markdown
Member Author

Everything passing on our ends locally, merging. Need to wait for pygfx to fix pygfx/pygfx#790 and for things to catch up with numpy v2 before all our CI works again.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@kushalkolar
kushalkolar merged commit 815e8b3 into main Jun 16, 2024
@kushalkolar
kushalkolar deleted the graphic-features-refactor branch June 18, 2024 23:13
@kushalkolar kushalkolar mentioned this pull request Jun 26, 2024
fix legend #544
Merged
@FlynnOConnell FlynnOConnell mentioned this pull request Oct 15, 2024
update to use latest fastplotlib 0.3.0 with store model kushalkolar/mesmerize-viz#45
Draft
8 tasks done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@almarklein almarklein almarklein left review comments
@clewis7 clewis7 clewis7 approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

probably get rid of CollectionIndexer and CollectionFeature in favor of numpy arrays for graphic collections Cleanup GraphicFeatures events pick_info need to improve how we handle buffers Look into sharing storage buffers GraphicCollection feature data is not accessible
scatter data doesn't work with int types, only with float Move Selection Feature classes to the corresponding selector module add features section for doc page for each graphic add test for cmap_values @setter Choose dtype for isolated buffer

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@kushalkolar @clewis7 @almarklein
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.