Repository: lace/polliwog

Issue:
Convert `Polyline.partition_by_length()` to work on a copy
This follows the pattern of most of the other mutating functions.

Discussion:

Patch:
diff --git a/polliwog/polyline/polyline.py b/polliwog/polyline/polyline.py
index 2f79230..a9fe270 100644
--- a/polliwog/polyline/polyline.py
+++ b/polliwog/polyline/polyline.py
@@ -285,15 +285,16 @@ class Polyline(object):
         else:
             return result
 
-    def partition_by_length(self, max_length, ret_indices=False):
+    def subdivided_by_length(self, max_length, ret_indices=False):
         """
-        Subdivide each line segment longer than max_length with
-        equal-length segments, such that none of the new segments
-        are longer than max_length.
-
-        ret_indices: If True, return the indices of the original vertices.
-          Otherwise return self for chaining.
+        Subdivide each line segment longer than `max_length` with
+        equal-length segments, such that none of the new segments are longer
+        than `max_length`. Returns a new Polyline.
 
+        Args:
+            max_length (float): The maximum lenth of a segment.
+            ret_indices (bool): When `True`, also returns the indices of the
+                original vertices.
         """
         import itertools
         from ..segment.segment_functions import subdivide_segment
@@ -319,45 +320,45 @@ class Polyline(object):
         ]
 
         splits_of_original_vs = np.vsplit(self.v, es_to_subdivide + 1)
