PolygonalCircuit: add normalized(), fix eq operator; Surface.find_label_border_polygonal_chains: include vertices with single neighbour

.github/workflows/python.yml

@@ -38,6 +38,7 @@ jobs:
         - 3.5
         - 3.6
         - 3.7
+        - 3.8
       fail-fast: false
     steps:
     - uses: actions/checkout@v1
@@ -74,6 +75,7 @@ jobs:
         - 3.5
         - 3.6
         - 3.7
+        - 3.8
       fail-fast: false
     defaults:
       run:

CHANGELOG.md

@@ -5,7 +5,15 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
+### Added
+- method `PolygonalCircuit.normalized()`
+
 ### Fixed
+- `Surface.find_label_border_polygonal_chains`:
+  always include vertices along border with single neighbour
+  (previously indeterministic behaviour)
+- `PolygonalCircuit`: fix equals operator for circuits
+  with different but equivalent vertex orders
 - type hints
 
 ## [1.1.1] - 2020-10-18

freesurfer_surface/__init__.py

@@ -190,10 +190,18 @@ class PolygonalChain:
             vertex_indices
         )  # type: typing.Deque[int]
 
+    def normalized(self) -> "PolygonalChain":
+        vertex_indices = list(self.vertex_indices)
+        min_index = vertex_indices.index(min(vertex_indices))
+        indices_min_first = vertex_indices[min_index:] + vertex_indices[:min_index]
+        if indices_min_first[1] < indices_min_first[-1]:
+            return PolygonalChain(indices_min_first)
+        return PolygonalChain(indices_min_first[0:1] + indices_min_first[-1:0:-1])
+
     def __eq__(self, other: object) -> bool:
