Skip to content

Commit

Permalink
Move the _is_created assignment to the top (napari#5078)
Browse files Browse the repository at this point in the history
# Description

The `_is_creating` flag is set too late in the drawing process.

If one clicks two times while `polygon` or `path` is selected, an error
is thrown. I also added a test and a fix for this scenario.

## Type of change

- [X] Bug-fix (non-breaking change which fixes an issue)
- [X] New feature (non-breaking change which adds functionality)

---------

Co-authored-by: Markus Stabrin <stabrin@MonsterMac.local>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Juan Nunez-Iglesias <jni@fastmail.com>
Co-authored-by: Peter Sobolewski <76622105+psobolewskiPhD@users.noreply.github.com>
Co-authored-by: Genevieve Buckley <30920819+GenevieveBuckley@users.noreply.github.com>
Co-authored-by: Andy Sweet <andrew.d.sweet@gmail.com>
Co-authored-by: Grzegorz Bokota <bokota+github@gmail.com>
Co-authored-by: Wouter-Michiel Vierdag <michiel.vierdag@embl.de>
Co-authored-by: Wouter-Michiel Vierdag <w-mv@hotmail.com>
  • Loading branch information
10 people authored Jul 16, 2024
1 parent 157b144 commit c51e291
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 1 deletion.
6 changes: 5 additions & 1 deletion napari/layers/shapes/_shapes_mouse_bindings.py
Original file line number Diff line number Diff line change
Expand Up @@ -311,12 +311,12 @@ def initiate_polygon_draw(
coordinates : Tuple[float, ...]
A tuple with the coordinates of the initial vertex in image data space.
"""
layer._is_creating = True
data = np.array([coordinates, coordinates])
layer.add(data, shape_type='path', gui=True)
layer.selected_data = {layer.nshapes - 1}
layer._value = (layer.nshapes - 1, 1)
layer._moving_value = copy(layer._value)
layer._is_creating = True
layer._set_highlight()


Expand Down Expand Up @@ -383,6 +383,10 @@ def add_vertex_to_path(
vertices = layer._data_view.shapes[index].data
vertices = np.concatenate((vertices, [coordinates]), axis=0)
value = layer.get_value(event.position, world=True)
# If there was no move event between two clicks value[1] is None
# and needs to be taken care of.
if value[1] is None:
value = layer._moving_value
layer._value = (value[0], value[1] + 1)
layer._moving_value = copy(layer._value)
layer._data_view.edit(index, vertices, new_type=new_type)
Expand Down
69 changes: 69 additions & 0 deletions napari/layers/shapes/_tests/test_shapes_mouse_bindings.py
Original file line number Diff line number Diff line change
Expand Up @@ -702,6 +702,75 @@ def test_after_in_add_mode_shape(mode, create_known_shapes_layer):
assert len(layer.selected_data) == 0


@pytest.mark.parametrize(
'mode',
[
'add_polygon',
'add_path',
],
)
def test_clicking_the_same_point_is_not_crashing(
mode, create_known_shapes_layer
):
layer, n_shapes, _ = create_known_shapes_layer

layer.mode = mode
position = tuple(layer.data[0][0])

for _ in range(2):
event = read_only_mouse_event(type='mouse_press', position=position)
mouse_press_callbacks(layer, event)

event = read_only_mouse_event(type='mouse_release', position=position)
mouse_release_callbacks(layer, event)

# If there was no move event between the two clicks, we expect value[1] must be None
assert layer.get_value(event.position, world=True)[1] is None


@pytest.mark.parametrize(
'mode',
[
'add_polygon',
'add_path',
],
)
def test_is_creating_is_false_on_creation(mode, create_known_shapes_layer):
layer, n_shapes, _ = create_known_shapes_layer

layer.mode = mode
position = tuple(layer.data[0][0])

def is_creating_is_True(event):
assert event.source._is_creating

def is_creating_is_False(event):
assert not event.source._is_creating

assert not layer._is_creating
layer.events.set_data.connect(is_creating_is_True)

event = read_only_mouse_event(type='mouse_press', position=position)
mouse_press_callbacks(layer, event)

assert layer._is_creating

event = read_only_mouse_event(type='mouse_release', position=position)
mouse_release_callbacks(layer, event)

assert layer._is_creating

layer.events.set_data.disconnect(is_creating_is_True)
layer.events.set_data.connect(is_creating_is_False)
end_click = read_only_mouse_event(
type='mouse_double_click',
position=position,
)
mouse_double_click_callbacks(layer, end_click)

assert not layer._is_creating


@pytest.mark.parametrize('mode', ['select', 'direct'])
def test_unselect_select_shape(mode, create_known_shapes_layer):
"""Select a shape by clicking on one in select mode."""
Expand Down

0 comments on commit c51e291

Please sign in to comment.