Repository: lace/polliwog

Issue:
Consider renaming Plane `points_in_front()` method to `points_on_or_in_front()`

Discussion:

Patch:
diff --git a/polliwog/plane/_plane_object.py b/polliwog/plane/_plane_object.py
index 60eb7cc..3c67e1d 100644
--- a/polliwog/plane/_plane_object.py
+++ b/polliwog/plane/_plane_object.py
@@ -148,6 +148,34 @@ class Plane(object):
         return np.sign(self.signed_distance(points))
 
     def points_in_front(self, points, inverted=False, ret_indices=False):
+        """
+        Given an array of points, return the points which lie in the
+        half-space in front of it (i.e. in the direction of the plane
+        normal).
+
+        Args:
+            points (np.arraylikw): An array of points.
+            inverted (bool): When `True`, return the points which lie on or
+                behind the plane instead.
+            ret_indices (bool): When `True`, return the indices instead of the
+                points themselves.
+
+        Note:
+            Use `points_on_or_in_front()` for points which lie either on the
+            plane or in front of it.
+        """
+        sign = self.sign(points)
+
+        if inverted:
+            mask = np.less(sign, 0)
+        else:
+            mask = np.greater(sign, 0)
+
+        indices = np.flatnonzero(mask)
+
+        return indices if ret_indices else points[indices]
+
+    def points_on_or_in_front(self, points, inverted=False, ret_indices=False):
         """
         Given an array of points, return the points which lie either on the
         plane or in the half-space in front of it (i.e. in the direction of
@@ -155,10 +183,14 @@ class Plane(object):
 
         Args:
             points (np.arraylikw): An array of points.
-            inverted (bool): When `True`, effectively invert the plane. Return
-                the points that lie on or behind the plane instead.
+            inverted (bool): When `True`, return the points behind the plane
+                instead.
             ret_indices (bool): When `True`, return the indices instead of the
                 points themselves.
+
+        Note:
+            Use `points_in_front()` to get points which lie only in front of
+            the plane.
         """
         sign = self.sign(points)
 
diff --git a/polliwog/pointcloud/__init__.py b/polliwog/pointcloud/__init__.py
index 7b816e6..d9ea056 100644
--- a/polliwog/pointcloud/__init__.py
+++ b/polliwog/pointcloud/__init__.py
@@ -2,6 +2,6 @@
 Functions for working with point clouds (i.e. unstructured sets of 3D points).
 """
 
-from ._pointcloud_functions import percentile
+from ._pointcloud_functions import extent, percentile
 
-__all__ = ["percentile"]
+__all__ = ["extent", "percentile"]
diff --git a/polliwog/pointcloud/_pointcloud_functions.py b/polliwog/pointcloud/_pointcloud_functions.py
index 5393544..9767839 100644
--- a/polliwog/pointcloud/_pointcloud_functions.py
+++ b/polliwog/pointcloud/_pointcloud_functions.py
@@ -28,3 +28,39 @@ def percentile(points, axis, percentile):
     selected_coord_on_axis = np.percentile(coords_on_axis, percentile)
     centroid = np.average(points, axis=0)
     return vg.reject(centroid, axis) + selected_coord_on_axis * axis
+
+
+def extent(points, ret_indices=False):
+    """
+    Find the distance between the two farthest-most points.
+
+    Args:
+        points (np.arraylike): A `kx3` stack of points.
+        ret_indices (bool): When `True`, return the indices along with the
+            distance.
+
+    Returns:
+        object: With `ret_indices=False`, the distance; with
+        `ret_indices=True` a tuple `(distance, first_index, second_index)`.
+
+    Note:
+        This is implemented using a brute-force method.
+    """
+    k = vg.shape.check(locals(), "points", (-1, 3))
+    if k < 2:
+        raise ValueError("At least two points are required")
+
+    farthest_i = -1
+    farthest_j = -1
+    farthest_distance = -1
+    for i, probe in enumerate(points):
+        distances = vg.euclidean_distance(points, probe)
+        this_farthest_j = np.argmax(distances)
+        if distances[this_farthest_j] > farthest_distance:
+            farthest_i = i
+            farthest_j = this_farthest_j
+            farthest_distance = distances[this_farthest_j]
+    if ret_indices:
+        return farthest_distance, farthest_i, farthest_j
+    else:
+        return farthest_distance
diff --git a/polliwog/shapes/_shapes.py b/polliwog/shapes/_shapes.py
index a21adcc..8c60dd4 100644
--- a/polliwog/shapes/_shapes.py
+++ b/polliwog/shapes/_shapes.py
@@ -1,10 +1,10 @@
 import numpy as np
+import vg
 
 __all__ = [
-    "create_rectangular_prism",
-    "create_cube",
-    "create_triangular_prism",
-    "create_rectangle",
+    "rectangular_prism",
+    "cube",
+    "triangular_prism",
 ]
 
 
@@ -15,7 +15,7 @@ def _maybe_flatten(vertices, faces, ret_unique_vertices_and_faces):
         return vertices[faces]
 
 
-def create_rectangular_prism(origin, size, ret_unique_vertices_and_faces=False):
+def rectangular_prism(origin, size, ret_unique_vertices_and_faces=False):
     """
     Tesselate an axis-aligned rectangular prism. One vertex is `origin`. The
     diametrically opposite vertex is `origin + size`.
