Adding copy and verify to Curve and Surface constructors. (#163)

The `_copy` argument is just being made public but `verify` is 100% new.

src/python/bezier/_base.py

@@ -27,7 +27,7 @@ class Base:
             shape. Must be convertible to a 2D NumPy array of floating point
             values, where the columns are the nodes and the rows correspond to
             each dimension the shape occurs in.
-        _copy (bool): Flag indicating if the nodes should be copied before
+        copy (bool): Flag indicating if the nodes should be copied before
             being stored. Defaults to :data:`True` since callers may
             freely mutate ``nodes`` after passing in.
 
@@ -38,11 +38,11 @@ class Base:
     __slots__ = ("_dimension", "_nodes")
     _degree = -1
 
-    def __init__(self, nodes, _copy=True):
+    def __init__(self, nodes, copy=True):
         nodes_np = sequence_to_array(nodes)
         dimension, _ = nodes_np.shape
         self._dimension = dimension
-        if _copy:
+        if copy:
             self._nodes = nodes_np.copy(order="F")
         else:
             self._nodes = nodes_np

src/python/bezier/curve.py

@@ -84,9 +84,11 @@ class Curve(_base.Base):
         degree (int): The degree of the curve. This is assumed to
             correctly correspond to the number of ``nodes``. Use
             :meth:`from_nodes` if the degree has not yet been computed.
-        _copy (bool): Flag indicating if the nodes should be copied before
+        copy (bool): Flag indicating if the nodes should be copied before
             being stored. Defaults to :data:`True` since callers may
             freely mutate ``nodes`` after passing in.
+        verify (bool): Flag indicating if the degree should be verified against
+            the number of nodes. Defaults to :data:`True`.
     """
 
     __slots__ = (
@@ -95,12 +97,13 @@ class Curve(_base.Base):
         "_degree",  # From constructor
     )
 
-    def __init__(self, nodes, degree, _copy=True):
-        super(Curve, self).__init__(nodes, _copy=_copy)
+    def __init__(self, nodes, degree, *, copy=True, verify=True):
+        super(Curve, self).__init__(nodes, copy=copy)
         self._degree = degree
+        self._verify_degree(verify)
 
     @classmethod
-    def from_nodes(cls, nodes, _copy=True):
+    def from_nodes(cls, nodes, copy=True):
         """Create a :class:`.Curve` from nodes.
 
         Computes the ``degree`` based on the shape of ``nodes``.
@@ -110,7 +113,7 @@ def from_nodes(cls, nodes, _copy=True):
                 Must be convertible to a 2D NumPy array of floating point
                 values, where the columns represent each node while the rows
                 are the dimension of the ambient space.
-            _copy (bool): Flag indicating if the nodes should be copied before
+            copy (bool): Flag indicating if the nodes should be copied before
                 being stored. Defaults to :data:`True` since callers may
                 freely mutate ``nodes`` after passing in.
 
@@ -120,7 +123,7 @@ def from_nodes(cls, nodes, _copy=True):
         nodes_np = _base.sequence_to_array(nodes)
         _, num_nodes = nodes_np.shape
         degree = cls._get_degree(num_nodes)
-        return cls(nodes_np, degree, _copy=_copy)
+        return cls(nodes_np, degree, copy=copy, verify=False)
 
     @staticmethod
     def _get_degree(num_nodes):
@@ -134,6 +137,31 @@ def _get_degree(num_nodes):
         """
         return num_nodes - 1
 
+    def _verify_degree(self, verify):
+        """Verify that the number of nodes matches the degree.
+
+        Args:
+            verify (bool): Flag indicating if the degree should be verified
+                against the number of nodes.
+
+        Raises:
+            ValueError: If ``verify`` is :data:`True` and the number of nodes
+                does not match the degree.
+        """
+        if not verify:
+            return
+
+        _, num_nodes = self._nodes.shape
+        expected_nodes = self._degree + 1
+        if expected_nodes == num_nodes:
+            return
+
+        msg = (
+            f"A degree {self._degree} curve should have "
+            f"{expected_nodes} nodes, not {num_nodes}."
+        )
+        raise ValueError(msg)
+
     @property
     def length(self):
         r"""The length of the current curve.
@@ -167,13 +195,13 @@ class defines ``__slots__`` so by default would not provide a
             "_degree": self._degree,
         }
 
