Skip to content

Commit

Permalink
Bug fixes to multiple issues with linked layers (napari#6623)
Browse files Browse the repository at this point in the history
# References and relevant issues
Closes napari#6619

# Description
While testing napari#6622 I noted a
number of issues with linked layers:
1) a removed layer remains linked (this was also reported in
napari#6619 )
2) if two points layers are linked and you try to add a point to either
one you get:
`ValueError: The truth value of a DataFrame is ambiguous. Use a.empty,
a.bool(), a.item(), a.any() or a.all().`
3) if you have a Labels layer linked to points, shapes, image and try to
paint, you get:
`ValueError: 'paint' is not a valid Mode`
4) if you have a Shapes layer linked to any other layer and try to draw
a shape, you get:
`ValueError: 'add_rectangle' is not a valid Mode`

This is a PR to fix them.
- unlink layers before removing from layer list
- add equality operator for pandas DF, so that `pick_equality_operator`
works for Points
- add things to the `exclude` list that are problematic because:
	- they either are data related (e.g. features/properties)
- depend on number of <data elements> (e.g. edge_width, face_color,
edge_color)
	- are not compatible between layers (e.g. mode)

I've added tests for:
- unlinking of removed layers (this fails on main)
- the pandas equality operator
- linked points layers (this fails on main)
- linked layer modes (this fails on main)

---------

Co-authored-by: Grzegorz Bokota <[email protected]>
  • Loading branch information
psobolewskiPhD and Czaki authored Jan 27, 2024
1 parent a7da21e commit dab2a86
Show file tree
Hide file tree
Showing 6 changed files with 72 additions and 2 deletions.
19 changes: 19 additions & 0 deletions napari/components/_tests/test_layers_list.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

from napari.components import LayerList
from napari.layers import Image
from napari.layers.utils._link_layers import get_linked_layers


def test_empty_layers_list():
Expand Down Expand Up @@ -152,6 +153,24 @@ def test_remove_selected():
assert len(layers) == 0


def test_remove_linked_layer():
"""Test removing a layer that is linked to other layers"""
layers = LayerList()
layer_a = Image(np.empty((10, 10)))
layer_b = Image(np.empty((15, 15)))
layer_c = Image(np.empty((15, 15)))
layers.extend([layer_a, layer_b, layer_c])

# link layer_c with layer_b
layers.link_layers([layer_c, layer_b])
assert len(get_linked_layers(layer_c)) == 1
assert len(get_linked_layers(layer_b)) == 1

layers.selection.add(layer_b)
layers.remove_selected()
assert len(get_linked_layers(layer_c)) == 0


@pytest.mark.filterwarnings('ignore::FutureWarning')
def test_move_selected():
"""
Expand Down
7 changes: 7 additions & 0 deletions napari/components/layerlist.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,13 @@ def insert(self, index: int, value: Layer):
new_layer.events._extent_augmented.connect(self._clean_cache)
super().insert(index, new_layer)

def remove_selected(self):
"""Remove selected layers from LayerList, but first unlink them."""
if not self.selection:
return
self.unlink_layers(self.selection)
super().remove_selected()

def toggle_selected_visibility(self):
"""Toggle visibility of selected layers"""
for layer in self.selection:
Expand Down
17 changes: 15 additions & 2 deletions napari/layers/utils/_link_layers.py
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,20 @@ def layers_linked(layers: Iterable[Layer], attributes: Iterable[str] = ()):
def _get_common_evented_attributes(
layers: Iterable[Layer],
exclude: abc.Set[str] = frozenset(
('thumbnail', 'status', 'name', 'data', 'extent', 'loaded')
(
'thumbnail',
'status',
'name',
'mode',
'data',
'features',
'properties',
'edge_width',
'edge_color',
'face_color',
'extent',
'loaded',
)
),
with_private=False,
) -> set[str]:
Expand All @@ -207,7 +220,7 @@ def _get_common_evented_attributes(
A set of layers to evaluate for attribute linking.
exclude : set, optional
Layer attributes that make no sense to link, or may error on changing.
{'thumbnail', 'status', 'name', 'data'}
{'thumbnail', 'status', 'name', 'mode', 'data', 'features', 'properties', 'extent', 'loaded'}
with_private : bool, optional
include private attributes
Expand Down
21 changes: 21 additions & 0 deletions napari/layers/utils/_tests/test_link_layers.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,27 @@ def test_link_invalid_param():
assert "Cannot link attributes that are not shared by all layers" in str(e)


def test_adding_points_to_linked_layer():
"""Test that points can be added to a Points layer that is linked"""
l1 = layers.Points(None)
l2 = layers.Points(None)
link_layers([l1, l2])

l2.add([20, 20])
assert len(l2.data)


def test_linking_layers_with_different_modes():
"""Test that layers with different modes can be linked"""
l1 = layers.Image(np.empty((10, 10)))
l2 = layers.Labels(np.empty((10, 10), dtype=np.uint8))
link_layers([l1, l2])

l2.mode = 'paint'
assert l1.mode == 'pan_zoom'
assert l2.mode == 'paint'


def test_double_linking_noop():
"""Test that linking already linked layers is a noop."""
l1 = layers.Points(None)
Expand Down
5 changes: 5 additions & 0 deletions napari/utils/_tests/test_misc.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from napari.utils.misc import (
StringEnum,
_is_array_type,
_pandas_dataframe_equal,
_quiet_array_equal,
abspath_or_url,
ensure_iterable,
Expand Down Expand Up @@ -186,6 +187,7 @@ def test_equality_operator():

import dask.array as da
import numpy as np
import pandas as pd
import xarray as xr
import zarr

Expand All @@ -200,6 +202,9 @@ class MyNPArray(np.ndarray):
pick_equality_operator(xr.DataArray(np.ones((1, 1))))
== _quiet_array_equal
)
assert pick_equality_operator(
pd.DataFrame({'A': [1]}) == _pandas_dataframe_equal
)


@pytest.mark.skipif(
Expand Down
5 changes: 5 additions & 0 deletions napari/utils/misc.py
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,10 @@ def _quiet_array_equal(*a, **k):
return np.array_equal(*a, **k)


def _pandas_dataframe_equal(df1, df2):
return df1.equals(df2)


def _arraylike_short_names(obj) -> Iterator[str]:
"""Yield all the short names of an array-like or its class."""
type_ = type(obj) if not inspect.isclass(obj) else obj
Expand Down Expand Up @@ -525,6 +529,7 @@ def pick_equality_operator(obj) -> Callable[[Any, Any], bool]:
'dask.Delayed': operator.is_, # dask.delayed.Delayed
'zarr.Array': operator.is_, # zarr.core.Array
'xarray.DataArray': _quiet_array_equal, # xarray.core.dataarray.DataArray
'pandas.DataFrame': _pandas_dataframe_equal, # pandas.DataFrame.equals
}

for name in _arraylike_short_names(obj):
Expand Down

0 comments on commit dab2a86

Please sign in to comment.