@@ -40,6 +40,9 @@ def create_rectangular_prism(origin, size, ret_unique_vertices_and_faces=False):
     """
     from ..tri import quads_to_tris
 
+    vg.shape.check(locals(), "origin", (3,))
+    vg.shape.check(locals(), "size", (3,))
+
     lower_base_plane = np.array(
         [
             # Lower base plane
@@ -73,7 +76,7 @@ def create_rectangular_prism(origin, size, ret_unique_vertices_and_faces=False):
     return _maybe_flatten(vertices, faces, ret_unique_vertices_and_faces)
 
 
-def create_cube(origin, size, ret_unique_vertices_and_faces=False):
+def cube(origin, size, ret_unique_vertices_and_faces=False):
     """
     Tesselate an axis-aligned cube. One vertex is `origin`. The diametrically
     opposite vertex is `size` units along `+x`, `+y`, and `+z`.
@@ -96,14 +99,18 @@ def create_cube(origin, size, ret_unique_vertices_and_faces=False):
         - With `ret_unique_vertices_and_faces=False`: a `12x3x3` matrix of
           flattened triangle coordinates.
     """
-    return create_rectangular_prism(
+    vg.shape.check(locals(), "origin", (3,))
+    if not isinstance(size, float):
+        raise ValueError("`size` should be a number")
+
+    return rectangular_prism(
         origin,
         np.repeat(size, 3),
         ret_unique_vertices_and_faces=ret_unique_vertices_and_faces,
     )
 
 
-def create_triangular_prism(p1, p2, p3, height, ret_unique_vertices_and_faces=False):
+def triangular_prism(p1, p2, p3, height, ret_unique_vertices_and_faces=False):
     """
     Tesselate a triangular prism whose base is the triangle `p1`, `p2`, `p3`.
     If the vertices are oriented in a counterclockwise direction, the prism