-        self.v = np.concatenate(
-            list(
-                itertools.chain(
-                    *zip(
-                        splits_of_original_vs,
-                        vs_to_insert + [np.empty((0, 3), dtype=np.float64)],
+        new_polyline = Polyline(
+            v=np.concatenate(
+                list(
+                    itertools.chain(
+                        *zip(
+                            splits_of_original_vs,
+                            vs_to_insert + [np.empty((0, 3), dtype=np.float64)],
+                        )
                     )
                 )
-            )
+            ),
+            is_closed=self.is_closed,
         )
 
-        if ret_indices:
-            # In a degenerate case, `partition_segment()` may return fewer than
-            # the requested number of indices. So, recompute the actual number of
-            # segments inserted.
-            num_segments_inserted = np.zeros(old_num_e, dtype=np.int64)
-            num_segments_inserted[es_to_subdivide] = [len(vs) for vs in vs_to_insert]
-            stepwise_index_offsets = np.concatenate(
-                [
-                    # The first vertex is never moved.
-                    np.zeros(1, dtype=np.int64),
-                    # In a closed polyline, the last edge goes back to vertex
-                    # 0. Subdivisions of that segment do not affect indexing of
-                    # any of the vertices (since the original end vertex is
-                    # still at index 0).
-                    num_segments_inserted[:-1]
-                    if self.is_closed
-                    else num_segments_inserted,
-                ]
-            )
-            cumulative_index_offsets = np.sum(
-                np.tril(
-                    np.broadcast_to(stepwise_index_offsets, (old_num_v, old_num_v))
-                ),
-                axis=1,
-            )
-            return np.arange(old_num_v) + cumulative_index_offsets
-        else:
-            return self
+        if not ret_indices:
+            return new_polyline
+
+        # In a degenerate case, `partition_segment()` may return fewer than
+        # the requested number of indices. So, recompute the actual number of
+        # segments inserted.
+        num_segments_inserted = np.zeros(old_num_e, dtype=np.int64)
+        num_segments_inserted[es_to_subdivide] = [len(vs) for vs in vs_to_insert]
+        stepwise_index_offsets = np.concatenate(
+            [
+                # The first vertex is never moved.
+                np.zeros(1, dtype=np.int64),
+                # In a closed polyline, the last edge goes back to vertex
+                # 0. Subdivisions of that segment do not affect indexing of
+                # any of the vertices (since the original end vertex is
+                # still at index 0).
+                num_segments_inserted[:-1] if self.is_closed else num_segments_inserted,
+            ]
+        )
+        cumulative_index_offsets = np.sum(
+            np.tril(np.broadcast_to(stepwise_index_offsets, (old_num_v, old_num_v))),
+            axis=1,
+        )
+        indices_of_original_vertices = np.arange(old_num_v) + cumulative_index_offsets
+        return new_polyline, indices_of_original_vertices
 
     def with_segments_bisected(self, segment_indices, ret_new_indices=False):
         """


Test patch:
diff --git a/polliwog/polyline/test_polyline.py b/polliwog/polyline/test_polyline.py
index 0fa38c3..9952359 100644
--- a/polliwog/polyline/test_polyline.py
+++ b/polliwog/polyline/test_polyline.py
@@ -67,6 +67,19 @@ def test_to_dict():
     np.testing.assert_array_equal(expected_dict["edges"], actual_dict["edges"])
 
 
+def test_copy():
+    original_vs = np.array(
+        [[0.0, 0.0, 0.0], [1.0, 0.0, 0.0], [1.0, 1.0, 0.0], [1.0, 2.0, 0.0]]
+    )
+    polyline = Polyline(v=original_vs.copy(), is_closed=False)
+    copy_of_polyline = polyline.copy()
+    assert polyline is not copy_of_polyline
+    assert polyline.is_closed == copy_of_polyline.is_closed
+    np.testing.assert_array_equal(polyline.v, copy_of_polyline.v)
+    polyline.v[0] = np.array([2.0, 3.0, 4.0])
+    np.testing.assert_array_equal(copy_of_polyline.v, original_vs)
+
+
 def test_bounding_box():
     bounding_box = Polyline(
         np.array([[0.0, 0.0, 0.0], [1.0, 0.0, 0.0], [1.0, 1.0, 0.0], [1.0, 2.0, 0.0]])
@@ -233,7 +246,7 @@ def test_length_of_empty_polyline():
     assert polyline.total_length == 0
 
 
-def test_partition_by_length_noop():
+def test_subdivided_by_length_noop():
     original = Polyline(
         np.array(
             [
@@ -246,8 +259,7 @@ def test_partition_by_length_noop():
         )
     )
 
-    result = original.copy()
-    indices = result.partition_by_length(1.0, ret_indices=True)
+    result, indices = original.subdivided_by_length(1.0, ret_indices=True)
 
     expected_indices = np.array([0, 1, 2, 3, 4])
 
@@ -257,15 +269,14 @@ def test_partition_by_length_noop():
     np.testing.assert_array_equal(result.v[indices], original.v)
 
 
-def test_partition_by_length_degenerate():
+def test_subdivided_by_length_degenerate():
     """
     This covers a bug that arose from a numerical stability issue in
     measurement on EC2 / MKL.
     """
     original = Polyline(np.array([[0.0, 0.0, 0.0], [1.0, 0.0, 0.0], [1.0, 0.0, 0.0]]))
 
-    result = original.copy()
-    indices = result.partition_by_length(1.0, ret_indices=True)
+    result, indices = original.subdivided_by_length(1.0, ret_indices=True)
 
     expected_indices = np.array([0, 1, 2])
 
@@ -275,7 +286,7 @@ def test_partition_by_length_degenerate():
     np.testing.assert_array_equal(result.v[indices], original.v)
 
 
-def test_partition_by_length_divide_by_two():
+def test_subdivided_by_length_divide_by_two():
     original = Polyline(
         np.array(
             [
@@ -307,17 +318,15 @@ def test_partition_by_length_divide_by_two():
     expected_indices = np.array([0, 2, 4, 6, 8])
 
     for max_length in (0.99, 0.75, 0.5):
-        result = original.copy()
-        indices = result.partition_by_length(max_length, ret_indices=True)
+        result, indices = original.subdivided_by_length(max_length, ret_indices=True)
 
         np.testing.assert_array_almost_equal(result.v, expected.v)
         np.testing.assert_array_equal(result.e, expected.e)
         np.testing.assert_array_equal(indices, expected_indices)
 
-        result_2 = original.copy()
-        ret = result_2.partition_by_length(max_length, ret_indices=False)
-        np.testing.assert_array_almost_equal(result.v, expected.v)
-        assert ret is result_2
+        result_2 = result.subdivided_by_length(max_length, ret_indices=False)
+        np.testing.assert_array_almost_equal(result_2.v, expected.v)
+        assert result_2 is not result
     np.testing.assert_array_equal(result.v[indices], original.v)
 
 
@@ -365,8 +374,7 @@ def test_partition_length_divide_by_five():
     expected_indices = np.array([0, 5, 10, 15, 20])
 
     for max_length in (0.2, 0.24):
-        result = original.copy()
-        indices = result.partition_by_length(max_length, ret_indices=True)
+        result, indices = original.subdivided_by_length(max_length, ret_indices=True)
 
         np.testing.assert_array_almost_equal(result.v, expected.v)
         np.testing.assert_array_equal(result.e, expected.e)
@@ -374,7 +382,7 @@ def test_partition_length_divide_by_five():
     np.testing.assert_array_equal(result.v[indices], original.v)
 
 
-def test_partition_by_length_divide_some_leave_some():
+def test_subdivided_by_length_divide_some_leave_some():
     original = Polyline(
         np.array(
             [
@@ -404,8 +412,7 @@ def test_partition_by_length_divide_some_leave_some():
     expected_indices = np.array([0, 1, 2, 5, 6])
 
     for max_length in (2.0, 2.99):
-        result = original.copy()
-        indices = result.partition_by_length(max_length, ret_indices=True)
+        result, indices = original.subdivided_by_length(max_length, ret_indices=True)
 
         np.testing.assert_array_almost_equal(result.v, expected.v)
         np.testing.assert_array_equal(result.e, expected.e)
@@ -413,7 +420,7 @@ def test_partition_by_length_divide_some_leave_some():
     np.testing.assert_array_equal(result.v[indices], original.v)
 
 
-def test_partition_by_length_closed():
+def test_subdivided_by_length_closed():
     original = Polyline(
         np.array(
             [
@@ -450,8 +457,7 @@ def test_partition_by_length_closed():
     expected_indices = np.array([0, 1, 2, 5, 6, 7])
 
     for max_length in (2.0, 2.5, 2.6):
-        result = original.copy()
-        indices = result.partition_by_length(max_length, ret_indices=True)
+        result, indices = original.subdivided_by_length(max_length, ret_indices=True)
 
         np.testing.assert_array_almost_equal(result.v, expected.v)
         np.testing.assert_array_equal(result.e, expected.e)