Skip to content

Commit

Permalink
Emit event from Shapes data setter (#6134)
Browse files Browse the repository at this point in the history
# Fixes/Closes

Closes #6125 

# Description
With #6117 the emitted data in the `data.setter` of `Points` has been
made more consistent with the emitted data when adding, changing or
removing data within the layer. However, in `Shapes` only the data
itself was emitted. Furthermore,
since the `data.setter` in Shapes called the public `.add` method and
emits data itself, a `layers.data` event was emitted twice.
This PR is a slight refactor so that `data.setter` can directly call the
private `_add_shapes` method, bypassing the `layer.data` emit in `.add`.


## Type of change

- [x] Bug-fix (non-breaking change which fixes an issue)
- [x] Maintenance (changes required to run napari, tests, & CI smoothly)

# How has this been tested?

- [x] example: the test suite for my feature covers cases x, y, and z
- [x] example: all tests pass with my change
- [x] example: I check if my changes works with both PySide and PyQt
backends
      as there are small differences between the two Qt bindings.  

## Final checklist:
- [x] My PR is the minimum possible work for the desired functionality
- [x] I have added tests that prove my fix is effective or that my
feature works
- [x] If I included new strings, I have used `trans.` to make them
localizable.
For more information see our [translations
guide](https://napari.org/developers/translations.html).
  • Loading branch information
melonora committed Aug 18, 2023
1 parent 670c971 commit 0e60fd6
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 29 deletions.
21 changes: 21 additions & 0 deletions napari/layers/shapes/_tests/test_shapes.py
Expand Up @@ -2239,3 +2239,24 @@ def test_editing_4d():
viewer.layers['rois'].data = [
np.around(x) for x in viewer.layers['rois'].data
]


def test_points_data_setter_emits_event():
data = np.random.random((4, 2))
emitted_events = Mock()
layer = Shapes(data)
layer.events.data.connect(emitted_events)
layer.data = np.random.random((4, 2))
emitted_events.assert_called_once()


def test_points_add_delete_only_emit_one_event():
data = np.random.random((4, 2))
emitted_events = Mock()
layer = Shapes(data)
layer.events.data.connect(emitted_events)
layer.add(np.random.random((4, 2)))
assert emitted_events.call_count == 1
layer.selected_data = {1}
layer.remove_selected()
assert emitted_events.call_count == 2
57 changes: 28 additions & 29 deletions napari/layers/shapes/shapes.py
Expand Up @@ -673,17 +673,23 @@ def data(self, data):
self._data_view.slice_key = np.array(self._slice_indices)[
self._slice_input.not_displayed
]
self.add(
self._add_shapes(
data,
shape_type=shape_type,
edge_width=edge_widths,
edge_color=edge_color,
face_color=face_color,
z_index=z_indices,
n_new_shapes=n_new_shapes,
)

self._update_dims()
self.events.data(value=self.data)
self.events.data(
value=self.data,
action=ActionType.CHANGE.value,
data_indices=slice(None),
vertex_indices=((),),
)
self._reset_editable()

def _on_selection(self, selected: bool):
Expand Down Expand Up @@ -1979,35 +1985,17 @@ def add(
"""
data, shape_type = extract_shape_type(data, shape_type)

if edge_width is None:
edge_width = self.current_edge_width

n_new_shapes = number_of_shapes(data)

if edge_color is None:
edge_color = self._get_new_shape_color(
n_new_shapes, attribute='edge'
)
if face_color is None:
face_color = self._get_new_shape_color(
n_new_shapes, attribute='face'
)
if self._data_view is not None:
z_index = z_index or max(self._data_view._z_index, default=-1) + 1
else:
z_index = z_index or 0

if n_new_shapes > 0:
total_shapes = n_new_shapes + self.nshapes
self._feature_table.resize(total_shapes)
self.text.apply(self.features)
self._add_shapes(
data,
shape_type=shape_type,
edge_width=edge_width,
edge_color=edge_color,
face_color=face_color,
z_index=z_index,
n_new_shapes=n_new_shapes,
)
self.events.data(
value=self.data,
Expand Down Expand Up @@ -2105,7 +2093,7 @@ def _init_shapes(
edge_color=edge_color,
face_color=face_color,
z_index=z_index,
z_refresh=False,
n_new_shapes=n_shapes,
)
self._data_view._update_z_order()
self.refresh_colors()
Expand All @@ -2119,7 +2107,7 @@ def _add_shapes(
edge_color=None,
face_color=None,
z_index=None,
z_refresh=True,
n_new_shapes=0,
):
"""Add shapes to the data view.
Expand Down Expand Up @@ -2160,13 +2148,24 @@ def _add_shapes(
same length as the length of `data` and each element will be
applied to each shape otherwise the same value will be used for all
shapes.
z_refresh : bool
If set to true, the mesh elements are reindexed with the new z order.
When shape_index is provided, z_refresh will be overwritten to false,
as the z indices will not change.
When adding a batch of shapes, set to false and then call
ShapesList._update_z_order() once at the end.
n_new_shapes: int
The number of new shapes to be added to the Shapes layer.
"""
if n_new_shapes > 0:
total_shapes = n_new_shapes + self.nshapes
self._feature_table.resize(total_shapes)
if hasattr(self, "text"):
self.text.apply(self.features)

if edge_color is None:
edge_color = self._get_new_shape_color(
n_new_shapes, attribute='edge'
)
if face_color is None:
face_color = self._get_new_shape_color(
n_new_shapes, attribute='face'
)

if edge_width is None:
edge_width = self.current_edge_width
if edge_color is None:
Expand Down

0 comments on commit 0e60fd6

Please sign in to comment.