-        return (
+        return isinstance(other, PolygonalChain) and (
-            isinstance(other, PolygonalChain)
+            self.vertex_indices == other.vertex_indices
-            and self.vertex_indices == other.vertex_indices
+            or self.normalized().vertex_indices == other.normalized().vertex_indices
         )
 
     def __repr__(self) -> str:
@@ -548,32 +556,82 @@ class Surface:
             if len(border_vertex_indices) == 2:
                 yield LineSegment(border_vertex_indices)
 
+    _VertexSubindex = typing.Tuple[int, int]
+
+    @classmethod
+    def _duplicate_border(
+        cls,
+        neighbour_indices: typing.DefaultDict[
+            _VertexSubindex, typing.Set[_VertexSubindex]
+        ],
+        previous_index: _VertexSubindex,
+        current_index: _VertexSubindex,
+        junction_counter: int,
+    ) -> None:
+        split_index = (current_index[0], junction_counter)
+        neighbour_indices[previous_index].add(split_index)
+        neighbour_indices[split_index].add(previous_index)
+        next_index, *extra_indices = filter(
+            lambda i: i != previous_index, neighbour_indices[current_index]
+        )
+        if extra_indices:
+            neighbour_indices[next_index].add(split_index)
+            neighbour_indices[split_index].add(next_index)
+            neighbour_indices[next_index].remove(current_index)
+            neighbour_indices[current_index].remove(next_index)
+            return
+        cls._duplicate_border(
+            neighbour_indices=neighbour_indices,
+            previous_index=split_index,
+            current_index=next_index,
+            junction_counter=junction_counter,
+        )
+
     def find_label_border_polygonal_chains(
         self, label: Label
     ) -> typing.Iterator[PolygonalChain]:
-        segments = set(self._find_label_border_segments(label))
+        neighbour_indices = collections.defaultdict(
-        available_chains = collections.deque(
+            set
-            PolygonalChain(segment.vertex_indices) for segment in segments
+        )  # type: typing.DefaultDict[_VertexSubindex, typing.Set[_VertexSubindex]] # type: ignore
-        )
+        for segment in self._find_label_border_segments(label):
-        # irrespective of its poor performance,
+            vertex_indices = [(i, 0) for i in segment.vertex_indices]
-        # we keep this approach since it's easy to read and fast enough
+            neighbour_indices[vertex_indices[0]].add(vertex_indices[1])
-        while available_chains:
+            neighbour_indices[vertex_indices[1]].add(vertex_indices[0])
-            chain = available_chains.pop()
+        junction_counter = 0
-            last_chains_len = None
+        found_leaf = True
-            while last_chains_len != len(available_chains):
+        while found_leaf:
-                last_chains_len = len(available_chains)
+            found_leaf = False
-                checked_chains = (
+            for leaf_index, leaf_neighbour_indices in neighbour_indices.items():
-                    collections.deque()
+                if len(leaf_neighbour_indices) == 1:
-                )  # type: typing.Deque[PolygonalChain]
+                    found_leaf = True
-                while available_chains:
+                    junction_counter += 1
-                    potential_neighbour = available_chains.pop()
+                    self._duplicate_border(
-                    try:
+                        neighbour_indices=neighbour_indices,
-                        chain.connect(potential_neighbour)
+                        previous_index=leaf_index,
-                    except PolygonalChainsNotOverlapingError:
+                        # pylint: disable=stop-iteration-return; false positive, has 1 item
-                        checked_chains.append(potential_neighbour)
+                        current_index=next(iter(leaf_neighbour_indices)),
-                available_chains = checked_chains
+                        junction_counter=junction_counter,
-            assert all((segment in segments) for segment in chain.segments())
+                    )
-            yield chain
+                    break
+        assert all(len(n) == 2 for n in neighbour_indices.values()), neighbour_indices
+        while neighbour_indices:
+            # pylint: disable=stop-iteration-return; has >= 1 item
+            chain = collections.deque([next(iter(neighbour_indices.keys()))])
+            chain.append(neighbour_indices[chain[0]].pop())
+            neighbour_indices[chain[1]].remove(chain[0])
+            while chain[0] != chain[-1]:
+                previous_index = chain[-1]
+                next_index = neighbour_indices[previous_index].pop()
+                neighbour_indices[next_index].remove(previous_index)
+                chain.append(next_index)
+                assert not neighbour_indices[previous_index], neighbour_indices[
+                    previous_index
+                ]
+                del neighbour_indices[previous_index]
+            assert not neighbour_indices[chain[0]], neighbour_indices[chain[0]]
+            del neighbour_indices[chain[0]]
+            chain.pop()
+            yield PolygonalChain(v[0] for v in chain)
 
     def _unused_vertices(self) -> typing.Set[int]:
         vertex_indices = set(range(len(self.vertices)))

setup.py

@@ -38,6 +38,7 @@ setuptools.setup(
         "Programming Language :: Python :: 3.5",
         "Programming Language :: Python :: 3.6",
         "Programming Language :: Python :: 3.7",
+        "Programming Language :: Python :: 3.8",
         "Topic :: Scientific/Engineering :: Information Analysis",
         "Topic :: Scientific/Engineering :: Medical Science Apps.",
         "Topic :: Utilities",

tests/test_polygonal_chain.py

@@ -18,6 +18,28 @@ def test_reassign_vertex_indices():
     assert tuple(chain.vertex_indices) == (1, 2, 3, 4)
 
 
+@pytest.mark.parametrize(
+    ("indices_init", "indices_normalized"),
+    (
+        ([0, 3], [0, 3]),
+        ([3, 0], [0, 3]),
+        ([0, 3, 2, 4], [0, 3, 2, 4]),
+        ([0, 4, 2, 3], [0, 3, 2, 4]),
+        ([2, 3, 0, 4], [0, 3, 2, 4]),
+        ([2, 4, 0, 3], [0, 3, 2, 4]),
+        ([3, 0, 4, 2], [0, 3, 2, 4]),
+        ([3, 2, 4, 0], [0, 3, 2, 4]),
+        ([4, 0, 3, 2], [0, 3, 2, 4]),
+        ([4, 2, 3, 0], [0, 3, 2, 4]),
+    ),
+)
+def test_normalized(indices_init, indices_normalized):
+    assert (
+        list(PolygonalChain(indices_init).normalized().vertex_indices)
+        == indices_normalized
+    )
+
+
 def test_eq():
     assert PolygonalChain((0, 1, 2)) == PolygonalChain((0, 1, 2))
     # pylint: disable=unneeded-not
@@ -27,6 +49,14 @@ def test_eq():
     assert not PolygonalChain((0, 1, 2)) == PolygonalChain((1, 2, 3, 4))
 
 
+def test_eq_normalized():
+    assert PolygonalChain((0, 1, 2)) == PolygonalChain((0, 2, 1))
+    assert PolygonalChain((1, 0, 2)) == PolygonalChain((0, 2, 1))
+    assert PolygonalChain((1, 0, 2, 4)) == PolygonalChain((0, 1, 4, 2))
+    # pylint: disable=unneeded-not
+    assert not PolygonalChain((1, 0, 2, 4)) == PolygonalChain((0, 1, 2, 4))
+
+
 def test_repr():
     assert repr(PolygonalChain([])) == "PolygonalChain(vertex_indices=())"
     assert repr(PolygonalChain((0, 2, 1))) == "PolygonalChain(vertex_indices=(0, 2, 1))"
@@ -57,10 +87,7 @@ def test_connect(vertex_indices_a, vertex_indices_b, expected_vertex_indices):
 
 
 @pytest.mark.parametrize(
-    ("vertex_indices_a", "vertex_indices_b"),
+    ("vertex_indices_a", "vertex_indices_b"), [((1, 2, 3), (2, 4))]
-    [
-        ((1, 2, 3), (2, 4)),
-    ],
 )
 def test_connect_fail(vertex_indices_a, vertex_indices_b):
     chain = PolygonalChain(vertex_indices_a)
@@ -69,11 +96,7 @@ def test_connect_fail(vertex_indices_a, vertex_indices_b):
 
 
 @pytest.mark.parametrize(
-    ("vertex_indices_a", "vertex_indices_b"),
+    ("vertex_indices_a", "vertex_indices_b"), [((1, 2, 3), ()), ((), (3, 4))]
-    [
-        ((1, 2, 3), ()),
-        ((), (3, 4)),
-    ],
 )
 def test_connect_fail_empty(vertex_indices_a, vertex_indices_b):
     chain = PolygonalChain(vertex_indices_a)

tests/test_surface.py

@@ -1,5 +1,6 @@
 import copy
 import datetime
+import unittest.mock
 
 import numpy
 import pytest
@@ -481,14 +482,39 @@ def test_find_label_border_polygonal_chains():
         lambda l: l.name == "precentral", surface.annotation.labels.values()
     )
     (border_chain,) = surface.find_label_border_polygonal_chains(precentral_label)
-    vertex_indices = list(border_chain.vertex_indices)
+    vertex_indices_normalized = list(border_chain.normalized().vertex_indices)
-    assert len(vertex_indices) == 418
+    assert len(vertex_indices_normalized) == 418
-    min_index = vertex_indices.index(min(vertex_indices))
+    assert vertex_indices_normalized[66:73] == [
-    vertex_indices_normalized = vertex_indices[min_index:] + vertex_indices[:min_index]
+        61044,
+        62119,
+        62118,
+        62107,
+        62118,
+        63255,
+        63264,
+    ]
     assert vertex_indices_normalized[:4] == [32065, 32072, 32073, 32080]
     assert vertex_indices_normalized[-4:] == [36281, 34870, 33454, 33450]
 
 
+def test_find_label_border_polygonal_chains_long_leaf():
+    surface = Surface()
+    with unittest.mock.patch.object(
+        surface,
+        "_find_label_border_segments",
+        return_value=[
+            LineSegment((0, 1)),
+            LineSegment((1, 2)),
+            LineSegment((0, 3)),
+            LineSegment((2, 3)),
+            LineSegment((2, 4)),
+            LineSegment((4, 5)),  # leaf
+        ],
+    ):
+        (border_chain,) = surface.find_label_border_polygonal_chains("dummy")
+    assert list(border_chain.normalized().vertex_indices) == [0, 1, 2, 4, 5, 4, 2, 3]
+
+
 def test__unused_vertices():
     surface = Surface()
     assert not surface._unused_vertices()