Do not add duplicate layers (napari#4544)

* do not add duplicate layers

* fix broken test

* disallow duplicate layers

* use zip_longest

* address issues, add tests

* cast to tuple

* use method

* oops

* add trans and do not consume generators willy-nilly

* fix test

napari/components/_tests/test_layers_list.py

@@ -128,8 +128,9 @@ def test_clearing_layerlist():
     """Test clearing layer list."""
     layers = LayerList()
     layer = Image(np.random.random((10, 10)))
+    layer2 = Image(np.random.random((10, 10)))
     layers.append(layer)
-    layers.append(layer)
+    layers.append(layer2)
     assert len(layers) == 2
 
     layers.clear()
@@ -529,3 +530,36 @@ def test_name_uniqueness():
     layers.append(Image(np.random.random((10, 15)), name="Image"))
     layers.append(Image(np.random.random((10, 15)), name="Image"))
     assert [x.name for x in layers] == ['Image [1]', 'Image', 'Image [2]']
+
+
+def test_readd_layers():
+    layers = LayerList()
+    imgs = []
+    for i in range(5):
+        img = Image(np.random.rand(10, 10, 10))
+        layers.append(img)
+        imgs.append(img)
+
+    assert layers == imgs
+
+    with pytest.raises(ValueError):
+        layers.append(imgs[1])
+    assert layers == imgs
+
+    layers[1] = layers[1]
+    assert layers == imgs
+
+    with pytest.raises(ValueError):
+        layers[1] = layers[2]
+    assert layers == imgs
+
+    layers[:3] = layers[:3]
+    assert layers == imgs
+
+    # invert a section
+    layers[:3] = layers[2::-1]
+    assert set(layers) == set(imgs)
+
+    with pytest.raises(ValueError):
+        layers[:3] = layers[:]
+    assert set(layers) == set(imgs)

napari/components/layerlist.py

@@ -126,8 +126,31 @@ def _update_name(self, event):
         layer = event.source
         layer.name = self._coerce_name(layer.name, layer)
 
+    def _ensure_unique(self, values, allow=()):
+        bad = set(self._list) - set(allow)
+        values = tuple(values) if isinstance(values, Iterable) else (values,)
+        for v in values:
+            if v in bad:
+                raise ValueError(
+                    trans._(
+                        "Layer '{v}' is already present in layer list",
+                        deferred=True,
+                        v=v,
+                    )
+                )
+        return values
+
+    def __setitem__(self, key, value):
+        old = self._list[key]
+        if isinstance(key, slice):
+            value = self._ensure_unique(value, old)
+        elif isinstance(key, int):
+            (value,) = self._ensure_unique((value,), (old,))
+        super().__setitem__(key, value)
+
     def insert(self, index: int, value: Layer):
         """Insert ``value`` before index."""
+        (value,) = self._ensure_unique((value,))
         new_layer = self._type_check(value)
         new_layer.name = self._coerce_name(new_layer.name)
         self._clean_cache()

napari/utils/_tests/test_proxies.py

@@ -105,9 +105,10 @@ def test_receive_return_proxy_object():
         assert isinstance(layer, PublicOnlyProxy)
         # remove and add it back, should be fine
         add_layer = getattr(pv, 'add_layer')
+        viewer.layers.pop()
 
     add_layer(layer)
-    assert len(viewer.layers) == 2
+    assert len(viewer.layers) == 1
 
 
 def test_viewer_method():

napari/utils/context/_tests/test_layerlist_context.py

@@ -1,31 +1,42 @@
+import numpy as np
+
+from napari.components.layerlist import LayerList
+from napari.layers import Image, Points
 from napari.utils.context import LayerListContextKeys
 
 
-def test_layerlist_context(layer_list, points_layer):
+def test_layerlist_context():
     assert 'num_selected_layers' in LayerListContextKeys.__members__
 
     ctx = {}
     LLCK = LayerListContextKeys(ctx)
     assert LLCK.num_selected_layers == 0
     assert ctx['num_selected_layers'] == 0
 
+    layer_list = LayerList()
+    points_layer = Points()
+
     layer_list.selection.events.changed.connect(LLCK.update)
     layer_list.append(points_layer)
     assert LLCK.num_selected_layers == 1
     assert ctx['num_selected_layers'] == 1
 
 
-def test_all_selected_layers_same_type(layer_list, points_layer, image_layer):
+def test_all_selected_layers_same_type():
     assert 'all_selected_layers_same_type' in LayerListContextKeys.__members__
 
     ctx = {}
     LLCK = LayerListContextKeys(ctx)
     assert LLCK.all_selected_layers_same_type is False
     assert ctx['all_selected_layers_same_type'] is False
 
+    layer_list = LayerList()
+    points_layer1 = Points()
+    points_layer2 = Points()
+    image_layer = Image(np.zeros((10, 10)))
     layer_list.selection.events.changed.connect(LLCK.update)
-    layer_list.append(points_layer)
-    layer_list.append(points_layer)
+    layer_list.append(points_layer1)
+    layer_list.append(points_layer2)
     layer_list.append(image_layer)
 
     layer_list.selection = layer_list[:2]  # two points layers selected

napari/utils/events/containers/_evented_list.py

@@ -108,9 +108,7 @@ def __init__(
     # def remove(self, value: T): ...
 
     def __setitem__(self, key, value):
-        old = self._list[key]
-        if value is old:  # https://github.com/napari/napari/pull/2120
-            return
+        old = self._list[key]  # https://github.com/napari/napari/pull/2120
         if isinstance(key, slice):
             if not isinstance(value, Iterable):
                 raise TypeError(
@@ -119,7 +117,11 @@ def __setitem__(self, key, value):
                         deferred=True,
                     )
                 )
-
+            value = list(
+                value
+            )  # make sure we don't empty generators and reuse them
+            if value == old:
+                return
             [self._type_check(v) for v in value]  # before we mutate the list
             if key.step is not None:  # extended slices are more restricted
                 indices = list(range(*key.indices(len(self))))
@@ -140,6 +142,8 @@ def __setitem__(self, key, value):
                 for i, v in enumerate(value):
                     self.insert(start + i, v)
         else:
+            if value is old:
+                return
             super().__setitem__(key, value)
             self.events.changed(index=key, old_value=old, value=value)