-    def _copy(self):
+    def copy(self):
         """Make a copy of the current curve.
 
         Returns:
             .Curve: Copy of current curve.
         """
-        return Curve(self._nodes, self._degree, _copy=True)
+        return Curve(self._nodes, self._degree, copy=True, verify=False)
 
     def evaluate(self, s):
         r"""Evaluate :math:`B(s)` along the curve.
@@ -315,8 +343,8 @@ def subdivide(self):
             Tuple[Curve, Curve]: The left and right sub-curves.
         """
         left_nodes, right_nodes = _curve_helpers.subdivide_nodes(self._nodes)
-        left = Curve(left_nodes, self._degree, _copy=False)
-        right = Curve(right_nodes, self._degree, _copy=False)
+        left = Curve(left_nodes, self._degree, copy=False, verify=False)
+        right = Curve(right_nodes, self._degree, copy=False, verify=False)
         return left, right
 
     def intersect(
@@ -443,7 +471,7 @@ def elevate(self):
             Curve: The degree-elevated curve.
         """
         new_nodes = _curve_helpers.elevate_nodes(self._nodes)
-        return Curve(new_nodes, self._degree + 1, _copy=False)
+        return Curve(new_nodes, self._degree + 1, copy=False, verify=False)
 
     def reduce_(self):
         r"""Return a degree-reduced version of the current curve.
@@ -540,7 +568,7 @@ def reduce_(self):
             Curve: The degree-reduced curve.
         """
         new_nodes = _curve_helpers.reduce_pseudo_inverse(self._nodes)
-        return Curve(new_nodes, self._degree - 1, _copy=False)
+        return Curve(new_nodes, self._degree - 1, copy=False, verify=False)
 
     def specialize(self, start, end):
         """Specialize the curve to a given sub-interval.
@@ -599,7 +627,7 @@ def specialize(self, start, end):
             Curve: The newly-specialized curve.
         """
         new_nodes = _curve_helpers.specialize_curve(self._nodes, start, end)
-        return Curve(new_nodes, self._degree, _copy=False)
+        return Curve(new_nodes, self._degree, copy=False, verify=False)
 
     def locate(self, point):
         r"""Find a point on the current curve.

src/python/bezier/surface.py

@@ -177,9 +177,11 @@ class Surface(_base.Base):
         degree (int): The degree of the surface. This is assumed to
             correctly correspond to the number of ``nodes``. Use
             :meth:`from_nodes` if the degree has not yet been computed.
-        _copy (bool): Flag indicating if the nodes should be copied before
+        copy (bool): Flag indicating if the nodes should be copied before
             being stored. Defaults to :data:`True` since callers may
             freely mutate ``nodes`` after passing in.
+        verify (bool): Flag indicating if the degree should be verified against
+            the number of nodes. Defaults to :data:`True`.
     """
 
     __slots__ = (
@@ -189,13 +191,14 @@ class Surface(_base.Base):
         "_edges",  # Empty default
     )
 
-    def __init__(self, nodes, degree, _copy=True):
-        super(Surface, self).__init__(nodes, _copy=_copy)
+    def __init__(self, nodes, degree, *, copy=True, verify=True):
+        super(Surface, self).__init__(nodes, copy=copy)
         self._degree = degree
         self._edges = None
+        self._verify_degree(verify)
 
     @classmethod
-    def from_nodes(cls, nodes, _copy=True):
+    def from_nodes(cls, nodes, copy=True):
         """Create a :class:`.Surface` from nodes.
 
         Computes the ``degree`` based on the shape of ``nodes``.
@@ -205,7 +208,7 @@ def from_nodes(cls, nodes, _copy=True):
                 surface. Must be convertible to a 2D NumPy array of floating
                 point values, where the columns represent each node while the
                 rows are the dimension of the ambient space.
-            _copy (bool): Flag indicating if the nodes should be copied before
+            copy (bool): Flag indicating if the nodes should be copied before
                 being stored. Defaults to :data:`True` since callers may
                 freely mutate ``nodes`` after passing in.
 
@@ -215,33 +218,58 @@ def from_nodes(cls, nodes, _copy=True):
         nodes_np = _base.sequence_to_array(nodes)
         _, num_nodes = nodes_np.shape
         degree = cls._get_degree(num_nodes)
-        return cls(nodes_np, degree, _copy=_copy)
+        # NOTE: **Explicitly** verify because ``_get_degree`` does not.
+        return cls(nodes_np, degree, copy=copy, verify=True)
 
     @staticmethod
     def _get_degree(num_nodes):
         """Get the degree of the current surface.
 
+        .. note::
+
+            If ``num_nodes`` isn't a triangular number, no degree can be
+            correct so the return value will be invalid. Callers should use
+            ``verify`` in the constructor to ensure correctness.
+
         Args:
             num_nodes (int): The number of control points for a
                 B |eacute| zier surface.
 
         Returns:
             int: The degree :math:`d` such that :math:`(d + 1)(d + 2)/2`
             equals ``num_nodes``.
-
-        Raises:
-            ValueError: If ``num_nodes`` isn't a triangular number.
         """
         # 8 * num_nodes = 4(d + 1)(d + 2)
         #               = 4d^2 + 12d + 8
         #               = (2d + 3)^2 - 1
         d_float = 0.5 * (np.sqrt(8.0 * num_nodes + 1.0) - 3.0)
-        d_int = int(np.round(d_float))
-        if (d_int + 1) * (d_int + 2) == 2 * num_nodes:
-            return d_int
+        return int(np.round(d_float))
 
-        else:
-            raise ValueError(num_nodes, "not a triangular number")
+    def _verify_degree(self, verify):
+        """Verify that the number of nodes matches the degree.
+
+        Args:
+            verify (bool): Flag indicating if the degree should be verified
+                against the number of nodes.
+
+        Raises:
+            ValueError: If ``verify`` is :data:`True` and the number of nodes
+                does not match the degree.
+        """
+        if not verify:
+            return
+
+        _, num_nodes = self._nodes.shape
+        twice_expected_nodes = (self._degree + 1) * (self._degree + 2)
+        # Avoid rounding by division by 2.
+        if twice_expected_nodes == 2 * num_nodes:
+            return
+
+        msg = (
+            f"A degree {self._degree} surface should have "
+            f"{0.5 * twice_expected_nodes:g} nodes, not {num_nodes}."
+        )
+        raise ValueError(msg)
 
     @property
     def area(self):
@@ -302,9 +330,15 @@ def _compute_edges(self):
         nodes1, nodes2, nodes3 = _surface_helpers.compute_edge_nodes(
             self._nodes, self._degree
         )
-        edge1 = _curve_mod.Curve(nodes1, self._degree, _copy=False)
-        edge2 = _curve_mod.Curve(nodes2, self._degree, _copy=False)
-        edge3 = _curve_mod.Curve(nodes3, self._degree, _copy=False)
+        edge1 = _curve_mod.Curve(
+            nodes1, self._degree, copy=False, verify=False
+        )
+        edge2 = _curve_mod.Curve(
+            nodes2, self._degree, copy=False, verify=False
+        )
+        edge3 = _curve_mod.Curve(
+            nodes3, self._degree, copy=False, verify=False
+        )
         return edge1, edge2, edge3
 
     def _get_edges(self):
@@ -353,9 +387,9 @@ def edges(self):
         # NOTE: It is crucial that we return copies here. Since the edges
         #       are cached, if they were mutable, callers could
         #       inadvertently mutate the cached value.
-        edge1 = edge1._copy()  # pylint: disable=protected-access
-        edge2 = edge2._copy()  # pylint: disable=protected-access
-        edge3 = edge3._copy()  # pylint: disable=protected-access
+        edge1 = edge1.copy()
+        edge2 = edge2.copy()
+        edge3 = edge3.copy()
         return edge1, edge2, edge3
 
     @staticmethod
@@ -751,10 +785,10 @@ def subdivide(self):
             nodes_d,
         ) = _surface_helpers.subdivide_nodes(self._nodes, self._degree)
         return (
-            Surface(nodes_a, self._degree, _copy=False),
-            Surface(nodes_b, self._degree, _copy=False),
-            Surface(nodes_c, self._degree, _copy=False),
-            Surface(nodes_d, self._degree, _copy=False),
+            Surface(nodes_a, self._degree, copy=False, verify=False),
+            Surface(nodes_b, self._degree, copy=False, verify=False),
+            Surface(nodes_c, self._degree, copy=False, verify=False),
+            Surface(nodes_d, self._degree, copy=False, verify=False),
         )
 
     def _compute_valid(self):
