From f895a5943bd93940901515d9156597965c6d4d6c Mon Sep 17 00:00:00 2001 From: David Eustis Date: Sat, 27 Jun 2026 16:40:58 +0000 Subject: [PATCH 01/12] pybind: Add S2Cap bindings Binds S2Cap to Python, including constructors, static factories (from_point, from_center_height, from_center_area, empty, full), properties (center, radius, height), predicates (is_valid, is_empty, is_full), geometric ops (complement, expanded, union), containment and intersection methods, mutation (add_point, add_cap), and the S2Region interface (get_cap_bound, cell_union_bound). Also resolves the TODO in s2cell_bindings.cc by wiring up S2Cell.get_cap_bound() now that S2Cap is bound before S2Cell in the module initialization order. get_rect_bound() on both S2Cap and S2Cell remains deferred pending S2LatLngRect bindings. --- src/python/BUILD.bazel | 15 ++ src/python/module.cc | 6 +- src/python/s2cap_bindings.cc | 170 +++++++++++++++++++ src/python/s2cap_test.py | 299 ++++++++++++++++++++++++++++++++++ src/python/s2cell_bindings.cc | 13 +- 5 files changed, 496 insertions(+), 7 deletions(-) create mode 100644 src/python/s2cap_bindings.cc create mode 100644 src/python/s2cap_test.py diff --git a/src/python/BUILD.bazel b/src/python/BUILD.bazel index b6b67d1e..bf5a9083 100644 --- a/src/python/BUILD.bazel +++ b/src/python/BUILD.bazel @@ -32,6 +32,7 @@ pybind_extension( ":s1angle_bindings", ":s1chord_angle_bindings", ":s1interval_bindings", + ":s2cap_bindings", ":s2cell_bindings", ":s2cell_id_bindings", ":s2latlng_bindings", @@ -124,6 +125,14 @@ pybind_library( ], ) +pybind_library( + name = "s2cap_bindings", + srcs = ["s2cap_bindings.cc"], + deps = [ + "//:s2", + ], +) + pybind_library( name = "s2cell_bindings", srcs = ["s2cell_bindings.cc"], @@ -186,6 +195,12 @@ py_test( deps = [":s2geometry_pybind"], ) +py_test( + name = "s2cap_test", + srcs = ["s2cap_test.py"], + deps = [":s2geometry_pybind"], +) + py_test( name = "s2cell_id_test", srcs = ["s2cell_id_test.py"], diff --git a/src/python/module.cc b/src/python/module.cc index cbb0a176..20f7a048 100644 --- a/src/python/module.cc +++ b/src/python/module.cc @@ -10,6 +10,7 @@ void bind_r2rect(py::module& m); void bind_s1angle(py::module& m); void bind_s1chord_angle(py::module& m); void bind_s1interval(py::module& m); +void bind_s2cap(py::module& m); void bind_s2cell(py::module& m); void bind_s2cell_id(py::module& m); void bind_s2latlng(py::module& m); @@ -44,6 +45,9 @@ PYBIND11_MODULE(s2geometry_bindings, m) { // Deps: s1angle, s2point, s2latlng, r2point, r2rect bind_s2cell_id(m); - // Deps: r2rect, s1chord_angle, s2cell_id, s2latlng, s2point + // Deps: s1angle, s1chord_angle, s2cell, s2point + bind_s2cap(m); + + // Deps: r2rect, s1chord_angle, s2cap, s2cell_id, s2latlng, s2point bind_s2cell(m); } diff --git a/src/python/s2cap_bindings.cc b/src/python/s2cap_bindings.cc new file mode 100644 index 00000000..26320daf --- /dev/null +++ b/src/python/s2cap_bindings.cc @@ -0,0 +1,170 @@ +#include +#include +#include + +#include +#include + +#include "s2/s1angle.h" +#include "s2/s1chord_angle.h" +#include "s2/s2cap.h" +#include "s2/s2cell.h" +#include "s2/s2cell_id.h" +#include "s2/s2point.h" + +namespace py = pybind11; + +void bind_s2cap(py::module& m) { + py::class_(m, "S2Cap", + "A disc-shaped region defined by a center and radius on the sphere.\n\n" + "The cap represents a portion of the unit sphere cut off by a plane.\n" + "The boundary is a circle; the cap is a closed set (contains its\n" + "boundary). A cap of radius Pi/2 is a hemisphere; Pi covers the full\n" + "sphere. See s2/s2cap.h for comprehensive documentation.") + + // Constructors + .def(py::init<>(), + "Construct an empty S2Cap.") + .def(py::init(), + py::arg("center"), py::arg("radius"), + "Construct a cap with the given center and S1Angle radius.\n\n" + "A negative radius yields an empty cap; radius >= Pi yields a full\n" + "cap. center should be unit length.") + .def(py::init(), + py::arg("center"), py::arg("radius"), + "Construct a cap with the given center and S1ChordAngle radius.\n\n" + "More efficient than the S1Angle constructor.\n" + "center should be unit length.") + + // Static factories + .def_static("from_point", &S2Cap::FromPoint, py::arg("center"), + "Return a cap containing a single point.\n\n" + "More efficient than S2Cap(center, S1ChordAngle::Zero()).") + .def_static("from_center_height", &S2Cap::FromCenterHeight, + py::arg("center"), py::arg("height"), + "Return a cap with the given center and height.\n\n" + "Height is the distance from the center point to the cutoff plane.\n" + "A negative height yields an empty cap; height >= 2 yields a full\n" + "cap. center should be unit length.") + .def_static("from_center_area", &S2Cap::FromCenterArea, + py::arg("center"), py::arg("area"), + "Return a cap with the given center and surface area in steradians.\n\n" + "The area also equals the solid angle subtended by the cap.\n" + "A negative area yields an empty cap; area >= 4*Pi yields a full cap.\n" + "center should be unit length.") + .def_static("empty", &S2Cap::Empty, + "Return an empty cap (contains no points).") + .def_static("full", &S2Cap::Full, + "Return a full cap (contains all points).") + + // Properties + .def_property_readonly("center", &S2Cap::center, + "The center of the cap as a unit-length S2Point.") + .def_property_readonly("radius", &S2Cap::radius, + "The radius as an S1ChordAngle.") + .def_property_readonly("height", &S2Cap::height, + "The height of the cap (distance from center point to cutoff plane).") + + // Computed accessors + .def("get_radius", &S2Cap::GetRadius, + "Return the radius as an S1Angle.\n\n" + "Requires a trigonometric operation; may differ slightly from the\n" + "value passed to the S1Angle constructor.") + .def("get_area", &S2Cap::GetArea, + "Return the surface area of the cap in steradians.") + .def("get_centroid", &S2Cap::GetCentroid, + "Return the true centroid of the cap multiplied by its surface area.\n\n" + "The result lies on the ray from the origin through the cap's center.\n" + "For zero-radius caps, always returns the origin (0, 0, 0).") + + // Predicates + .def("is_valid", &S2Cap::is_valid, + "Return true if the cap is valid (center is unit-length, " + "radius in [-1, 4]).") + .def("is_empty", &S2Cap::is_empty, + "Return true if the cap contains no points.") + .def("is_full", &S2Cap::is_full, + "Return true if the cap contains all points.") + + // Geometric operations + .def("complement", &S2Cap::Complement, + "Return the complement of the interior of the cap.\n\n" + "Same boundary as this cap but no shared interior points.\n" + "Note: complement of a singleton equals complement of an empty cap.") + .def("expanded", &S2Cap::Expanded, py::arg("distance"), + "Return a cap containing all points within distance of this cap.\n\n" + "Any expansion of an empty cap is still empty.") + .def("union", &S2Cap::Union, py::arg("other"), + "Return the smallest cap enclosing this cap and other.") + + // Containment / intersection + .def("contains", py::overload_cast( + &S2Cap::Contains, py::const_), + py::arg("other"), + "Return true if this cap contains the given cap.") + .def("contains_point", py::overload_cast( + &S2Cap::Contains, py::const_), + py::arg("point"), + "Return true if this cap contains the given point.\n\n" + "point should be unit length.") + .def("contains_cell", py::overload_cast( + &S2Cap::Contains, py::const_), + py::arg("cell"), + "Return true if this cap contains the given cell.") + .def("intersects", py::overload_cast( + &S2Cap::Intersects, py::const_), + py::arg("other"), + "Return true if this cap intersects the given cap.") + .def("interior_intersects", &S2Cap::InteriorIntersects, py::arg("other"), + "Return true if the interior of this cap intersects other.\n\n" + "This relationship is not symmetric: only the interior of this cap\n" + "is tested, not the interior of other.") + .def("interior_contains_point", &S2Cap::InteriorContains, + py::arg("point"), + "Return true if the interior of this cap contains the given point.\n\n" + "point should be unit length.") + .def("may_intersect", &S2Cap::MayIntersect, py::arg("cell"), + "Return true if this cap may intersect the given cell.") + + // Mutation + .def("add_point", &S2Cap::AddPoint, py::arg("point"), + "Expand the cap to include point if necessary.\n\n" + "If empty, sets center to point. Otherwise center is unchanged.\n" + "point should be unit length.") + .def("add_cap", &S2Cap::AddCap, py::arg("other"), + "Expand the cap to include other if necessary.\n\n" + "If this cap is empty, sets it to other.") + + // S2Region interface + .def("get_cap_bound", &S2Cap::GetCapBound, + "Return a bounding cap for this cap (returns self).") + // get_rect_bound() is deferred until S2LatLngRect is bound. + .def("cell_union_bound", [](const S2Cap& self) { + std::vector cell_ids; + self.GetCellUnionBound(&cell_ids); + return cell_ids; + }, + "Return a list of S2CellIds whose union covers this cap.") + + // Operators + .def(py::self == py::self, "Return true if caps are identical.") + .def(py::self != py::self, "Return true if caps are not identical.") + .def("approx_equals", &S2Cap::ApproxEquals, + py::arg("other"), + py::arg("max_error") = S1Angle::Radians(1e-14), + "Return true if this cap is approximately equal to other.\n\n" + "Checks that the angle between centers and the difference in chord\n" + "radii are both within max_error radians.") + + // String representation + .def("__repr__", [](const S2Cap& self) { + std::ostringstream oss; + oss << self; + return oss.str(); + }) + .def("__str__", [](const S2Cap& self) { + std::ostringstream oss; + oss << self; + return oss.str(); + }); +} diff --git a/src/python/s2cap_test.py b/src/python/s2cap_test.py new file mode 100644 index 00000000..7a358349 --- /dev/null +++ b/src/python/s2cap_test.py @@ -0,0 +1,299 @@ +"""Tests for S2Cap pybind11 bindings.""" + +import math +import unittest +import s2geometry_pybind as s2 + + +class TestS2Cap(unittest.TestCase): + + # --- Constructors --- + + def test_default_constructor_is_empty(self): + cap = s2.S2Cap() + self.assertTrue(cap.is_empty()) + self.assertFalse(cap.is_full()) + + def test_constructor_s1angle(self): + center = s2.S2Point(1.0, 0.0, 0.0) + radius = s2.S1Angle.from_radians(math.pi / 4) + cap = s2.S2Cap(center, radius) + self.assertTrue(cap.is_valid()) + self.assertFalse(cap.is_empty()) + self.assertFalse(cap.is_full()) + + def test_constructor_s1chord_angle(self): + center = s2.S2Point(1.0, 0.0, 0.0) + radius = s2.S1ChordAngle(s2.S1Angle.from_radians(math.pi / 4)) + cap = s2.S2Cap(center, radius) + self.assertTrue(cap.is_valid()) + + def test_constructor_negative_radius_is_empty(self): + center = s2.S2Point(1.0, 0.0, 0.0) + cap = s2.S2Cap(center, s2.S1Angle.from_radians(-1.0)) + self.assertTrue(cap.is_empty()) + + def test_constructor_pi_radius_is_full(self): + center = s2.S2Point(1.0, 0.0, 0.0) + cap = s2.S2Cap(center, s2.S1Angle.from_radians(math.pi)) + self.assertTrue(cap.is_full()) + + # --- Static factories --- + + def test_from_point(self): + p = s2.S2Point(0.0, 1.0, 0.0) + cap = s2.S2Cap.from_point(p) + self.assertTrue(cap.contains_point(p)) + self.assertTrue(cap.is_valid()) + + def test_from_center_height(self): + center = s2.S2Point(1.0, 0.0, 0.0) + cap = s2.S2Cap.from_center_height(center, 0.5) + self.assertTrue(cap.is_valid()) + self.assertAlmostEqual(cap.height, 0.5) + + def test_from_center_height_negative_is_empty(self): + center = s2.S2Point(1.0, 0.0, 0.0) + self.assertTrue(s2.S2Cap.from_center_height(center, -1.0).is_empty()) + + def test_from_center_height_two_is_full(self): + center = s2.S2Point(1.0, 0.0, 0.0) + self.assertTrue(s2.S2Cap.from_center_height(center, 2.0).is_full()) + + def test_from_center_area(self): + center = s2.S2Point(1.0, 0.0, 0.0) + area = 2 * math.pi # hemisphere + cap = s2.S2Cap.from_center_area(center, area) + self.assertTrue(cap.is_valid()) + self.assertAlmostEqual(cap.get_area(), area, places=10) + + def test_empty(self): + cap = s2.S2Cap.empty() + self.assertTrue(cap.is_empty()) + self.assertFalse(cap.is_full()) + + def test_full(self): + cap = s2.S2Cap.full() + self.assertTrue(cap.is_full()) + self.assertFalse(cap.is_empty()) + + # --- Properties --- + + def test_center(self): + center = s2.S2Point(1.0, 0.0, 0.0) + cap = s2.S2Cap.from_point(center) + self.assertEqual(cap.center, center) + + def test_radius_type(self): + center = s2.S2Point(1.0, 0.0, 0.0) + chord = s2.S1ChordAngle(s2.S1Angle.from_radians(1.0)) + cap = s2.S2Cap(center, chord) + self.assertIsInstance(cap.radius, s2.S1ChordAngle) + + def test_height(self): + center = s2.S2Point(1.0, 0.0, 0.0) + cap = s2.S2Cap.from_center_height(center, 0.5) + self.assertAlmostEqual(cap.height, 0.5) + + def test_get_radius(self): + center = s2.S2Point(1.0, 0.0, 0.0) + angle = s2.S1Angle.from_radians(1.0) + cap = s2.S2Cap(center, angle) + self.assertIsInstance(cap.get_radius(), s2.S1Angle) + self.assertAlmostEqual(cap.get_radius().radians, 1.0, places=14) + + def test_get_area(self): + # Full cap has area 4*pi + self.assertAlmostEqual(s2.S2Cap.full().get_area(), 4 * math.pi) + # Empty cap has area 0 + self.assertAlmostEqual(s2.S2Cap.empty().get_area(), 0.0) + + def test_get_centroid(self): + center = s2.S2Point(1.0, 0.0, 0.0) + cap = s2.S2Cap.from_center_height(center, 0.5) + centroid = cap.get_centroid() + self.assertIsInstance(centroid, s2.S2Point) + # Centroid lies along the same axis as center + self.assertGreater(centroid.x, 0.0) + self.assertAlmostEqual(centroid.y, 0.0) + self.assertAlmostEqual(centroid.z, 0.0) + + # --- Predicates --- + + def test_is_valid(self): + self.assertTrue(s2.S2Cap.empty().is_valid()) + self.assertTrue(s2.S2Cap.full().is_valid()) + center = s2.S2Point(1.0, 0.0, 0.0) + self.assertTrue(s2.S2Cap(center, s2.S1Angle.from_radians(1.0)).is_valid()) + + def test_is_empty(self): + self.assertTrue(s2.S2Cap.empty().is_empty()) + self.assertFalse(s2.S2Cap.full().is_empty()) + + def test_is_full(self): + self.assertTrue(s2.S2Cap.full().is_full()) + self.assertFalse(s2.S2Cap.empty().is_full()) + + # --- Geometric operations --- + + def test_complement_of_empty_is_full(self): + self.assertTrue(s2.S2Cap.empty().complement().is_full()) + + def test_complement_of_full_is_empty(self): + self.assertTrue(s2.S2Cap.full().complement().is_empty()) + + def test_complement_roundtrip(self): + center = s2.S2Point(1.0, 0.0, 0.0) + cap = s2.S2Cap(center, s2.S1Angle.from_radians(1.0)) + comp = cap.complement() + self.assertFalse(comp.is_empty()) + self.assertFalse(comp.is_full()) + + def test_expanded(self): + center = s2.S2Point(1.0, 0.0, 0.0) + cap = s2.S2Cap.from_point(center) + delta = s2.S1Angle.from_radians(0.1) + expanded = cap.expanded(delta) + self.assertGreater(expanded.get_radius().radians, + cap.get_radius().radians) + + def test_expanded_empty_stays_empty(self): + self.assertTrue( + s2.S2Cap.empty().expanded(s2.S1Angle.from_radians(1.0)).is_empty()) + + def test_union_contains_both_centers(self): + p1 = s2.S2Point(1.0, 0.0, 0.0) + p2 = s2.S2Point(math.cos(0.1), math.sin(0.1), 0.0) + cap1 = s2.S2Cap(p1, s2.S1Angle.from_radians(0.01)) + cap2 = s2.S2Cap(p2, s2.S1Angle.from_radians(0.01)) + u = cap1.union(cap2) + # The centers of both input caps are strictly inside the union radius, + # so contains_point is reliable here. + self.assertTrue(u.contains_point(p1)) + self.assertTrue(u.contains_point(p2)) + + # --- Containment / intersection --- + + def test_full_contains_any_cap(self): + center = s2.S2Point(1.0, 0.0, 0.0) + cap = s2.S2Cap(center, s2.S1Angle.from_radians(1.0)) + self.assertTrue(s2.S2Cap.full().contains(cap)) + + def test_empty_contained_by_any_cap(self): + center = s2.S2Point(1.0, 0.0, 0.0) + cap = s2.S2Cap(center, s2.S1Angle.from_radians(1.0)) + self.assertTrue(cap.contains(s2.S2Cap.empty())) + + def test_contains_point(self): + center = s2.S2Point(1.0, 0.0, 0.0) + cap = s2.S2Cap(center, s2.S1Angle.from_radians(1.0)) + self.assertTrue(cap.contains_point(center)) + antipode = s2.S2Point(-1.0, 0.0, 0.0) + self.assertFalse(cap.contains_point(antipode)) + + def test_contains_cell(self): + cell = s2.S2Cell(s2.S2CellId(s2.S2Point(1.0, 0.0, 0.0))) + self.assertTrue(s2.S2Cap.full().contains_cell(cell)) + self.assertFalse(s2.S2Cap.empty().contains_cell(cell)) + + def test_intersects(self): + p = s2.S2Point(1.0, 0.0, 0.0) + cap = s2.S2Cap(p, s2.S1Angle.from_radians(1.0)) + self.assertTrue(cap.intersects(cap)) + self.assertFalse(cap.intersects(s2.S2Cap.empty())) + + def test_interior_intersects(self): + p = s2.S2Point(1.0, 0.0, 0.0) + cap = s2.S2Cap(p, s2.S1Angle.from_radians(1.0)) + self.assertTrue(cap.interior_intersects(cap)) + + def test_interior_contains_point(self): + center = s2.S2Point(1.0, 0.0, 0.0) + cap = s2.S2Cap(center, s2.S1Angle.from_radians(1.0)) + self.assertTrue(cap.interior_contains_point(center)) + + def test_may_intersect_cell(self): + center = s2.S2Point(1.0, 0.0, 0.0) + cell = s2.S2Cell(s2.S2CellId(center)) + self.assertTrue(s2.S2Cap.full().may_intersect(cell)) + + # --- Mutation --- + + def test_add_point(self): + cap = s2.S2Cap.empty() + p = s2.S2Point(1.0, 0.0, 0.0) + cap.add_point(p) + self.assertTrue(cap.contains_point(p)) + + def test_add_cap(self): + cap1 = s2.S2Cap.empty() + p = s2.S2Point(1.0, 0.0, 0.0) + cap2 = s2.S2Cap.from_point(p) + cap1.add_cap(cap2) + self.assertTrue(cap1.contains_point(p)) + + # --- S2Region interface --- + + def test_get_cap_bound(self): + center = s2.S2Point(1.0, 0.0, 0.0) + cap = s2.S2Cap(center, s2.S1Angle.from_radians(1.0)) + bound = cap.get_cap_bound() + self.assertIsInstance(bound, s2.S2Cap) + self.assertTrue(bound.approx_equals(cap)) + + def test_cell_union_bound(self): + cap = s2.S2Cap.full() + cell_ids = cap.cell_union_bound() + self.assertIsInstance(cell_ids, list) + self.assertGreater(len(cell_ids), 0) + for cid in cell_ids: + self.assertIsInstance(cid, s2.S2CellId) + + # --- Operators --- + + def test_equality(self): + center = s2.S2Point(1.0, 0.0, 0.0) + cap1 = s2.S2Cap(center, s2.S1Angle.from_radians(1.0)) + cap2 = s2.S2Cap(center, s2.S1Angle.from_radians(1.0)) + self.assertEqual(cap1, cap2) + + def test_inequality(self): + cap1 = s2.S2Cap.from_point(s2.S2Point(1.0, 0.0, 0.0)) + cap2 = s2.S2Cap.from_point(s2.S2Point(0.0, 1.0, 0.0)) + self.assertNotEqual(cap1, cap2) + + def test_approx_equals(self): + center = s2.S2Point(1.0, 0.0, 0.0) + cap = s2.S2Cap(center, s2.S1Angle.from_radians(1.0)) + self.assertTrue(cap.approx_equals(cap)) + + def test_approx_equals_with_max_error(self): + center = s2.S2Point(1.0, 0.0, 0.0) + cap = s2.S2Cap(center, s2.S1Angle.from_radians(1.0)) + self.assertTrue(cap.approx_equals(cap, s2.S1Angle.from_radians(1e-10))) + + # --- String representation --- + + def test_repr(self): + cap = s2.S2Cap.empty() + r = repr(cap) + self.assertIn("Center=", r) + self.assertIn("Radius=", r) + + def test_str(self): + cap = s2.S2Cap.empty() + self.assertIsInstance(str(cap), str) + + # --- S2Cell.get_cap_bound --- + + def test_s2cell_get_cap_bound(self): + cell = s2.S2Cell(s2.S2CellId.from_face(0)) + cap = cell.get_cap_bound() + self.assertIsInstance(cap, s2.S2Cap) + self.assertTrue(cap.is_valid()) + # The cap must contain the cell's center. + self.assertTrue(cap.contains_point(cell.center())) + + +if __name__ == "__main__": + unittest.main() diff --git a/src/python/s2cell_bindings.cc b/src/python/s2cell_bindings.cc index ea9cb0c7..d2f8b0d7 100644 --- a/src/python/s2cell_bindings.cc +++ b/src/python/s2cell_bindings.cc @@ -9,6 +9,7 @@ #include "absl/hash/hash.h" #include "absl/strings/str_cat.h" #include "s2/s1chord_angle.h" +#include "s2/s2cap.h" #include "s2/s2cell.h" #include "s2/s2cell_id.h" #include "s2/s2latlng.h" @@ -236,7 +237,12 @@ void bind_s2cell(py::module& m) { std::ostringstream oss; oss << cell.id(); return oss.str(); - }); + }) + + // S2Region bound methods (depends on S2Cap, bound before S2Cell) + .def("get_cap_bound", &S2Cell::GetCapBound, + "Return the smallest cap containing this cell."); + // TODO: bind get_rect_bound() once S2LatLngRect is bound. py::enum_(cls, "Boundary", py::arithmetic()) .value("BOTTOM_EDGE", S2Cell::kBottomEdge) @@ -244,9 +250,4 @@ void bind_s2cell(py::module& m) { .value("TOP_EDGE", S2Cell::kTopEdge) .value("LEFT_EDGE", S2Cell::kLeftEdge) .export_values(); - - // TODO: The following S2Cell methods are not yet bound because they depend - // on types that have not been bound yet: - // - get_cap_bound() -> S2Cap - // - get_rect_bound() -> S2LatLngRect } From b48c2c6daca48111ce4a67ca15f1d657ed333ca2 Mon Sep 17 00:00:00 2001 From: David Eustis Date: Sat, 27 Jun 2026 17:03:24 +0000 Subject: [PATCH 02/12] pybind: Fix S2Cap review findings - Fix __repr__ to prefix class name per README convention - Rename get_radius/get_area/get_centroid to radius_angle/area/centroid - Rename get_cap_bound to cap_bound (drop Get prefix per convention) - Add __hash__ so S2Cap can be used in sets and as dict keys - Add @abseil-cpp//absl/hash BUILD dep - Fix module.cc deps comment (s2cell is not a dep of s2cap) - Update tests for renamed methods - Add __hash__ test and approx_equals false-case test --- src/python/BUILD.bazel | 1 + src/python/module.cc | 2 +- src/python/s2cap_bindings.cc | 15 +++++++---- src/python/s2cap_test.py | 47 +++++++++++++++++++++++++++-------- src/python/s2cell_bindings.cc | 2 +- 5 files changed, 49 insertions(+), 18 deletions(-) diff --git a/src/python/BUILD.bazel b/src/python/BUILD.bazel index bf5a9083..301225c8 100644 --- a/src/python/BUILD.bazel +++ b/src/python/BUILD.bazel @@ -130,6 +130,7 @@ pybind_library( srcs = ["s2cap_bindings.cc"], deps = [ "//:s2", + "@abseil-cpp//absl/hash", ], ) diff --git a/src/python/module.cc b/src/python/module.cc index 20f7a048..e3258f84 100644 --- a/src/python/module.cc +++ b/src/python/module.cc @@ -45,7 +45,7 @@ PYBIND11_MODULE(s2geometry_bindings, m) { // Deps: s1angle, s2point, s2latlng, r2point, r2rect bind_s2cell_id(m); - // Deps: s1angle, s1chord_angle, s2cell, s2point + // Deps: s1angle, s1chord_angle, s2point bind_s2cap(m); // Deps: r2rect, s1chord_angle, s2cap, s2cell_id, s2latlng, s2point diff --git a/src/python/s2cap_bindings.cc b/src/python/s2cap_bindings.cc index 26320daf..37c43187 100644 --- a/src/python/s2cap_bindings.cc +++ b/src/python/s2cap_bindings.cc @@ -5,6 +5,8 @@ #include #include +#include "absl/hash/hash.h" + #include "s2/s1angle.h" #include "s2/s1chord_angle.h" #include "s2/s2cap.h" @@ -66,13 +68,13 @@ void bind_s2cap(py::module& m) { "The height of the cap (distance from center point to cutoff plane).") // Computed accessors - .def("get_radius", &S2Cap::GetRadius, + .def("radius_angle", &S2Cap::GetRadius, "Return the radius as an S1Angle.\n\n" "Requires a trigonometric operation; may differ slightly from the\n" "value passed to the S1Angle constructor.") - .def("get_area", &S2Cap::GetArea, + .def("area", &S2Cap::GetArea, "Return the surface area of the cap in steradians.") - .def("get_centroid", &S2Cap::GetCentroid, + .def("centroid", &S2Cap::GetCentroid, "Return the true centroid of the cap multiplied by its surface area.\n\n" "The result lies on the ray from the origin through the cap's center.\n" "For zero-radius caps, always returns the origin (0, 0, 0).") @@ -136,7 +138,7 @@ void bind_s2cap(py::module& m) { "If this cap is empty, sets it to other.") // S2Region interface - .def("get_cap_bound", &S2Cap::GetCapBound, + .def("cap_bound", &S2Cap::GetCapBound, "Return a bounding cap for this cap (returns self).") // get_rect_bound() is deferred until S2LatLngRect is bound. .def("cell_union_bound", [](const S2Cap& self) { @@ -155,11 +157,14 @@ void bind_s2cap(py::module& m) { "Return true if this cap is approximately equal to other.\n\n" "Checks that the angle between centers and the difference in chord\n" "radii are both within max_error radians.") + .def("__hash__", [](const S2Cap& self) { + return absl::HashOf(self.center(), self.radius().length2()); + }) // String representation .def("__repr__", [](const S2Cap& self) { std::ostringstream oss; - oss << self; + oss << "S2Cap(" << self << ")"; return oss.str(); }) .def("__str__", [](const S2Cap& self) { diff --git a/src/python/s2cap_test.py b/src/python/s2cap_test.py index 7a358349..0fb0f16e 100644 --- a/src/python/s2cap_test.py +++ b/src/python/s2cap_test.py @@ -65,7 +65,7 @@ def test_from_center_area(self): area = 2 * math.pi # hemisphere cap = s2.S2Cap.from_center_area(center, area) self.assertTrue(cap.is_valid()) - self.assertAlmostEqual(cap.get_area(), area, places=10) + self.assertAlmostEqual(cap.area(), area, places=10) def test_empty(self): cap = s2.S2Cap.empty() @@ -99,19 +99,19 @@ def test_get_radius(self): center = s2.S2Point(1.0, 0.0, 0.0) angle = s2.S1Angle.from_radians(1.0) cap = s2.S2Cap(center, angle) - self.assertIsInstance(cap.get_radius(), s2.S1Angle) - self.assertAlmostEqual(cap.get_radius().radians, 1.0, places=14) + self.assertIsInstance(cap.radius_angle(), s2.S1Angle) + self.assertAlmostEqual(cap.radius_angle().radians, 1.0, places=14) def test_get_area(self): # Full cap has area 4*pi - self.assertAlmostEqual(s2.S2Cap.full().get_area(), 4 * math.pi) + self.assertAlmostEqual(s2.S2Cap.full().area(), 4 * math.pi) # Empty cap has area 0 - self.assertAlmostEqual(s2.S2Cap.empty().get_area(), 0.0) + self.assertAlmostEqual(s2.S2Cap.empty().area(), 0.0) def test_get_centroid(self): center = s2.S2Point(1.0, 0.0, 0.0) cap = s2.S2Cap.from_center_height(center, 0.5) - centroid = cap.get_centroid() + centroid = cap.centroid() self.assertIsInstance(centroid, s2.S2Point) # Centroid lies along the same axis as center self.assertGreater(centroid.x, 0.0) @@ -154,8 +154,8 @@ def test_expanded(self): cap = s2.S2Cap.from_point(center) delta = s2.S1Angle.from_radians(0.1) expanded = cap.expanded(delta) - self.assertGreater(expanded.get_radius().radians, - cap.get_radius().radians) + self.assertGreater(expanded.radius_angle().radians, + cap.radius_angle().radians) def test_expanded_empty_stays_empty(self): self.assertTrue( @@ -237,7 +237,7 @@ def test_add_cap(self): def test_get_cap_bound(self): center = s2.S2Point(1.0, 0.0, 0.0) cap = s2.S2Cap(center, s2.S1Angle.from_radians(1.0)) - bound = cap.get_cap_bound() + bound = cap.cap_bound() self.assertIsInstance(bound, s2.S2Cap) self.assertTrue(bound.approx_equals(cap)) @@ -277,6 +277,7 @@ def test_approx_equals_with_max_error(self): def test_repr(self): cap = s2.S2Cap.empty() r = repr(cap) + self.assertTrue(r.startswith("S2Cap(")) self.assertIn("Center=", r) self.assertIn("Radius=", r) @@ -284,16 +285,40 @@ def test_str(self): cap = s2.S2Cap.empty() self.assertIsInstance(str(cap), str) - # --- S2Cell.get_cap_bound --- + # --- S2Cell.cap_bound --- def test_s2cell_get_cap_bound(self): cell = s2.S2Cell(s2.S2CellId.from_face(0)) - cap = cell.get_cap_bound() + cap = cell.cap_bound() self.assertIsInstance(cap, s2.S2Cap) self.assertTrue(cap.is_valid()) # The cap must contain the cell's center. self.assertTrue(cap.contains_point(cell.center())) + # --- __hash__ --- + + def test_hash(self): + center = s2.S2Point(1.0, 0.0, 0.0) + cap = s2.S2Cap(center, s2.S1Angle.from_radians(1.0)) + # S2Cap must be hashable. + h = hash(cap) + self.assertIsInstance(h, int) + # Can be used as a dict key. + d = {cap: "value"} + self.assertEqual(d[cap], "value") + # Can be used in a set. + s = {cap} + self.assertIn(cap, s) + + # --- approx_equals false case --- + + def test_approx_equals_false(self): + cap1 = s2.S2Cap(s2.S2Point(1.0, 0.0, 0.0), + s2.S1Angle.from_radians(1.0)) + cap2 = s2.S2Cap(s2.S2Point(0.0, 1.0, 0.0), + s2.S1Angle.from_radians(1.0)) + self.assertFalse(cap1.approx_equals(cap2)) + if __name__ == "__main__": unittest.main() diff --git a/src/python/s2cell_bindings.cc b/src/python/s2cell_bindings.cc index d2f8b0d7..020f346c 100644 --- a/src/python/s2cell_bindings.cc +++ b/src/python/s2cell_bindings.cc @@ -240,7 +240,7 @@ void bind_s2cell(py::module& m) { }) // S2Region bound methods (depends on S2Cap, bound before S2Cell) - .def("get_cap_bound", &S2Cell::GetCapBound, + .def("cap_bound", &S2Cell::GetCapBound, "Return the smallest cap containing this cell."); // TODO: bind get_rect_bound() once S2LatLngRect is bound. From ac076e6ed6da034190fa60d505ea39ec7a5b24bd Mon Sep 17 00:00:00 2001 From: David Eustis Date: Sat, 27 Jun 2026 17:19:29 +0000 Subject: [PATCH 03/12] pybind: Replace S2Cap add_point/add_cap with from_points/from_caps Mutating methods are inconsistent with the otherwise-immutable API. from_points and from_caps provide the same bounding-region-from-collection pattern without mutating the receiver. --- src/python/s2cap_bindings.cc | 25 +++++++++++------- src/python/s2cap_test.py | 50 +++++++++++++++++++++++++----------- 2 files changed, 51 insertions(+), 24 deletions(-) diff --git a/src/python/s2cap_bindings.cc b/src/python/s2cap_bindings.cc index 37c43187..6794289f 100644 --- a/src/python/s2cap_bindings.cc +++ b/src/python/s2cap_bindings.cc @@ -58,6 +58,22 @@ void bind_s2cap(py::module& m) { "Return an empty cap (contains no points).") .def_static("full", &S2Cap::Full, "Return a full cap (contains all points).") + .def_static("from_points", [](const std::vector& points) { + if (points.empty()) return S2Cap::Empty(); + S2Cap result = S2Cap::FromPoint(points[0]); + for (size_t i = 1; i < points.size(); ++i) result.AddPoint(points[i]); + return result; + }, py::arg("points"), + "Return the smallest cap containing all given points.\n\n" + "Returns the empty cap if the list is empty.") + .def_static("from_caps", [](const std::vector& caps) { + if (caps.empty()) return S2Cap::Empty(); + S2Cap result = caps[0]; + for (size_t i = 1; i < caps.size(); ++i) result.AddCap(caps[i]); + return result; + }, py::arg("caps"), + "Return the smallest cap containing all given caps.\n\n" + "Returns the empty cap if the list is empty.") // Properties .def_property_readonly("center", &S2Cap::center, @@ -128,15 +144,6 @@ void bind_s2cap(py::module& m) { .def("may_intersect", &S2Cap::MayIntersect, py::arg("cell"), "Return true if this cap may intersect the given cell.") - // Mutation - .def("add_point", &S2Cap::AddPoint, py::arg("point"), - "Expand the cap to include point if necessary.\n\n" - "If empty, sets center to point. Otherwise center is unchanged.\n" - "point should be unit length.") - .def("add_cap", &S2Cap::AddCap, py::arg("other"), - "Expand the cap to include other if necessary.\n\n" - "If this cap is empty, sets it to other.") - // S2Region interface .def("cap_bound", &S2Cap::GetCapBound, "Return a bounding cap for this cap (returns self).") diff --git a/src/python/s2cap_test.py b/src/python/s2cap_test.py index 0fb0f16e..e33d3e22 100644 --- a/src/python/s2cap_test.py +++ b/src/python/s2cap_test.py @@ -77,6 +77,41 @@ def test_full(self): self.assertTrue(cap.is_full()) self.assertFalse(cap.is_empty()) + def test_from_points_empty(self): + cap = s2.S2Cap.from_points([]) + self.assertTrue(cap.is_empty()) + + def test_from_points_single(self): + p = s2.S2Point(1.0, 0.0, 0.0) + cap = s2.S2Cap.from_points([p]) + self.assertTrue(cap.contains_point(p)) + self.assertTrue(cap.is_empty() or cap.height >= 0) + + def test_from_points_multiple(self): + p1 = s2.S2Point(1.0, 0.0, 0.0) + p2 = s2.S2Point(0.0, 1.0, 0.0) + p3 = s2.S2Point(0.0, 0.0, 1.0) + cap = s2.S2Cap.from_points([p1, p2, p3]) + self.assertTrue(cap.contains_point(p1)) + self.assertTrue(cap.contains_point(p2)) + self.assertTrue(cap.contains_point(p3)) + + def test_from_caps_empty(self): + cap = s2.S2Cap.from_caps([]) + self.assertTrue(cap.is_empty()) + + def test_from_caps_single(self): + c = s2.S2Cap(s2.S2Point(1.0, 0.0, 0.0), s2.S1Angle.from_degrees(10.0)) + result = s2.S2Cap.from_caps([c]) + self.assertTrue(result.contains_point(c.center)) + + def test_from_caps_multiple(self): + c1 = s2.S2Cap(s2.S2Point(1.0, 0.0, 0.0), s2.S1Angle.from_degrees(10.0)) + c2 = s2.S2Cap(s2.S2Point(0.0, 1.0, 0.0), s2.S1Angle.from_degrees(10.0)) + result = s2.S2Cap.from_caps([c1, c2]) + self.assertTrue(result.contains_point(c1.center)) + self.assertTrue(result.contains_point(c2.center)) + # --- Properties --- def test_center(self): @@ -217,21 +252,6 @@ def test_may_intersect_cell(self): cell = s2.S2Cell(s2.S2CellId(center)) self.assertTrue(s2.S2Cap.full().may_intersect(cell)) - # --- Mutation --- - - def test_add_point(self): - cap = s2.S2Cap.empty() - p = s2.S2Point(1.0, 0.0, 0.0) - cap.add_point(p) - self.assertTrue(cap.contains_point(p)) - - def test_add_cap(self): - cap1 = s2.S2Cap.empty() - p = s2.S2Point(1.0, 0.0, 0.0) - cap2 = s2.S2Cap.from_point(p) - cap1.add_cap(cap2) - self.assertTrue(cap1.contains_point(p)) - # --- S2Region interface --- def test_get_cap_bound(self): From d48934e030d67077e3482830b38f5e50269239be Mon Sep 17 00:00:00 2001 From: David Eustis Date: Sat, 27 Jun 2026 18:03:01 +0000 Subject: [PATCH 04/12] pybind: Move bind_s2cap before bind_s2latlng/s2cell_id S2Cap only depends on s1angle, s1chord_angle, and s2point, all of which are already bound at that point. Order bindings by fewest dependencies. --- src/python/module.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/python/module.cc b/src/python/module.cc index e3258f84..10d53d88 100644 --- a/src/python/module.cc +++ b/src/python/module.cc @@ -39,15 +39,15 @@ PYBIND11_MODULE(s2geometry_bindings, m) { // Deps: s1angle, s2point bind_s1chord_angle(m); + // Deps: s1angle, s1chord_angle, s2point + bind_s2cap(m); + // Deps: s1angle, s2point bind_s2latlng(m); // Deps: s1angle, s2point, s2latlng, r2point, r2rect bind_s2cell_id(m); - // Deps: s1angle, s1chord_angle, s2point - bind_s2cap(m); - // Deps: r2rect, s1chord_angle, s2cap, s2cell_id, s2latlng, s2point bind_s2cell(m); } From 386993e321cc999f01e2e6d8c7ecb36cfd2be358 Mon Sep 17 00:00:00 2001 From: David Eustis Date: Sat, 27 Jun 2026 18:10:00 +0000 Subject: [PATCH 05/12] pybind: Validate S2Cap center at construction, remove is_valid Invalid caps are now impossible to construct from Python: both S2Point constructors raise ValueError for non-unit-length centers. is_valid() is removed as it would always return True. --- src/python/s2cap_bindings.cc | 28 +++++++++++++++++----------- src/python/s2cap_test.py | 17 +++++------------ 2 files changed, 22 insertions(+), 23 deletions(-) diff --git a/src/python/s2cap_bindings.cc b/src/python/s2cap_bindings.cc index 6794289f..d33831d7 100644 --- a/src/python/s2cap_bindings.cc +++ b/src/python/s2cap_bindings.cc @@ -27,16 +27,25 @@ void bind_s2cap(py::module& m) { // Constructors .def(py::init<>(), "Construct an empty S2Cap.") - .def(py::init(), + .def(py::init([](const S2Point& center, S1Angle radius) { + if (!S2::IsUnitLength(center)) { + throw py::value_error("center must be a unit-length S2Point"); + } + return S2Cap(center, radius); + }), py::arg("center"), py::arg("radius"), - "Construct a cap with the given center and S1Angle radius.\n\n" - "A negative radius yields an empty cap; radius >= Pi yields a full\n" - "cap. center should be unit length.") - .def(py::init(), + "Construct a cap with the given center and radius.\n\n" + "center must be unit length. Negative radius produces the empty cap;\n" + "radius >= 180 degrees produces the full cap.") + .def(py::init([](const S2Point& center, S1ChordAngle radius) { + if (!S2::IsUnitLength(center)) { + throw py::value_error("center must be a unit-length S2Point"); + } + return S2Cap(center, radius); + }), py::arg("center"), py::arg("radius"), - "Construct a cap with the given center and S1ChordAngle radius.\n\n" - "More efficient than the S1Angle constructor.\n" - "center should be unit length.") + "Construct a cap with the given center and radius as S1ChordAngle.\n\n" + "center must be unit length.") // Static factories .def_static("from_point", &S2Cap::FromPoint, py::arg("center"), @@ -96,9 +105,6 @@ void bind_s2cap(py::module& m) { "For zero-radius caps, always returns the origin (0, 0, 0).") // Predicates - .def("is_valid", &S2Cap::is_valid, - "Return true if the cap is valid (center is unit-length, " - "radius in [-1, 4]).") .def("is_empty", &S2Cap::is_empty, "Return true if the cap contains no points.") .def("is_full", &S2Cap::is_full, diff --git a/src/python/s2cap_test.py b/src/python/s2cap_test.py index e33d3e22..09afe8b2 100644 --- a/src/python/s2cap_test.py +++ b/src/python/s2cap_test.py @@ -18,7 +18,6 @@ def test_constructor_s1angle(self): center = s2.S2Point(1.0, 0.0, 0.0) radius = s2.S1Angle.from_radians(math.pi / 4) cap = s2.S2Cap(center, radius) - self.assertTrue(cap.is_valid()) self.assertFalse(cap.is_empty()) self.assertFalse(cap.is_full()) @@ -26,7 +25,11 @@ def test_constructor_s1chord_angle(self): center = s2.S2Point(1.0, 0.0, 0.0) radius = s2.S1ChordAngle(s2.S1Angle.from_radians(math.pi / 4)) cap = s2.S2Cap(center, radius) - self.assertTrue(cap.is_valid()) + self.assertFalse(cap.is_empty()) + + def test_constructor_rejects_non_unit_center(self): + with self.assertRaises(ValueError): + s2.S2Cap(s2.S2Point(1.0, 1.0, 1.0), s2.S1Angle.from_degrees(10.0)) def test_constructor_negative_radius_is_empty(self): center = s2.S2Point(1.0, 0.0, 0.0) @@ -44,12 +47,10 @@ def test_from_point(self): p = s2.S2Point(0.0, 1.0, 0.0) cap = s2.S2Cap.from_point(p) self.assertTrue(cap.contains_point(p)) - self.assertTrue(cap.is_valid()) def test_from_center_height(self): center = s2.S2Point(1.0, 0.0, 0.0) cap = s2.S2Cap.from_center_height(center, 0.5) - self.assertTrue(cap.is_valid()) self.assertAlmostEqual(cap.height, 0.5) def test_from_center_height_negative_is_empty(self): @@ -64,7 +65,6 @@ def test_from_center_area(self): center = s2.S2Point(1.0, 0.0, 0.0) area = 2 * math.pi # hemisphere cap = s2.S2Cap.from_center_area(center, area) - self.assertTrue(cap.is_valid()) self.assertAlmostEqual(cap.area(), area, places=10) def test_empty(self): @@ -155,12 +155,6 @@ def test_get_centroid(self): # --- Predicates --- - def test_is_valid(self): - self.assertTrue(s2.S2Cap.empty().is_valid()) - self.assertTrue(s2.S2Cap.full().is_valid()) - center = s2.S2Point(1.0, 0.0, 0.0) - self.assertTrue(s2.S2Cap(center, s2.S1Angle.from_radians(1.0)).is_valid()) - def test_is_empty(self): self.assertTrue(s2.S2Cap.empty().is_empty()) self.assertFalse(s2.S2Cap.full().is_empty()) @@ -311,7 +305,6 @@ def test_s2cell_get_cap_bound(self): cell = s2.S2Cell(s2.S2CellId.from_face(0)) cap = cell.cap_bound() self.assertIsInstance(cap, s2.S2Cap) - self.assertTrue(cap.is_valid()) # The cap must contain the cell's center. self.assertTrue(cap.contains_point(cell.center())) From 67e0b128e23bc0690a60effc1c53b6591780a1fa Mon Sep 17 00:00:00 2001 From: David Eustis Date: Sat, 27 Jun 2026 18:54:03 +0000 Subject: [PATCH 06/12] pybind: Use README section names in s2cap_bindings.cc Static factories -> Factory methods; fold Computed accessors and S2Region interface labels into Geometric operations. --- src/python/s2cap_bindings.cc | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/python/s2cap_bindings.cc b/src/python/s2cap_bindings.cc index d33831d7..a34021b4 100644 --- a/src/python/s2cap_bindings.cc +++ b/src/python/s2cap_bindings.cc @@ -47,7 +47,7 @@ void bind_s2cap(py::module& m) { "Construct a cap with the given center and radius as S1ChordAngle.\n\n" "center must be unit length.") - // Static factories + // Factory methods .def_static("from_point", &S2Cap::FromPoint, py::arg("center"), "Return a cap containing a single point.\n\n" "More efficient than S2Cap(center, S1ChordAngle::Zero()).") @@ -92,7 +92,7 @@ void bind_s2cap(py::module& m) { .def_property_readonly("height", &S2Cap::height, "The height of the cap (distance from center point to cutoff plane).") - // Computed accessors + // Geometric operations .def("radius_angle", &S2Cap::GetRadius, "Return the radius as an S1Angle.\n\n" "Requires a trigonometric operation; may differ slightly from the\n" @@ -110,7 +110,6 @@ void bind_s2cap(py::module& m) { .def("is_full", &S2Cap::is_full, "Return true if the cap contains all points.") - // Geometric operations .def("complement", &S2Cap::Complement, "Return the complement of the interior of the cap.\n\n" "Same boundary as this cap but no shared interior points.\n" @@ -150,7 +149,6 @@ void bind_s2cap(py::module& m) { .def("may_intersect", &S2Cap::MayIntersect, py::arg("cell"), "Return true if this cap may intersect the given cell.") - // S2Region interface .def("cap_bound", &S2Cap::GetCapBound, "Return a bounding cap for this cap (returns self).") // get_rect_bound() is deferred until S2LatLngRect is bound. From d015b5c468c12941fcd8dc51ec2ac85c30a18fdd Mon Sep 17 00:00:00 2001 From: David Eustis Date: Sat, 27 Jun 2026 19:30:38 +0000 Subject: [PATCH 07/12] pybind: Normalize S2Cap center rather than raising ValueError Silently normalizing is more ergonomic: callers working with approximate unit vectors don't need to pre-normalize, and it matches the behavior of S2Cap::FromPoint and other factories. --- src/python/s2cap_bindings.cc | 16 +++++----------- src/python/s2cap_test.py | 9 +++++---- 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/src/python/s2cap_bindings.cc b/src/python/s2cap_bindings.cc index a34021b4..89704c50 100644 --- a/src/python/s2cap_bindings.cc +++ b/src/python/s2cap_bindings.cc @@ -28,24 +28,18 @@ void bind_s2cap(py::module& m) { .def(py::init<>(), "Construct an empty S2Cap.") .def(py::init([](const S2Point& center, S1Angle radius) { - if (!S2::IsUnitLength(center)) { - throw py::value_error("center must be a unit-length S2Point"); - } - return S2Cap(center, radius); + return S2Cap(center.Normalize(), radius); }), py::arg("center"), py::arg("radius"), "Construct a cap with the given center and radius.\n\n" - "center must be unit length. Negative radius produces the empty cap;\n" - "radius >= 180 degrees produces the full cap.") + "center is normalized if not already unit length. Negative radius\n" + "produces the empty cap; radius >= 180 degrees produces the full cap.") .def(py::init([](const S2Point& center, S1ChordAngle radius) { - if (!S2::IsUnitLength(center)) { - throw py::value_error("center must be a unit-length S2Point"); - } - return S2Cap(center, radius); + return S2Cap(center.Normalize(), radius); }), py::arg("center"), py::arg("radius"), "Construct a cap with the given center and radius as S1ChordAngle.\n\n" - "center must be unit length.") + "center is normalized if not already unit length.") // Factory methods .def_static("from_point", &S2Cap::FromPoint, py::arg("center"), diff --git a/src/python/s2cap_test.py b/src/python/s2cap_test.py index 09afe8b2..e6b99cab 100644 --- a/src/python/s2cap_test.py +++ b/src/python/s2cap_test.py @@ -27,10 +27,6 @@ def test_constructor_s1chord_angle(self): cap = s2.S2Cap(center, radius) self.assertFalse(cap.is_empty()) - def test_constructor_rejects_non_unit_center(self): - with self.assertRaises(ValueError): - s2.S2Cap(s2.S2Point(1.0, 1.0, 1.0), s2.S1Angle.from_degrees(10.0)) - def test_constructor_negative_radius_is_empty(self): center = s2.S2Point(1.0, 0.0, 0.0) cap = s2.S2Cap(center, s2.S1Angle.from_radians(-1.0)) @@ -41,6 +37,11 @@ def test_constructor_pi_radius_is_full(self): cap = s2.S2Cap(center, s2.S1Angle.from_radians(math.pi)) self.assertTrue(cap.is_full()) + def test_constructor_normalizes_center(self): + p = s2.S2Point(2.0, 0.0, 0.0) # not unit length + cap = s2.S2Cap(p, s2.S1Angle.from_degrees(10.0)) + self.assertAlmostEqual(cap.center.norm(), 1.0, places=15) + # --- Static factories --- def test_from_point(self): From d6b537bef167834950dcad9c69b8989fd0812277 Mon Sep 17 00:00:00 2001 From: David Eustis Date: Sat, 27 Jun 2026 19:36:49 +0000 Subject: [PATCH 08/12] pybind: Remove cap_bound from S2Cell (will land in S2Region PR) --- src/python/s2cell_bindings.cc | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/python/s2cell_bindings.cc b/src/python/s2cell_bindings.cc index 020f346c..d3321bec 100644 --- a/src/python/s2cell_bindings.cc +++ b/src/python/s2cell_bindings.cc @@ -9,7 +9,6 @@ #include "absl/hash/hash.h" #include "absl/strings/str_cat.h" #include "s2/s1chord_angle.h" -#include "s2/s2cap.h" #include "s2/s2cell.h" #include "s2/s2cell_id.h" #include "s2/s2latlng.h" @@ -237,12 +236,7 @@ void bind_s2cell(py::module& m) { std::ostringstream oss; oss << cell.id(); return oss.str(); - }) - - // S2Region bound methods (depends on S2Cap, bound before S2Cell) - .def("cap_bound", &S2Cell::GetCapBound, - "Return the smallest cap containing this cell."); - // TODO: bind get_rect_bound() once S2LatLngRect is bound. + }); py::enum_(cls, "Boundary", py::arithmetic()) .value("BOTTOM_EDGE", S2Cell::kBottomEdge) From b75505cc84bdf4688a1f9403868a83b538b07514 Mon Sep 17 00:00:00 2001 From: David Eustis Date: Sat, 27 Jun 2026 19:50:41 +0000 Subject: [PATCH 09/12] pybind: Normalize S2Cap factory method centers from_point, from_center_height, and from_center_area now normalize the center point consistent with the constructors, so callers don't need to pre-normalize. --- src/python/s2cap_bindings.cc | 24 ++++++++++++++---------- src/python/s2cap_test.py | 7 ------- 2 files changed, 14 insertions(+), 17 deletions(-) diff --git a/src/python/s2cap_bindings.cc b/src/python/s2cap_bindings.cc index 89704c50..d4c44850 100644 --- a/src/python/s2cap_bindings.cc +++ b/src/python/s2cap_bindings.cc @@ -42,21 +42,25 @@ void bind_s2cap(py::module& m) { "center is normalized if not already unit length.") // Factory methods - .def_static("from_point", &S2Cap::FromPoint, py::arg("center"), + .def_static("from_point", [](const S2Point& center) { + return S2Cap::FromPoint(center.Normalize()); + }, py::arg("center"), "Return a cap containing a single point.\n\n" - "More efficient than S2Cap(center, S1ChordAngle::Zero()).") - .def_static("from_center_height", &S2Cap::FromCenterHeight, - py::arg("center"), py::arg("height"), + "center is normalized if not already unit length.") + .def_static("from_center_height", [](const S2Point& center, double height) { + return S2Cap::FromCenterHeight(center.Normalize(), height); + }, py::arg("center"), py::arg("height"), "Return a cap with the given center and height.\n\n" "Height is the distance from the center point to the cutoff plane.\n" - "A negative height yields an empty cap; height >= 2 yields a full\n" - "cap. center should be unit length.") - .def_static("from_center_area", &S2Cap::FromCenterArea, - py::arg("center"), py::arg("area"), + "center is normalized if not already unit length.\n" + "A negative height yields an empty cap; height >= 2 yields a full cap.") + .def_static("from_center_area", [](const S2Point& center, double area) { + return S2Cap::FromCenterArea(center.Normalize(), area); + }, py::arg("center"), py::arg("area"), "Return a cap with the given center and surface area in steradians.\n\n" "The area also equals the solid angle subtended by the cap.\n" - "A negative area yields an empty cap; area >= 4*Pi yields a full cap.\n" - "center should be unit length.") + "center is normalized if not already unit length.\n" + "A negative area yields an empty cap; area >= 4*Pi yields a full cap.") .def_static("empty", &S2Cap::Empty, "Return an empty cap (contains no points).") .def_static("full", &S2Cap::Full, diff --git a/src/python/s2cap_test.py b/src/python/s2cap_test.py index e6b99cab..91a739f3 100644 --- a/src/python/s2cap_test.py +++ b/src/python/s2cap_test.py @@ -302,13 +302,6 @@ def test_str(self): # --- S2Cell.cap_bound --- - def test_s2cell_get_cap_bound(self): - cell = s2.S2Cell(s2.S2CellId.from_face(0)) - cap = cell.cap_bound() - self.assertIsInstance(cap, s2.S2Cap) - # The cap must contain the cell's center. - self.assertTrue(cap.contains_point(cell.center())) - # --- __hash__ --- def test_hash(self): From 8ec51330a030a214b9731881ba41cd89d39ce82a Mon Sep 17 00:00:00 2001 From: David Eustis Date: Sat, 27 Jun 2026 20:20:30 +0000 Subject: [PATCH 10/12] pybind: Restore s2cell TODO comment (cap_bound/rect_bound deferred to S2Region PR) --- src/python/s2cell_bindings.cc | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/python/s2cell_bindings.cc b/src/python/s2cell_bindings.cc index d3321bec..523a7a73 100644 --- a/src/python/s2cell_bindings.cc +++ b/src/python/s2cell_bindings.cc @@ -244,4 +244,9 @@ void bind_s2cell(py::module& m) { .value("TOP_EDGE", S2Cell::kTopEdge) .value("LEFT_EDGE", S2Cell::kLeftEdge) .export_values(); + + // TODO: The following S2Cell methods are not yet bound because they depend + // on types that have not been bound yet: + // - cap_bound() -> S2Cap + // - rect_bound() -> S2LatLngRect } From ebb701259fe4c1cde8e95d382a148ad8afefd8b6 Mon Sep 17 00:00:00 2001 From: David Eustis Date: Sat, 27 Jun 2026 20:29:35 +0000 Subject: [PATCH 11/12] pybind: Revert s2cell_bindings.cc to master No s2cell changes belong in the s2cap PR; they will land with s2region. --- src/python/s2cell_bindings.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/python/s2cell_bindings.cc b/src/python/s2cell_bindings.cc index 523a7a73..ea9cb0c7 100644 --- a/src/python/s2cell_bindings.cc +++ b/src/python/s2cell_bindings.cc @@ -247,6 +247,6 @@ void bind_s2cell(py::module& m) { // TODO: The following S2Cell methods are not yet bound because they depend // on types that have not been bound yet: - // - cap_bound() -> S2Cap - // - rect_bound() -> S2LatLngRect + // - get_cap_bound() -> S2Cap + // - get_rect_bound() -> S2LatLngRect } From 04a980a75eb440186301a42c892670c1da610efa Mon Sep 17 00:00:00 2001 From: David Eustis Date: Sat, 27 Jun 2026 20:40:33 +0000 Subject: [PATCH 12/12] pybind: Use README section headings in s2cap_test.py - Plain section headers matching the bindings (no dashes) - Add class docstring - Drop assertIsInstance; use value assertions throughout - Consolidate orphaned sub-sections into Geometric operations / Operators --- src/python/s2cap_test.py | 146 +++++++++++++++------------------------ 1 file changed, 57 insertions(+), 89 deletions(-) diff --git a/src/python/s2cap_test.py b/src/python/s2cap_test.py index 91a739f3..fc962c55 100644 --- a/src/python/s2cap_test.py +++ b/src/python/s2cap_test.py @@ -6,8 +6,9 @@ class TestS2Cap(unittest.TestCase): + """Test cases for S2Cap bindings.""" - # --- Constructors --- + # Constructors def test_default_constructor_is_empty(self): cap = s2.S2Cap() @@ -42,7 +43,7 @@ def test_constructor_normalizes_center(self): cap = s2.S2Cap(p, s2.S1Angle.from_degrees(10.0)) self.assertAlmostEqual(cap.center.norm(), 1.0, places=15) - # --- Static factories --- + # Factory methods def test_from_point(self): p = s2.S2Point(0.0, 1.0, 0.0) @@ -86,7 +87,6 @@ def test_from_points_single(self): p = s2.S2Point(1.0, 0.0, 0.0) cap = s2.S2Cap.from_points([p]) self.assertTrue(cap.contains_point(p)) - self.assertTrue(cap.is_empty() or cap.height >= 0) def test_from_points_multiple(self): p1 = s2.S2Point(1.0, 0.0, 0.0) @@ -113,59 +113,54 @@ def test_from_caps_multiple(self): self.assertTrue(result.contains_point(c1.center)) self.assertTrue(result.contains_point(c2.center)) - # --- Properties --- + # Properties def test_center(self): center = s2.S2Point(1.0, 0.0, 0.0) cap = s2.S2Cap.from_point(center) self.assertEqual(cap.center, center) - def test_radius_type(self): + def test_radius(self): center = s2.S2Point(1.0, 0.0, 0.0) chord = s2.S1ChordAngle(s2.S1Angle.from_radians(1.0)) cap = s2.S2Cap(center, chord) - self.assertIsInstance(cap.radius, s2.S1ChordAngle) + self.assertEqual(cap.radius, chord) def test_height(self): center = s2.S2Point(1.0, 0.0, 0.0) cap = s2.S2Cap.from_center_height(center, 0.5) self.assertAlmostEqual(cap.height, 0.5) - def test_get_radius(self): + # Predicates + + def test_is_empty(self): + self.assertTrue(s2.S2Cap.empty().is_empty()) + self.assertFalse(s2.S2Cap.full().is_empty()) + + def test_is_full(self): + self.assertTrue(s2.S2Cap.full().is_full()) + self.assertFalse(s2.S2Cap.empty().is_full()) + + # Geometric operations + + def test_radius_angle(self): center = s2.S2Point(1.0, 0.0, 0.0) - angle = s2.S1Angle.from_radians(1.0) - cap = s2.S2Cap(center, angle) - self.assertIsInstance(cap.radius_angle(), s2.S1Angle) + cap = s2.S2Cap(center, s2.S1Angle.from_radians(1.0)) self.assertAlmostEqual(cap.radius_angle().radians, 1.0, places=14) - def test_get_area(self): - # Full cap has area 4*pi + def test_area(self): self.assertAlmostEqual(s2.S2Cap.full().area(), 4 * math.pi) - # Empty cap has area 0 self.assertAlmostEqual(s2.S2Cap.empty().area(), 0.0) - def test_get_centroid(self): + def test_centroid(self): center = s2.S2Point(1.0, 0.0, 0.0) cap = s2.S2Cap.from_center_height(center, 0.5) centroid = cap.centroid() - self.assertIsInstance(centroid, s2.S2Point) - # Centroid lies along the same axis as center + # Centroid lies along the same axis as center. self.assertGreater(centroid.x, 0.0) self.assertAlmostEqual(centroid.y, 0.0) self.assertAlmostEqual(centroid.z, 0.0) - # --- Predicates --- - - def test_is_empty(self): - self.assertTrue(s2.S2Cap.empty().is_empty()) - self.assertFalse(s2.S2Cap.full().is_empty()) - - def test_is_full(self): - self.assertTrue(s2.S2Cap.full().is_full()) - self.assertFalse(s2.S2Cap.empty().is_full()) - - # --- Geometric operations --- - def test_complement_of_empty_is_full(self): self.assertTrue(s2.S2Cap.empty().complement().is_full()) @@ -197,29 +192,23 @@ def test_union_contains_both_centers(self): cap1 = s2.S2Cap(p1, s2.S1Angle.from_radians(0.01)) cap2 = s2.S2Cap(p2, s2.S1Angle.from_radians(0.01)) u = cap1.union(cap2) - # The centers of both input caps are strictly inside the union radius, - # so contains_point is reliable here. + # Centers lie strictly inside the union radius, so contains_point is reliable. self.assertTrue(u.contains_point(p1)) self.assertTrue(u.contains_point(p2)) - # --- Containment / intersection --- - def test_full_contains_any_cap(self): - center = s2.S2Point(1.0, 0.0, 0.0) - cap = s2.S2Cap(center, s2.S1Angle.from_radians(1.0)) + cap = s2.S2Cap(s2.S2Point(1.0, 0.0, 0.0), s2.S1Angle.from_radians(1.0)) self.assertTrue(s2.S2Cap.full().contains(cap)) def test_empty_contained_by_any_cap(self): - center = s2.S2Point(1.0, 0.0, 0.0) - cap = s2.S2Cap(center, s2.S1Angle.from_radians(1.0)) + cap = s2.S2Cap(s2.S2Point(1.0, 0.0, 0.0), s2.S1Angle.from_radians(1.0)) self.assertTrue(cap.contains(s2.S2Cap.empty())) def test_contains_point(self): center = s2.S2Point(1.0, 0.0, 0.0) cap = s2.S2Cap(center, s2.S1Angle.from_radians(1.0)) self.assertTrue(cap.contains_point(center)) - antipode = s2.S2Point(-1.0, 0.0, 0.0) - self.assertFalse(cap.contains_point(antipode)) + self.assertFalse(cap.contains_point(s2.S2Point(-1.0, 0.0, 0.0))) def test_contains_cell(self): cell = s2.S2Cell(s2.S2CellId(s2.S2Point(1.0, 0.0, 0.0))) @@ -227,14 +216,12 @@ def test_contains_cell(self): self.assertFalse(s2.S2Cap.empty().contains_cell(cell)) def test_intersects(self): - p = s2.S2Point(1.0, 0.0, 0.0) - cap = s2.S2Cap(p, s2.S1Angle.from_radians(1.0)) + cap = s2.S2Cap(s2.S2Point(1.0, 0.0, 0.0), s2.S1Angle.from_radians(1.0)) self.assertTrue(cap.intersects(cap)) self.assertFalse(cap.intersects(s2.S2Cap.empty())) def test_interior_intersects(self): - p = s2.S2Point(1.0, 0.0, 0.0) - cap = s2.S2Cap(p, s2.S1Angle.from_radians(1.0)) + cap = s2.S2Cap(s2.S2Point(1.0, 0.0, 0.0), s2.S1Angle.from_radians(1.0)) self.assertTrue(cap.interior_intersects(cap)) def test_interior_contains_point(self): @@ -242,52 +229,57 @@ def test_interior_contains_point(self): cap = s2.S2Cap(center, s2.S1Angle.from_radians(1.0)) self.assertTrue(cap.interior_contains_point(center)) - def test_may_intersect_cell(self): - center = s2.S2Point(1.0, 0.0, 0.0) - cell = s2.S2Cell(s2.S2CellId(center)) + def test_may_intersect(self): + cell = s2.S2Cell(s2.S2CellId(s2.S2Point(1.0, 0.0, 0.0))) self.assertTrue(s2.S2Cap.full().may_intersect(cell)) - # --- S2Region interface --- - - def test_get_cap_bound(self): + def test_cap_bound(self): center = s2.S2Point(1.0, 0.0, 0.0) cap = s2.S2Cap(center, s2.S1Angle.from_radians(1.0)) bound = cap.cap_bound() - self.assertIsInstance(bound, s2.S2Cap) self.assertTrue(bound.approx_equals(cap)) def test_cell_union_bound(self): - cap = s2.S2Cap.full() - cell_ids = cap.cell_union_bound() - self.assertIsInstance(cell_ids, list) + cell_ids = s2.S2Cap.full().cell_union_bound() self.assertGreater(len(cell_ids), 0) - for cid in cell_ids: - self.assertIsInstance(cid, s2.S2CellId) - # --- Operators --- + # Operators def test_equality(self): center = s2.S2Point(1.0, 0.0, 0.0) cap1 = s2.S2Cap(center, s2.S1Angle.from_radians(1.0)) cap2 = s2.S2Cap(center, s2.S1Angle.from_radians(1.0)) - self.assertEqual(cap1, cap2) + self.assertTrue(cap1 == cap2) + self.assertFalse(cap1 != cap2) def test_inequality(self): cap1 = s2.S2Cap.from_point(s2.S2Point(1.0, 0.0, 0.0)) cap2 = s2.S2Cap.from_point(s2.S2Point(0.0, 1.0, 0.0)) - self.assertNotEqual(cap1, cap2) + self.assertTrue(cap1 != cap2) + self.assertFalse(cap1 == cap2) def test_approx_equals(self): - center = s2.S2Point(1.0, 0.0, 0.0) - cap = s2.S2Cap(center, s2.S1Angle.from_radians(1.0)) + cap = s2.S2Cap(s2.S2Point(1.0, 0.0, 0.0), s2.S1Angle.from_radians(1.0)) self.assertTrue(cap.approx_equals(cap)) def test_approx_equals_with_max_error(self): - center = s2.S2Point(1.0, 0.0, 0.0) - cap = s2.S2Cap(center, s2.S1Angle.from_radians(1.0)) + cap = s2.S2Cap(s2.S2Point(1.0, 0.0, 0.0), s2.S1Angle.from_radians(1.0)) self.assertTrue(cap.approx_equals(cap, s2.S1Angle.from_radians(1e-10))) - # --- String representation --- + def test_approx_equals_false(self): + cap1 = s2.S2Cap(s2.S2Point(1.0, 0.0, 0.0), s2.S1Angle.from_radians(1.0)) + cap2 = s2.S2Cap(s2.S2Point(0.0, 1.0, 0.0), s2.S1Angle.from_radians(1.0)) + self.assertFalse(cap1.approx_equals(cap2)) + + def test_hash(self): + center = s2.S2Point(1.0, 0.0, 0.0) + cap1 = s2.S2Cap(center, s2.S1Angle.from_radians(1.0)) + cap2 = s2.S2Cap(center, s2.S1Angle.from_radians(1.0)) + self.assertEqual(hash(cap1), hash(cap2)) + s = {cap1, cap2} + self.assertEqual(len(s), 1) + + # String representation def test_repr(self): cap = s2.S2Cap.empty() @@ -298,33 +290,9 @@ def test_repr(self): def test_str(self): cap = s2.S2Cap.empty() - self.assertIsInstance(str(cap), str) - - # --- S2Cell.cap_bound --- - - # --- __hash__ --- - - def test_hash(self): - center = s2.S2Point(1.0, 0.0, 0.0) - cap = s2.S2Cap(center, s2.S1Angle.from_radians(1.0)) - # S2Cap must be hashable. - h = hash(cap) - self.assertIsInstance(h, int) - # Can be used as a dict key. - d = {cap: "value"} - self.assertEqual(d[cap], "value") - # Can be used in a set. - s = {cap} - self.assertIn(cap, s) - - # --- approx_equals false case --- - - def test_approx_equals_false(self): - cap1 = s2.S2Cap(s2.S2Point(1.0, 0.0, 0.0), - s2.S1Angle.from_radians(1.0)) - cap2 = s2.S2Cap(s2.S2Point(0.0, 1.0, 0.0), - s2.S1Angle.from_radians(1.0)) - self.assertFalse(cap1.approx_equals(cap2)) + s = str(cap) + self.assertIn("Center=", s) + self.assertIn("Radius=", s) if __name__ == "__main__":