@@ -129,6 +136,12 @@ def create_triangular_prism(p1, p2, p3, height, ret_unique_vertices_and_faces=Fa
     """
     from .. import Plane
 
+    vg.shape.check(locals(), "p1", (3,))
+    vg.shape.check(locals(), "p2", (3,))
+    vg.shape.check(locals(), "p3", (3,))
+    if not isinstance(height, float):
+        raise ValueError("`height` should be a number")
+
     base_plane = Plane.from_points(p1, p2, p3)
     lower_base_to_upper_base = height * -base_plane.normal
     vertices = np.vstack(([p1, p2, p3], [p1, p2, p3] + lower_base_to_upper_base))
@@ -148,28 +161,3 @@ def create_triangular_prism(p1, p2, p3, height, ret_unique_vertices_and_faces=Fa
     )
 
     return _maybe_flatten(vertices, faces, ret_unique_vertices_and_faces)
-
-
-def create_rectangle(ret_unique_vertices_and_faces=False):
-    """
-    Create a rectangle.
-
-    Args:
-        ret_unique_vertices_and_faces (bool): When `True` return a vertex
-            array containing the unique vertices and an array of faces (i.e.
-            vertex indices). When `False`, return a flattened array of
-            triangle coordinates.
-
-    Returns:
-        object:
-
-        - With `ret_unique_vertices_and_faces=True`: a tuple containing
-          an `4x3` array of vertices and a `2x3` array of triangle faces.
-        - With `ret_unique_vertices_and_faces=False`: a `16x3x3` matrix of
-          flattened triangle coordinates.
-    """
-    vertices = np.array(
-        [[1.0, 0.0, 0.0], [-1.0, 0.0, 0.0], [0.0, 0.0, 1.0], [0.0, 0.0, -1.0]]
-    )
-    faces = np.array([[0, 1, 2], [3, 1, 0]], dtype=np.uint64)
-    return _maybe_flatten(vertices, faces, ret_unique_vertices_and_faces)


Test patch:
diff --git a/polliwog/plane/test_functions.py b/polliwog/plane/test_functions.py
index 895155a..e29934e 100644
--- a/polliwog/plane/test_functions.py
+++ b/polliwog/plane/test_functions.py
@@ -20,7 +20,7 @@ def assert_plane_equation_satisfies_points(plane_equation, points):
 
 
 def test_plane_normal_from_points_parity():
-    from ..shapes import create_triangular_prism
+    from ..shapes import triangular_prism
     from ..tri import surface_normals
 
     points = np.array([[3.0, 0.0, 0.0], [0.0, 3.0, 0.0], [0.0, 0.0, 3.0]])
@@ -37,7 +37,7 @@ def test_plane_normal_from_points_parity():
     p1 = np.array([3.0, 0.0, 0.0])
     p2 = np.array([0.0, 3.0, 0.0])
     p3 = np.array([0.0, 0.0, 3.0])
-    vertices = create_triangular_prism(p1, p2, p3, 1.0)
+    vertices = triangular_prism(p1, p2, p3, 1.0)
 
     np.testing.assert_allclose(
         plane_normal_from_points(vertices), surface_normals(vertices)
diff --git a/polliwog/plane/test_plane.py b/polliwog/plane/test_plane.py
index 8c86bd3..5fc77c7 100644
--- a/polliwog/plane/test_plane.py
+++ b/polliwog/plane/test_plane.py
@@ -117,15 +117,48 @@ def test_points_in_front():
 
     plane = Plane(sample, normal)
 
-    pts = np.array([[425.0, 425.0, 25.0], [-500.0, -500.0, 25.0]])
+    pts = np.array([[425.0, 425.0, 25.0], [-500.0, -500.0, 25.0], [1.0, 1.0, 0.0]])
+
+    expected_indices = np.array([0])
+    np.testing.assert_array_equal(plane.points_in_front(pts), pts[expected_indices])
+    np.testing.assert_array_equal(
+        plane.points_in_front(pts, ret_indices=True), expected_indices
+    )
+
+    expected_indices = np.array([1])
+    np.testing.assert_array_equal(
+        plane.points_in_front(pts, inverted=True), pts[expected_indices]
+    )
+    np.testing.assert_array_equal(
+        plane.points_in_front(pts, inverted=True, ret_indices=True), expected_indices
+    )
+
+
+def test_points_on_or_in_front():
+    # diagonal plane @ origin - draw a picture!
+    normal = np.array([1.0, 1.0, 0.0])
+    normal /= np.linalg.norm(normal)
+    sample = np.array([1.0, 1.0, 0.0])
+
+    plane = Plane(sample, normal)
+
+    pts = np.array([[425.0, 425.0, 25.0], [-500.0, -500.0, 25.0], [1.0, 1.0, 0.0]])
+
+    expected_indices = np.array([0, 2])
+    np.testing.assert_array_equal(
+        plane.points_on_or_in_front(pts), pts[expected_indices]
+    )
+    np.testing.assert_array_equal(
+        plane.points_on_or_in_front(pts, ret_indices=True), expected_indices
+    )
 
-    np.testing.assert_array_equal(plane.points_in_front(pts), pts[0:1])
+    expected_indices = np.array([1, 2])
     np.testing.assert_array_equal(
-        plane.points_in_front(pts, ret_indices=True), np.array([0])
+        plane.points_on_or_in_front(pts, inverted=True), pts[expected_indices]
     )
-    np.testing.assert_array_equal(plane.points_in_front(pts, inverted=True), pts[1:2])
     np.testing.assert_array_equal(
-        plane.points_in_front(pts, inverted=True, ret_indices=True), np.array([1])
+        plane.points_on_or_in_front(pts, inverted=True, ret_indices=True),
+        expected_indices,
     )
 
 
diff --git a/polliwog/pointcloud/test_pointcloud_functions.py b/polliwog/pointcloud/test_pointcloud_functions.py
index cae1a55..6a1722f 100644
--- a/polliwog/pointcloud/test_pointcloud_functions.py
+++ b/polliwog/pointcloud/test_pointcloud_functions.py
@@ -2,7 +2,7 @@ import math
 import numpy as np
 import pytest
 import vg
-from ._pointcloud_functions import percentile
+from ._pointcloud_functions import extent, percentile
 
 
 def random_points_along_side_of_cylinder(radius=1.0, height=3.0, n_samples=1):
@@ -30,3 +30,18 @@ def test_percentile():
 
     with pytest.raises(ValueError, match="Axis must be non-zero"):
         percentile(points=points, axis=np.array([0, 0, 0]), percentile=75)
+
+
+def test_extent():
+    points = np.array(
+        [[0, 0, 0], [10, 10, 0], [10, 0, 0], [0, 11, 0]], dtype=np.float64
+    )
+    np.testing.assert_almost_equal(extent(points), 14.87, decimal=2)
+
+    distance, i, j = extent(points, ret_indices=True)
+    np.testing.assert_almost_equal(distance, 14.87, decimal=2)
+    assert i == 2
+    assert j == 3
+
+    with pytest.raises(ValueError, match="At least two points are required"):
+        extent(np.array([[0, 0, 0]]))
diff --git a/polliwog/shapes/test_shapes.py b/polliwog/shapes/test_shapes.py
index 2dd4d7a..09d04e1 100644
--- a/polliwog/shapes/test_shapes.py
+++ b/polliwog/shapes/test_shapes.py
@@ -1,8 +1,9 @@
 import numpy as np
-from ._shapes import create_cube, create_rectangle, create_rectangular_prism
+import pytest
+from ._shapes import cube, rectangular_prism, triangular_prism
 
 
-def test_create_rectangular_prism():
+def test_rectangular_prism():
     origin = np.array([3.0, 4.0, 5.0])
     size = np.array([2.0, 10.0, 20.0])
 
@@ -35,36 +36,42 @@ def test_create_rectangular_prism():
         ]
     )
 
-    vertices, faces = create_rectangular_prism(
+    vertices, faces = rectangular_prism(
         origin=origin, size=size, ret_unique_vertices_and_faces=True
     )
     np.testing.assert_array_equal(faces, expected_faces)
     np.testing.assert_array_equal(vertices, expected_vertices)
 
-    flattened_vertices = create_rectangular_prism(
+    flattened_vertices = rectangular_prism(
         origin=origin, size=size, ret_unique_vertices_and_faces=False
     )
     np.testing.assert_array_equal(flattened_vertices, expected_vertices[expected_faces])
 
 
-def test_create_cube():
+def test_cube():
     origin = np.array([3.0, 4.0, 5.0])
     size = 2.0
 
-    flattened_vertices = create_cube(origin=origin, size=size)
+    flattened_vertices = cube(origin=origin, size=size)
 
     expected_first_triangle = np.array(
         [[3.0, 4.0, 5.0], [5.0, 4.0, 5.0], [5.0, 4.0, 7.0]]
     )
     np.testing.assert_array_equal(flattened_vertices[0], expected_first_triangle)
 
+    with pytest.raises(ValueError, match="`size` should be a number"):
+        cube(origin=origin, size="not a number")
 
-def test_create_rectangle():
-    expected_vertices = np.array(
-        [[1.0, 0.0, 0.0], [-1.0, 0.0, 0.0], [0.0, 0.0, 1.0], [0.0, 0.0, -1.0]]
-    )
-    expected_faces = np.array([[0, 1, 2], [3, 1, 0]])
 
-    vertices, faces = create_rectangle(ret_unique_vertices_and_faces=True)
-    np.testing.assert_array_equal(faces, expected_faces)
-    np.testing.assert_array_equal(vertices, expected_vertices)
+def test_triangular_prism():
+    p1 = np.array([3.0, 0.0, 0.0])
+    p2 = np.array([0.0, 3.0, 0.0])
+    p3 = np.array([0.0, 0.0, 3.0])
+
+    flattened_vertices = triangular_prism(p1, p2, p3, 1.0)
+
+    expected_first_triangle = np.array([p1, p2, p3])
+    np.testing.assert_array_equal(flattened_vertices[0], expected_first_triangle)
+
+    with pytest.raises(ValueError, match="`height` should be a number"):
+        triangular_prism(p1, p2, p3, "not-a-number")
diff --git a/polliwog/tri/test_functions.py b/polliwog/tri/test_functions.py
index 74337ca..3f4a5e9 100644
--- a/polliwog/tri/test_functions.py
+++ b/polliwog/tri/test_functions.py
@@ -21,12 +21,12 @@ def test_surface_normals_from_points_single():
 
 
 def test_surface_normals_from_points_vectorized():
-    from ..shapes import create_triangular_prism
+    from ..shapes import triangular_prism
 
     p1 = np.array([3.0, 0.0, 0.0])
     p2 = np.array([0.0, 3.0, 0.0])
     p3 = np.array([0.0, 0.0, 3.0])
-    vertices = create_triangular_prism(p1, p2, p3, 1.0)
+    vertices = triangular_prism(p1, p2, p3, 1.0)
 
     expected_normals = vg.normalize(
         np.array(