@@ -1110,7 +1144,7 @@ def elevate(self):
         # Hold off on division until the end, to (attempt to) avoid round-off.
         denominator = self._degree + 1.0
         new_nodes /= denominator
-        return Surface(new_nodes, self._degree + 1, _copy=False)
+        return Surface(new_nodes, self._degree + 1, copy=False, verify=False)
 
 
 def _make_intersection(edge_info, all_edge_nodes):
@@ -1137,7 +1171,7 @@ def _make_intersection(edge_info, all_edge_nodes):
         nodes = all_edge_nodes[index]
         new_nodes = _curve_helpers.specialize_curve(nodes, start, end)
         degree = new_nodes.shape[1] - 1
-        edge = _curve_mod.Curve(new_nodes, degree, _copy=False)
+        edge = _curve_mod.Curve(new_nodes, degree, copy=False, verify=False)
         edges.append(edge)
     return curved_polygon.CurvedPolygon(
         *edges, metadata=edge_info, _verify=False

tests/functional/test_surface_locate.py

@@ -20,15 +20,15 @@
 CONFIG = utils.Config()
 # F1 = sympy.Matrix([[s, t]])
 SURFACE1 = bezier.Surface.from_nodes(
-    np.asfortranarray([[0.0, 1.0, 0.0], [0.0, 0.0, 1.0]]), _copy=False
+    np.asfortranarray([[0.0, 1.0, 0.0], [0.0, 0.0, 1.0]]), copy=False
 )
 # F2 = sympy.Matrix([[
 #     (-t**2 + 2 * s + t) / 2, (s**2 + 2 * s * t - s + 2 * t) / 2]])
 SURFACE2 = bezier.Surface.from_nodes(
     np.asfortranarray(
         [[0.0, 0.5, 1.0, 0.25, 0.75, 0.0], [0.0, -0.25, 0.0, 0.5, 0.75, 1.0]]
     ),
-    _copy=False,
+    copy=False,
 )
 # F3 = sympy.Matrix([[
 #     -(2 * s * t - 4 * s - t) / 4, (s**2 - s * t + 4 * t) / 4]])
@@ -39,14 +39,14 @@
             [0.0, 0.0, 0.25, 0.5, 0.375, 1.0],
         ]
     ),
