Fix several issues with relabel_sequential (#3740)

* Fix several issues with relabel_sequential

* pep8

* Update skimage/segmentation/tests/test_join.py

* update tests

* Update skimage/segmentation/_join.py

skimage/segmentation/_join.py

@@ -62,7 +62,7 @@ def relabel_sequential(label_field, offset=1):
     Parameters
     ----------
     label_field : numpy array of int, arbitrary shape
-        An array of labels.
+        An array of labels, which must be non-negative integers.
     offset : int, optional
         The return labels will start at `offset`, which should be
         strictly positive.
@@ -72,14 +72,16 @@ def relabel_sequential(label_field, offset=1):
     relabeled : numpy array of int, same shape as `label_field`
         The input label field with labels mapped to
         {offset, ..., number_of_labels + offset - 1}.
+        The data type will be the same as `label_field`, except when
+        offset + number_of_labels causes overflow of the current data type.
     forward_map : numpy array of int, shape ``(label_field.max() + 1,)``
         The map from the original label space to the returned label
         space. Can be used to re-apply the same mapping. See examples
-        for usage.
+        for usage. The data type will be the same as `relabeled`.
     inverse_map : 1D numpy array of int, of length offset + number of labels
         The map from the new label space to the original space. This
         can be used to reconstruct the original label field from the
-        relabeled one.
+        relabeled one. The data type will be the same as `relabeled`.
 
     Notes
     -----
@@ -114,20 +116,29 @@ def relabel_sequential(label_field, offset=1):
     >>> relab
     array([5, 5, 6, 6, 7, 9, 8])
     """
+    offset = int(offset)
+    if offset <= 0:
+        raise ValueError("Offset must be strictly positive.")
+    if np.min(label_field) < 0:
+        raise ValueError("Cannot relabel array that contains negative values.")
     m = label_field.max()
-    if not np.issubdtype(label_field.dtype, np.signedinteger):
+    if not np.issubdtype(label_field.dtype, np.integer):
         new_type = np.min_scalar_type(int(m))
         label_field = label_field.astype(new_type)
         m = m.astype(new_type)  # Ensures m is an integer
     labels = np.unique(label_field)
     labels0 = labels[labels != 0]
-    if m == len(labels0):  # nothing to do, already 1...n labels
+    required_type = np.min_scalar_type(offset + len(labels0))
+    if np.dtype(required_type).itemsize > np.dtype(label_field.dtype).itemsize:
+        label_field = label_field.astype(required_type)
+    new_labels0 = np.arange(offset, offset + len(labels0))
+    if np.all(labels0 == new_labels0):
         return label_field, labels, labels
-    forward_map = np.zeros(m + 1, int)
-    forward_map[labels0] = np.arange(offset, offset + len(labels0))
+    forward_map = np.zeros(int(m + 1), dtype=label_field.dtype)
+    forward_map[labels0] = new_labels0
     if not (labels == 0).any():
         labels = np.concatenate(([0], labels))
-    inverse_map = np.zeros(offset - 1 + len(labels), dtype=np.intp)
+    inverse_map = np.zeros(offset - 1 + len(labels), dtype=label_field.dtype)
     inverse_map[(offset - 1):] = labels
     relabeled = forward_map[label_field]
     return relabeled, forward_map, inverse_map

skimage/segmentation/tests/test_join.py

@@ -3,6 +3,7 @@
 
 from skimage._shared import testing
 from skimage._shared.testing import assert_array_equal
+import pytest
 
 
 def test_join_segmentations():
@@ -91,3 +92,55 @@ def test_relabel_sequential_dtype():
     assert_array_equal(fw, fw_ref)
     inv_ref = np.array([0, 0, 0, 0, 0, 1,  5,  8, 42, 99])
     assert_array_equal(inv, inv_ref)
+
+
+@pytest.mark.parametrize('dtype', (np.byte, np.short, np.intc, np.int_,
+                                   np.longlong, np.ubyte, np.ushort,
+                                   np.uintc, np.uint, np.ulonglong))
+@pytest.mark.parametrize('data_already_sequential', (False, True))
+def test_relabel_sequential_int_dtype_stability(data_already_sequential,
+                                                dtype):
+    if data_already_sequential:
+        ar = np.array([1, 3, 0, 2, 5, 4], dtype=dtype)
+    else:
+        ar = np.array([1, 1, 5, 5, 8, 99, 42, 0], dtype=dtype)
+    assert all(a.dtype == dtype for a in relabel_sequential(ar))
+
+
+def test_relabel_sequential_int_dtype_overflow():
+    ar = np.array([1, 3, 0, 2, 5, 4], dtype=np.uint8)
+    offset = 254
+    ar_relab, fw, inv = relabel_sequential(ar, offset=offset)
+    assert all(a.dtype == np.uint16 for a in (ar_relab, fw, inv))
+    ar_relab_ref = np.where(ar > 0, ar.astype(np.int) + offset - 1, 0)
+    assert_array_equal(ar_relab, ar_relab_ref)
+
+
+def test_relabel_sequential_negative_values():
+    ar = np.array([1, 1, 5, -5, 8, 99, 42, 0])
+    with pytest.raises(ValueError):
+        relabel_sequential(ar)
+
+
+@pytest.mark.parametrize('offset', (0, -3))
+@pytest.mark.parametrize('data_already_sequential', (False, True))
+def test_relabel_sequential_nonpositive_offset(data_already_sequential,
+                                               offset):
+    if data_already_sequential:
+        ar = np.array([1, 3, 0, 2, 5, 4])
+    else:
+        ar = np.array([1, 1, 5, 5, 8, 99, 42, 0])
+    with pytest.raises(ValueError):
+        relabel_sequential(ar, offset=offset)
+
+
+@pytest.mark.parametrize('offset', (1, 5))
+@pytest.mark.parametrize('with0', (False, True))
+def test_relabel_sequential_already_sequential(offset, with0):
+    if with0:
+        ar = np.array([1, 3, 0, 2, 5, 4])
+    else:
+        ar = np.array([1, 3, 2, 5, 4])
+    ar_relab, fw, inv = relabel_sequential(ar, offset=offset)
+    ar_relab_ref = np.where(ar > 0, ar + offset - 1, 0)
+    assert_array_equal(ar_relab, ar_relab_ref)