From 70cd66a913dc253f54fd9b938b22988d654fde69 Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Wed, 8 Jan 2020 22:07:26 -0800 Subject: [PATCH 1/2] Making `_copy` a public argument (`copy`). --- src/python/bezier/_base.py | 6 +-- src/python/bezier/curve.py | 26 +++++------ src/python/bezier/surface.py | 36 +++++++-------- tests/functional/test_surface_locate.py | 8 ++-- tests/functional/utils.py | 4 +- tests/unit/test__base.py | 8 ++-- tests/unit/test__plot_helpers.py | 2 +- tests/unit/test__py_surface_helpers.py | 8 ++-- tests/unit/test_curve.py | 12 ++--- tests/unit/test_surface.py | 58 ++++++++++++------------- 10 files changed, 84 insertions(+), 84 deletions(-) diff --git a/src/python/bezier/_base.py b/src/python/bezier/_base.py index 38be0dba..b5a35677 100644 --- a/src/python/bezier/_base.py +++ b/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 diff --git a/src/python/bezier/curve.py b/src/python/bezier/curve.py index 59b6aec5..bf0c7737 100644 --- a/src/python/bezier/curve.py +++ b/src/python/bezier/curve.py @@ -84,7 +84,7 @@ 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. """ @@ -95,12 +95,12 @@ 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): + super(Curve, self).__init__(nodes, copy=copy) self._degree = degree @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 +110,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 +120,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) @staticmethod def _get_degree(num_nodes): @@ -167,13 +167,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) def evaluate(self, s): r"""Evaluate :math:`B(s)` along the curve. @@ -315,8 +315,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) + right = Curve(right_nodes, self._degree, copy=False) return left, right def intersect( @@ -443,7 +443,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) def reduce_(self): r"""Return a degree-reduced version of the current curve. @@ -540,7 +540,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) def specialize(self, start, end): """Specialize the curve to a given sub-interval. @@ -599,7 +599,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) def locate(self, point): r"""Find a point on the current curve. diff --git a/src/python/bezier/surface.py b/src/python/bezier/surface.py index e08e0540..e5164805 100644 --- a/src/python/bezier/surface.py +++ b/src/python/bezier/surface.py @@ -177,7 +177,7 @@ 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. """ @@ -189,13 +189,13 @@ 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): + super(Surface, self).__init__(nodes, copy=copy) self._degree = degree self._edges = None @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 +205,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,7 +215,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) @staticmethod def _get_degree(num_nodes): @@ -302,9 +302,9 @@ 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) + edge2 = _curve_mod.Curve(nodes2, self._degree, copy=False) + edge3 = _curve_mod.Curve(nodes3, self._degree, copy=False) return edge1, edge2, edge3 def _get_edges(self): @@ -353,9 +353,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 +751,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), + Surface(nodes_b, self._degree, copy=False), + Surface(nodes_c, self._degree, copy=False), + Surface(nodes_d, self._degree, copy=False), ) def _compute_valid(self): @@ -1110,7 +1110,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) def _make_intersection(edge_info, all_edge_nodes): @@ -1137,7 +1137,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) edges.append(edge) return curved_polygon.CurvedPolygon( *edges, metadata=edge_info, _verify=False diff --git a/tests/functional/test_surface_locate.py b/tests/functional/test_surface_locate.py index 5d9cc71c..ec4788bc 100644 --- a/tests/functional/test_surface_locate.py +++ b/tests/functional/test_surface_locate.py @@ -20,7 +20,7 @@ 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]]) @@ -28,7 +28,7 @@ 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]] diff --git a/tests/functional/utils.py b/tests/functional/utils.py index 2272de94..c9cd694e 100644 --- a/tests/functional/utils.py +++ b/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 diff --git a/tests/unit/test__base.py b/tests/unit/test__base.py index a503fda5..2074f449 100644 --- a/tests/unit/test__base.py +++ b/tests/unit/test__base.py @@ -36,7 +36,7 @@ def test_constructor(self): def test_constructor_without_copy(self): nodes = np.asfortranarray([[0.0, 1.0, 2.0], [0.0, 1.0, 3.0]]) - shape = self._make_one(nodes, _copy=False) + shape = self._make_one(nodes, copy=False) self.assertEqual(shape._degree, -1) self.assertEqual(shape._dimension, 2) self.assertIs(shape._nodes, nodes) @@ -56,7 +56,7 @@ def test_constructor_wrong_dimension(self): def test_constructor_change_order(self): nodes = np.array([[10.0, 1.0], [3.5, 2.0]], order="C") - shape = self._make_one(nodes, _copy=False) + shape = self._make_one(nodes, copy=False) self.assertFalse(nodes.flags.f_contiguous) self.assertTrue(shape._nodes.flags.f_contiguous) @@ -64,14 +64,14 @@ def test_constructor_change_order(self): def test_constructor_non_array(self): nodes = [[10.25, 20.0], [30.5, 4.0]] - shape = self._make_one(nodes, _copy=False) + shape = self._make_one(nodes, copy=False) self.assertIsInstance(shape._nodes, np.ndarray) self.assertTrue(np.all(nodes == shape._nodes)) def test_constructor_convert_dtype(self): nodes = np.asfortranarray([[10, 20], [30, 4]]) - shape = self._make_one(nodes, _copy=False) + shape = self._make_one(nodes, copy=False) self.assertEqual(nodes.dtype, np.dtype(int)) self.assertEqual(shape._nodes.dtype, np.float64) diff --git a/tests/unit/test__plot_helpers.py b/tests/unit/test__plot_helpers.py index 5d221fc9..1ca04c0a 100644 --- a/tests/unit/test__plot_helpers.py +++ b/tests/unit/test__plot_helpers.py @@ -108,7 +108,7 @@ def _plot_check(self, ax, expected, color): def _get_info(nodes): from bezier import surface as surface_mod - surface = surface_mod.Surface(nodes, 1, _copy=False) + surface = surface_mod.Surface(nodes, 1, copy=False) edges = surface._get_edges() expected = np.empty((2, 7), order="F") expected[:, 0] = 0.5 * (nodes[:, 0] + nodes[:, 1]) diff --git a/tests/unit/test__py_surface_helpers.py b/tests/unit/test__py_surface_helpers.py index b6d050d4..4339782e 100644 --- a/tests/unit/test__py_surface_helpers.py +++ b/tests/unit/test__py_surface_helpers.py @@ -849,7 +849,7 @@ def test_linear(self): nodes = np.asfortranarray([[0.0, 1.0, 0.0], [0.0, 0.0, 2.0]]) degree = 1 - surface = bezier.Surface(nodes, degree=degree, _copy=False) + surface = bezier.Surface(nodes, degree=degree, copy=False) self.assertTrue(surface.is_valid) st_vals = np.asfortranarray(RANDOM((13, 2))) result = self._call_function_under_test(nodes, degree, st_vals) @@ -863,7 +863,7 @@ def test_nonlinear(self): [[0.0, 0.5, 1.0, 0.0, 1.0, 0.0], [0.0, 0.0, 0.0, 0.5, 1.0, 1.0]] ) degree = 2 - surface = bezier.Surface(nodes, degree=degree, _copy=False) + surface = bezier.Surface(nodes, degree=degree, copy=False) self.assertTrue(surface.is_valid) # B(s, t) = [s(t + 1), t(s + 1)] st_vals = np.asfortranarray( @@ -1210,7 +1210,7 @@ def test_not_corner(self): def test_s_corner(self): import bezier - surface = bezier.Surface(UNIT_TRIANGLE, degree=1, _copy=False) + surface = bezier.Surface(UNIT_TRIANGLE, degree=1, copy=False) edge_nodes1 = tuple(edge._nodes for edge in surface.edges) edge_nodes2 = () intersection = make_intersect(2, 0.0, None, 0.5) @@ -1240,7 +1240,7 @@ def test_s_corner(self): def test_t_corner(self): import bezier - surface = bezier.Surface(UNIT_TRIANGLE, degree=1, _copy=False) + surface = bezier.Surface(UNIT_TRIANGLE, degree=1, copy=False) edge_nodes1 = () edge_nodes2 = tuple(edge._nodes for edge in surface.edges) intersection = make_intersect(None, 0.5, 1, 0.0) diff --git a/tests/unit/test_curve.py b/tests/unit/test_curve.py index f0de417f..293d1c39 100644 --- a/tests/unit/test_curve.py +++ b/tests/unit/test_curve.py @@ -32,7 +32,7 @@ def _make_one(self, *args, **kwargs): def test_constructor(self): nodes = np.asfortranarray([[0.0, 0.625, 1.0], [0.0, 0.5, 0.75]]) - curve = self._make_one(nodes, 2, _copy=False) + curve = self._make_one(nodes, 2, copy=False) self.assertEqual(curve._degree, 2) self.assertEqual(curve._dimension, 2) self.assertIs(curve._nodes, nodes) @@ -74,7 +74,7 @@ def test_length_property(self): self.assertEqual(curve.length, 5.0) def test___dict___property(self): - curve = self._make_one(self.ZEROS, 1, _copy=False) + curve = self._make_one(self.ZEROS, 1, copy=False) props_dict = curve.__dict__ expected = {"_nodes": self.ZEROS, "_dimension": 2, "_degree": 1} self.assertEqual(props_dict, expected) @@ -82,11 +82,11 @@ def test___dict___property(self): expected["_dimension"] = 47 self.assertNotEqual(curve._dimension, expected["_dimension"]) - def test__copy(self): + def test_copy(self): nodes = np.asfortranarray([[2.0, 3.5, 4.0], [0.0, 1.0, 0.0]]) curve = self._make_one(nodes, 2) - new_curve = curve._copy() + new_curve = curve.copy() self.assertEqual(curve._degree, new_curve._degree) self.assertEqual(curve._dimension, new_curve._dimension) self.assertTrue(np.all(curve._nodes == new_curve._nodes)) @@ -124,7 +124,7 @@ def test_plot_defaults(self, new_axis_mock): ax = unittest.mock.Mock(spec=["plot"]) new_axis_mock.return_value = ax nodes = np.asfortranarray([[0.0, 1.0], [1.0, 3.0]]) - curve = self._make_one(nodes, 1, _copy=False) + curve = self._make_one(nodes, 1, copy=False) num_pts = 2 # This value is crucial for the plot call. result = curve.plot(num_pts) self.assertIs(result, ax) @@ -139,7 +139,7 @@ def test_plot_defaults(self, new_axis_mock): @unittest.mock.patch("bezier._plot_helpers.new_axis") def test_plot_explicit(self, new_axis_mock): nodes = np.asfortranarray([[0.0, 1.0], [0.0, 1.0]]) - curve = self._make_one(nodes, 1, _copy=False) + curve = self._make_one(nodes, 1, copy=False) num_pts = 2 # This value is crucial for the plot call. ax = unittest.mock.Mock(spec=["plot"]) color = (0.75, 1.0, 1.0) diff --git a/tests/unit/test_surface.py b/tests/unit/test_surface.py index ece50805..19c7f59e 100644 --- a/tests/unit/test_surface.py +++ b/tests/unit/test_surface.py @@ -44,7 +44,7 @@ class NoSlots(self._get_target_class()): def test_constructor(self): nodes = np.asfortranarray([[0.0, 0.625, 1.0], [0.0, 0.5, 0.75]]) - surface = self._make_one(nodes, 1, _copy=False) + surface = self._make_one(nodes, 1, copy=False) self.assertEqual(surface._degree, 1) self.assertEqual(surface._dimension, 2) self.assertIs(surface._nodes, nodes) @@ -239,22 +239,22 @@ def test_edges_property(self): def test_edges_property_cached(self): surface = self._make_one_no_slots(self.ZEROS, 1) # Create mock "edges" to be computed. - sentinel1 = unittest.mock.Mock(spec=["_copy"]) - sentinel2 = unittest.mock.Mock(spec=["_copy"]) - sentinel3 = unittest.mock.Mock(spec=["_copy"]) + sentinel1 = unittest.mock.Mock(spec=["copy"]) + sentinel2 = unittest.mock.Mock(spec=["copy"]) + sentinel3 = unittest.mock.Mock(spec=["copy"]) expected = sentinel1, sentinel2, sentinel3 surface._compute_edges = unittest.mock.Mock(return_value=expected) # Make sure the "edges" when copied just return themselves. - sentinel1._copy.return_value = sentinel1 - sentinel2._copy.return_value = sentinel2 - sentinel3._copy.return_value = sentinel3 + sentinel1.copy.return_value = sentinel1 + sentinel2.copy.return_value = sentinel2 + sentinel3.copy.return_value = sentinel3 # Access the property and check the mocks. self.assertEqual(surface.edges, expected) surface._compute_edges.assert_any_call() self.assertEqual(surface._compute_edges.call_count, 1) - sentinel1._copy.assert_called_once_with() - sentinel2._copy.assert_called_once_with() - sentinel3._copy.assert_called_once_with() + sentinel1.copy.assert_called_once_with() + sentinel2.copy.assert_called_once_with() + sentinel3.copy.assert_called_once_with() # Access again but make sure no more calls to _compute_edges(). self.assertEqual(surface.edges, expected) self.assertEqual(surface._compute_edges.call_count, 1) @@ -282,7 +282,7 @@ def test__verify_barycentric(self): klass._verify_barycentric(0.875, 0.25, -0.125) def test_evaluate_barycentric(self): - surface = self._make_one(self.UNIT_TRIANGLE, 1, _copy=False) + surface = self._make_one(self.UNIT_TRIANGLE, 1, copy=False) lambda_vals = (0.25, 0.0, 0.75) # Just make sure we call the helper. patch = unittest.mock.patch( @@ -320,7 +320,7 @@ def test_evaluate_barycentric_multi_wrong_dimension(self): def _eval_bary_multi_helper(self, **kwargs): nodes = np.asfortranarray([[0.0, 2.0, -3.0], [0.0, 1.0, 2.0]]) - surface = self._make_one(nodes, 1, _copy=False) + surface = self._make_one(nodes, 1, copy=False) param_vals = np.asfortranarray([[1.0, 0.0, 0.0]]) patch = unittest.mock.patch( "bezier._surface_helpers.evaluate_barycentric_multi", @@ -374,7 +374,7 @@ def test_evaluate_cartesian_no_verify(self): def test_evaluate_cartesian_calls_helper(self): nodes = self.ZEROS - surface = self._make_one_no_slots(nodes, 1, _copy=False) + surface = self._make_one_no_slots(nodes, 1, copy=False) patch = unittest.mock.patch( "bezier._surface_helpers.evaluate_barycentric", return_value=unittest.mock.sentinel.point, @@ -394,7 +394,7 @@ def test_evaluate_cartesian_multi_wrong_dimension(self): def _eval_cartesian_multi_helper(self, **kwargs): nodes = np.asfortranarray([[2.0, 0.0, 3.0], [3.0, 2.0, 7.5]]) - surface = self._make_one(nodes, 1, _copy=False) + surface = self._make_one(nodes, 1, copy=False) param_vals = np.asfortranarray([[1.0, 0.0]]) patch = unittest.mock.patch( "bezier._surface_helpers.evaluate_cartesian_multi", @@ -415,7 +415,7 @@ def test_plot_wrong_dimension(self): nodes = np.asfortranarray( [[0.0, 1.0, 2.0], [0.0, 3.0, 6.0], [0.0, 4.0, 9.0]] ) - surface = self._make_one(nodes, 1, _copy=False) + surface = self._make_one(nodes, 1, copy=False) with self.assertRaises(NotImplementedError): surface.plot(32) @@ -424,7 +424,7 @@ def test_plot_wrong_dimension(self): def test_plot_defaults(self, add_patch_mock, new_axis_mock): ax = unittest.mock.Mock(spec=[]) new_axis_mock.return_value = ax - curve = self._make_one(self.UNIT_TRIANGLE, 1, _copy=False) + curve = self._make_one(self.UNIT_TRIANGLE, 1, copy=False) pts_per_edge = 16 result = curve.plot(pts_per_edge) self.assertIs(result, ax) @@ -439,7 +439,7 @@ def test_plot_defaults(self, add_patch_mock, new_axis_mock): def test_plot_explicit(self, add_patch_mock, new_axis_mock): ax = unittest.mock.Mock(spec=["plot"]) color = (0.5, 0.5, 0.5) - curve = self._make_one(self.UNIT_TRIANGLE, 1, _copy=False) + curve = self._make_one(self.UNIT_TRIANGLE, 1, copy=False) pts_per_edge = 16 result = curve.plot(pts_per_edge, color=color, ax=ax, with_nodes=True) self.assertIs(result, ax) @@ -464,7 +464,7 @@ def test_plot_explicit(self, add_patch_mock, new_axis_mock): def test_subdivide(self): klass = self._get_target_class() degree = 1 - surface = self._make_one(self.UNIT_TRIANGLE, degree, _copy=False) + surface = self._make_one(self.UNIT_TRIANGLE, degree, copy=False) surface_a, surface_b, surface_c, surface_d = surface.subdivide() # Check sub-surface A. self.assertIsInstance(surface_a, klass) @@ -501,11 +501,11 @@ def test__compute_valid_linear(self): # Change the nodes from counterclockwise to clockwise, so the # Jacobian becomes negatively signed. nodes = np.asfortranarray(nodes[:, (1, 0, 2)]) - surface = self._make_one(nodes, 1, _copy=False) + surface = self._make_one(nodes, 1, copy=False) self.assertFalse(surface._compute_valid()) # Collinear. nodes = np.asfortranarray([[0.0, 1.0, 2.0], [0.0, 2.0, 4.0]]) - surface = self._make_one(nodes, 1, _copy=False) + surface = self._make_one(nodes, 1, copy=False) self.assertFalse(surface._compute_valid()) def test__compute_valid_quadratic(self): @@ -516,18 +516,18 @@ def test__compute_valid_quadratic(self): [0.0, -0.1875, 0.0, 0.5, 0.625, 1.0], ] ) - surface = self._make_one(nodes, 2, _copy=False) + surface = self._make_one(nodes, 2, copy=False) self.assertTrue(surface._compute_valid()) # Change the nodes from counterclockwise to clockwise, so the # Jacobian becomes negatively signed. nodes = np.asfortranarray(nodes[:, (2, 1, 0, 4, 3, 5)]) - surface = self._make_one(nodes, 2, _copy=False) + surface = self._make_one(nodes, 2, copy=False) self.assertFalse(surface._compute_valid()) # Mixed sign Jacobian: B(L1, L2, L3) = [L1^2 + L2^2, L2^2 + L3^2] nodes = np.asfortranarray( [[1.0, 0.0, 1.0, 0.0, 0.0, 0.0], [0.0, 0.0, 1.0, 0.0, 0.0, 1.0]] ) - surface = self._make_one(nodes, 2, _copy=False) + surface = self._make_one(nodes, 2, copy=False) self.assertFalse(surface._compute_valid()) def test__compute_valid_cubic(self): @@ -538,12 +538,12 @@ def test__compute_valid_cubic(self): [0.0, 0.0, 0.0, 0.0, 1.0, 1.0, 1.25, 2.0, 2.25, 3.0], ] ) - surface = self._make_one(nodes, 3, _copy=False) + surface = self._make_one(nodes, 3, copy=False) self.assertTrue(surface._compute_valid()) # Change the nodes from counterclockwise to clockwise, so the # Jacobian becomes negatively signed. nodes = np.asfortranarray(nodes[:, (3, 2, 1, 0, 6, 5, 4, 8, 7, 9)]) - surface = self._make_one(nodes, 3, _copy=False) + surface = self._make_one(nodes, 3, copy=False) self.assertFalse(surface._compute_valid()) # Mixed sign Jacobian: B(L1, L2, L3) = [L1^3 + L2^3, L2^3 + L3^3] nodes = np.asfortranarray( @@ -552,7 +552,7 @@ def test__compute_valid_cubic(self): [0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 0.0, 0.0, 1.0], ] ) - surface = self._make_one(nodes, 3, _copy=False) + surface = self._make_one(nodes, 3, copy=False) self.assertFalse(surface._compute_valid()) def test__compute_valid_bad_degree(self): @@ -572,7 +572,7 @@ def test_is_valid_property(self): self.assertTrue(surface.is_valid) def test___dict___property(self): - surface = self._make_one(self.UNIT_TRIANGLE, 1, _copy=False) + surface = self._make_one(self.UNIT_TRIANGLE, 1, copy=False) props_dict = surface.__dict__ expected = { "_nodes": self.UNIT_TRIANGLE, @@ -754,9 +754,9 @@ def test_it(self): import bezier nodes1 = np.asfortranarray([[0.0, 1.0, 0.0], [0.0, 0.0, 1.0]]) - surface1 = bezier.Surface(nodes1, degree=1, _copy=False) + surface1 = bezier.Surface(nodes1, degree=1, copy=False) nodes2 = np.asfortranarray([[0.25, -0.75, 0.25], [0.25, 0.25, -0.75]]) - surface2 = bezier.Surface(nodes2, degree=1, _copy=False) + surface2 = bezier.Surface(nodes2, degree=1, copy=False) edge_nodes = tuple(edge._nodes for edge in surface1.edges) + tuple( edge._nodes for edge in surface2.edges ) From fa32f7ef9182df16f750484758d211116af125c6 Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Wed, 8 Jan 2020 23:02:00 -0800 Subject: [PATCH 2/2] Adding `verify=True` argument to Curve and Surface constructors. --- src/python/bezier/curve.py | 44 ++++++++++++++++++---- src/python/bezier/surface.py | 72 ++++++++++++++++++++++++++---------- tests/unit/test_curve.py | 13 ++++++- tests/unit/test_surface.py | 36 ++++++++++++++---- 4 files changed, 129 insertions(+), 36 deletions(-) diff --git a/src/python/bezier/curve.py b/src/python/bezier/curve.py index bf0c7737..de18ce36 100644 --- a/src/python/bezier/curve.py +++ b/src/python/bezier/curve.py @@ -87,6 +87,8 @@ class Curve(_base.Base): 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,9 +97,10 @@ class Curve(_base.Base): "_degree", # From constructor ) - def __init__(self, nodes, degree, copy=True): + 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): @@ -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. @@ -173,7 +201,7 @@ def copy(self): 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. diff --git a/src/python/bezier/surface.py b/src/python/bezier/surface.py index e5164805..6d8f8271 100644 --- a/src/python/bezier/surface.py +++ b/src/python/bezier/surface.py @@ -180,6 +180,8 @@ class Surface(_base.Base): 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,10 +191,11 @@ class Surface(_base.Base): "_edges", # Empty default ) - def __init__(self, nodes, degree, copy=True): + 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): @@ -215,12 +218,19 @@ 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. @@ -228,20 +238,38 @@ def _get_degree(num_nodes): 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): @@ -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 diff --git a/tests/unit/test_curve.py b/tests/unit/test_curve.py index 293d1c39..009da0c6 100644 --- a/tests/unit/test_curve.py +++ b/tests/unit/test_curve.py @@ -18,6 +18,7 @@ class TestCurve(utils.NumPyTestCase): + ZEROS = np.zeros((2, 2), order="F") @staticmethod @@ -37,6 +38,16 @@ def test_constructor(self): self.assertEqual(curve._dimension, 2) self.assertIs(curve._nodes, nodes) + def test_constructor_invalid_num_nodes(self): + nodes = np.empty((1, 3), order="F") + with self.assertRaises(ValueError) as exc_info: + self._make_one(nodes, 7, copy=False) + + exc_args = exc_info.exception.args + self.assertEqual( + exc_args, ("A degree 7 curve should have 8 nodes, not 3.",) + ) + def test_constructor_wrong_dimension(self): nodes = np.asfortranarray([1.0, 2.0]) with self.assertRaises(ValueError): @@ -243,7 +254,7 @@ def test_intersect_unsupported_dimension(self): [[0.0, 0.5, 1.0], [0.0, -0.25, 0.0], [0.0, 0.75, 1.25]] ) curve1 = self._make_one(nodes, 2) - curve2 = self._make_one(nodes[:, :2], 2) + curve2 = self._make_one(nodes[:, :2], 1) with self.assertRaises(NotImplementedError): curve1.intersect(curve2) with self.assertRaises(NotImplementedError): diff --git a/tests/unit/test_surface.py b/tests/unit/test_surface.py index 19c7f59e..1409be31 100644 --- a/tests/unit/test_surface.py +++ b/tests/unit/test_surface.py @@ -18,6 +18,7 @@ class TestSurface(utils.NumPyTestCase): + REF_TRIANGLE = utils.ref_triangle_uniform_nodes(5) REF_TRIANGLE3 = utils.ref_triangle_uniform_nodes(3) QUADRATIC = np.asfortranarray( @@ -58,6 +59,16 @@ def test_constructor_wrong_dimension(self): with self.assertRaises(ValueError): self._make_one(nodes, 1) + def test_constructor_invalid_degree(self): + nodes = np.empty((1, 6), order="F") + with self.assertRaises(ValueError) as exc_info: + self._make_one(nodes, 1) + + exc_args = exc_info.exception.args + self.assertEqual( + exc_args, ("A degree 1 surface should have 3 nodes, not 6.",) + ) + def test_from_nodes_factory(self): nodes = np.asfortranarray( [ @@ -84,6 +95,20 @@ def test_from_nodes_factory_non_array(self): self.assertTrue(np.all(surface._nodes == nodes)) self.assertIsNone(surface._edges) + def test_from_nodes_factory_invalid_degree(self): + klass = self._get_target_class() + messages = { + 2: "A degree 1 surface should have 3 nodes, not 2.", + 9: "A degree 3 surface should have 10 nodes, not 9.", + } + for num_nodes, message in messages.items(): + with self.assertRaises(ValueError) as exc_info: + nodes = np.empty((1, num_nodes), order="F") + klass.from_nodes(nodes) + + exc_args = exc_info.exception.args + self.assertEqual(exc_args, (message,)) + def test___repr__(self): nodes = np.zeros((3, 15), order="F") surface = self._make_one(nodes, 4) @@ -98,15 +123,10 @@ def test__get_degree_valid(self): self.assertEqual(3, klass._get_degree(10)) self.assertEqual(11, klass._get_degree(78)) - def test__get_degree_invalid(self): - klass = self._get_target_class() - with self.assertRaises(ValueError): - klass._get_degree(2) - with self.assertRaises(ValueError): - klass._get_degree(9) - def test_area_property_wrong_dimension(self): - nodes = np.asfortranarray([[0.0, 0.0], [1.0, 2.0], [2.0, 3.0]]) + nodes = np.asfortranarray( + [[0.0, 0.0, 0.0], [1.0, 2.0, 0.0], [2.0, 3.0, 0.0]] + ) surface = self._make_one(nodes, 1) with self.assertRaises(NotImplementedError) as exc_info: getattr(surface, "area")