-    _copy=False,
+    copy=False,
 )
 # F4 = sympy.Matrix([[2 * (s + 2 * t) * (1 - t), 2 * t * (s + 1)]])
 SURFACE4 = bezier.Surface.from_nodes(
     np.asfortranarray(
         [[0.0, 1.0, 2.0, 2.0, 2.0, 0.0], [0.0, 0.0, 0.0, 1.0, 2.0, 2.0]]
     ),
-    _copy=False,
+    copy=False,
 )
 POINTS = np.asfortranarray(
     [[0.0, 0.25, 0.59375, 0.265625, 1.25], [0.0, 0.25, 0.25, 0.73046875, 1.25]]

tests/functional/utils.py

@@ -370,7 +370,7 @@ class CurveInfo:  # pylint: disable=too-few-public-methods
     def __init__(self, id_, control_points, implicitized=None, note=None):
         self.id_ = id_
         self.control_points = control_points
-        self.curve = bezier.Curve.from_nodes(control_points, _copy=False)
+        self.curve = bezier.Curve.from_nodes(control_points, copy=False)
         self.implicitized = implicitized
         self.note = note
 
@@ -679,7 +679,7 @@ class SurfaceInfo:  # pylint: disable=too-few-public-methods
     def __init__(self, id_, control_points, note=None):
         self.id_ = id_
         self.control_points = control_points
-        self.surface = bezier.Surface.from_nodes(control_points, _copy=False)
+        self.surface = bezier.Surface.from_nodes(control_points, copy=False)
         self.note = note
 
     @classmethod