diff --git a/src/python/BUILD.bazel b/src/python/BUILD.bazel index 1093ca2c..2c307ecb 100644 --- a/src/python/BUILD.bazel +++ b/src/python/BUILD.bazel @@ -25,14 +25,26 @@ py_library( pybind_extension( name = "s2geometry_bindings", srcs = ["module.cc"], + copts = ["-DNDEBUG"], deps = [ + ":s1interval_bindings", ":s2point_bindings", ], ) +pybind_library( + name = "s1interval_bindings", + srcs = ["s1interval_bindings.cc"], + copts = ["-DNDEBUG"], + deps = [ + "//:s2", + ], +) + pybind_library( name = "s2point_bindings", srcs = ["s2point_bindings.cc"], + copts = ["-DNDEBUG"], deps = [ "//:s2", ], @@ -42,6 +54,12 @@ pybind_library( # Python Tests # ======================================== +py_test( + name = "s1interval_test", + srcs = ["s1interval_test.py"], + deps = [":s2geometry_pybind"], +) + py_test( name = "s2point_test", srcs = ["s2point_test.py"], diff --git a/src/python/README.md b/src/python/README.md index a47277d3..ace22c6a 100644 --- a/src/python/README.md +++ b/src/python/README.md @@ -13,19 +13,9 @@ The S2 Geometry library is transitioning from SWIG-based bindings to pybind11-ba Once the pybind11 bindings are feature-complete and stable, the SWIG bindings will be deprecated and the pybind11 package will be renamed to `s2geometry` to become the primary Python API. -## Directory Structure +## User Guide -``` -python/ -├── module.cc # Binding module entry point -├── s2point_bindings.cc # Bindings for S2Point (add more *_bindings.cc as needed) -├── s2geometry_pybind/ # Dir for Python package -│ └── __init__.py # Package initialization -├── s2point_test.py # Tests for S2Point (add more *_test.py as needed) -└── BUILD.bazel # Build rules for bindings, library, and tests -``` - -## Usage Example +### Usage Example ```python import s2geometry_pybind as s2 @@ -36,8 +26,65 @@ sum_point = p1 + p2 print(sum_point) ``` +### Interface Notes + +The Python bindings follow the C++ API closely but with Pythonic conventions: + +**Naming Conventions:** +- Core classes exist within the top-level module; we may define submodules for utility classes. +- Class names remain unchanged (e.g., `S2Point`, `S1Angle`, `R1Interval`) +- Method names are converted to snake_case (converted from UpperCamelCase C++ function names) + +**Properties vs. Methods:** +- Simple coordinate accessors are properties: `point.x`, `point.y`, `interval.lo`, `interval.hi` +- If the C++ class has a trivial set_foo method for the property, then the Python property is mutable (otherwise it is a read-only property). +- Other functions are not properties: `angle.radians()`, `angle.degrees()`, `interval.length()` + +**Invalid/Unnormalized Values:** +- Some constructors accept invalid values or unnormalized values. +- Examples: + - `S1Interval(0.0, 4.0)` with `4.0 > π` creates interval where `is_valid()` is `False` + - `S2Point(3.0, 4.0, 0.0)` creates an unnormalized point outside of the unit sphere; use `normalize()` to normalize. +- In **C++** invalid values will typically trigger (`ABSL_DCHECK`) when compiled with debug options. +- In **Python bindings** debug assertions (`ABSL_DCHECK`) are disabled, matching the production optimized C++ behavior. + +**Documentation:** +- Python docstrings provide essential information about parameters, return values, and key behaviors +- For comprehensive documentation including edge cases and algorithmic details, refer to the C++ header files +- The C++ documentation is the authoritative source of truth + +**Operators:** +- Standard Python operators work as expected: `+`, `-`, `*`, `==`, `!=`, `<`, `>` (for C++ classes that implement those operators) + +**String Representations:** +- `repr()` prefixes the class name and delegates to C++ `operator<<` for the value +- `str()` delegates to C++ `operator<<` for a cleaner output +- Example: `repr(S1Interval(0.0, 2.0))` returns `'S1Interval([0, 2])'` while `str()` returns `'[0, 2]'` + +**Vector Inheritance:** +- In C++, various geometry classes inherit from or expose vector types (e.g., `S2Point` inherits from `Vector3_d`, `R2Point` is a type alias for `Vector2_d`, `R1Interval` returns bounds as `Vector2_d`) +- The Python bindings do **not** expose this inheritance hierarchy; it is treated as an implementation detail +- Instead, classes that inherit from a vector expose key functions from the `BasicVector` interface (e.g., `norm()`, `dot_prod()`, `cross_prod()`) +- C++ functions that accept or return a vector object use a Python tuple (of length matching the vector dimension) +- Array indexing operators (e.g., `point[0]`) are not currently supported + +**Serialization:** +- The C++ Encoder/Decoder serialization functions are not currently supported + ## Development +### Directory Structure + +``` +python/ +├── module.cc # Binding module entry point +├── s2point_bindings.cc # Bindings for S2Point (add more *_bindings.cc as needed) +├── s2geometry_pybind/ # Dir for Python package +│ └── __init__.py # Package initialization +├── s2point_test.py # Tests for S2Point (add more *_test.py as needed) +└── BUILD.bazel # Build rules for bindings, library, and tests +``` + ### Building with Bazel (pybind11 bindings) Bazel can be used for development and testing of the new pybind11-based bindings. @@ -82,4 +129,18 @@ To add bindings for a new class: 1. Create `_bindings.cc` with pybind11 bindings 2. Update `BUILD.bazel` to add a new `pybind_library` target 3. Update `module.cc` to call your binding function -4. Create tests in `_test.py` \ No newline at end of file +4. Create tests in `_test.py` + +### Binding File Organization + +Use the following sections to organize functions within the bindings files and tests. Secondarily, follow the order in which functions are declared in the C++ headers. + +1. **Constructors** - Default constructors and constructors with parameters +2. **Factory methods** - Static factory methods (e.g., `from_degrees`, `from_radians`, `zero`, `invalid`) +3. **Properties** - Mutable and read-only properties (e.g., coordinate accessors like `x`, `y`, `lo`, `hi`) +4. **Predicates** - Simple boolean state checks (e.g., `is_empty`, `is_valid`, `is_full`) +5. **Geometric operations** - All other methods including conversions, computations, containment checks, set operations, normalization, and distance calculations +6. **Vector operations** - Methods from the Vector base class (e.g., `norm`, `norm2`, `normalize`, `dot_prod`, `cross_prod`, `angle`). Only applicable to classes that inherit from `util/math/vector.h` +7. **Operators** - Operator overloads (e.g., `==`, `+`, `*`, comparison operators) +8. **String representation** - `__repr__` (which also provides `__str__`), and string conversion methods like `to_string_in_degrees` +9. **Module-level functions** - Standalone functions (e.g., trigonometric functions for S1Angle) diff --git a/src/python/module.cc b/src/python/module.cc index a79b8771..4fc354b6 100644 --- a/src/python/module.cc +++ b/src/python/module.cc @@ -3,9 +3,13 @@ namespace py = pybind11; // Forward declarations for binding functions +void bind_s1interval(py::module& m); void bind_s2point(py::module& m); PYBIND11_MODULE(s2geometry_bindings, m) { m.doc() = "S2 Geometry Python bindings using pybind11"; + + // Bind core geometry classes in dependency order + bind_s1interval(m); bind_s2point(m); } diff --git a/src/python/s1interval_bindings.cc b/src/python/s1interval_bindings.cc new file mode 100644 index 00000000..38e46113 --- /dev/null +++ b/src/python/s1interval_bindings.cc @@ -0,0 +1,119 @@ +#include +#include + +#include + +#include "s2/s1interval.h" + +namespace py = pybind11; + +void bind_s1interval(py::module& m) { + py::class_(m, "S1Interval") + // Constructors + .def(py::init<>(), "Default constructor creates an empty interval") + .def(py::init(), + py::arg("lo"), py::arg("hi"), + "Constructor that accepts the endpoints of the interval.\n\n" + "Both endpoints must be in the range -Pi to Pi inclusive.") + + // Static factory methods + .def_static("empty", &S1Interval::Empty, "Returns the empty interval") + .def_static("full", &S1Interval::Full, "Returns the full interval") + .def_static("from_point", &S1Interval::FromPoint, py::arg("p"), + "Constructs an interval containing a single point") + .def_static("from_point_pair", &S1Interval::FromPointPair, + py::arg("p1"), py::arg("p2"), + "Constructs the minimal interval containing two points") + + // Properties + .def_property("lo", &S1Interval::lo, &S1Interval::set_lo, "Lower bound") + .def_property("hi", &S1Interval::hi, &S1Interval::set_hi, "Upper bound") + .def("bounds", [](const S1Interval& self) { + return py::make_tuple(self.lo(), self.hi()); + }, "Return bounds as a tuple (lo, hi)") + + // Predicates + .def("is_valid", &S1Interval::is_valid, + "An interval is valid if neither bound exceeds Pi in absolute value") + .def("is_full", &S1Interval::is_full, + "Return true if the interval contains all points on the unit circle") + .def("is_empty", &S1Interval::is_empty, + "Return true if the interval is empty, i.e. it contains no points") + .def("is_inverted", &S1Interval::is_inverted, + "Return true if lo() > hi(). (This is true for empty intervals.)") + + // Geometric operations + .def("center", &S1Interval::GetCenter, + "Return the midpoint of the interval.\n\n" + "For full and empty intervals, the result is arbitrary.") + .def("length", &S1Interval::GetLength, + "Return the length of the interval.\n\n" + "The length of an empty interval is negative.") + .def("complement_center", &S1Interval::GetComplementCenter, + "Return the midpoint of the complement of the interval.\n\n" + "For full and empty intervals, the result is arbitrary. For a\n" + "singleton interval, the result is its antipodal point on S1.") + .def("contains", py::overload_cast(&S1Interval::Contains, py::const_), + py::arg("p"), + "Return true if the interval (which is closed) contains the point 'p'") + .def("interior_contains", py::overload_cast( + &S1Interval::InteriorContains, py::const_), + py::arg("p"), "Return true if the interior of the interval contains the point 'p'") + .def("contains", py::overload_cast( + &S1Interval::Contains, py::const_), + py::arg("other"), + "Return true if the interval contains the given interval 'y'") + .def("interior_contains", py::overload_cast( + &S1Interval::InteriorContains, py::const_), + py::arg("other"), + "Return true if the interior of this interval contains the entire interval 'y'") + .def("intersects", &S1Interval::Intersects, py::arg("other"), + "Return true if the two intervals contain any points in common") + .def("interior_intersects", &S1Interval::InteriorIntersects, + py::arg("other"), + "Return true if the interior of this interval contains any point of 'y'") + .def("add_point", &S1Interval::AddPoint, py::arg("p"), + "Expand the interval to contain the given point 'p'.\n\n" + "The point should be an angle in the range [-Pi, Pi].") + .def("project", &S1Interval::Project, py::arg("p"), + "Return the closest point in the interval to 'p'.\n\n" + "The interval must be non-empty.") + .def("expanded", &S1Interval::Expanded, py::arg("margin"), + "Return interval expanded on each side by 'margin' (radians).\n\n" + "If 'margin' is negative, shrink the interval instead. The resulting\n" + "interval may be empty or full. Any expansion of a full interval remains\n" + "full, and any expansion of an empty interval remains empty.") + .def("union", &S1Interval::Union, py::arg("other"), + "Return the smallest interval containing this interval and 'y'") + .def("intersection", &S1Interval::Intersection, py::arg("other"), + "Return the smallest interval containing the intersection with 'y'.\n\n" + "Note that the region of intersection may consist of two disjoint intervals.") + .def("complement", &S1Interval::Complement, + "Return the complement of the interior of the interval") + .def("directed_hausdorff_distance", + &S1Interval::GetDirectedHausdorffDistance, + py::arg("other"), + "Return the directed Hausdorff distance to 'y'") + // Note: default value must match C++ signature in s1interval.h + .def("approx_equals", &S1Interval::ApproxEquals, + py::arg("other"), py::arg("max_error") = 1e-15, + "Return true if approximately equal to 'y'.\n\n" + "Two intervals are approximately equal if each endpoint can be moved\n" + "by at most 'max_error' (radians) to match the other interval.") + + // Operators + .def(py::self == py::self, "Return true if two intervals contain the same set of points") + .def(py::self != py::self, "Return true if two intervals do not contain the same set of points") + + // String representation + .def("__repr__", [](const S1Interval& i) { + std::ostringstream oss; + oss << "S1Interval(" << i << ")"; + return oss.str(); + }) + .def("__str__", [](const S1Interval& i) { + std::ostringstream oss; + oss << i; + return oss.str(); + }); +} diff --git a/src/python/s1interval_test.py b/src/python/s1interval_test.py new file mode 100644 index 00000000..0759dea3 --- /dev/null +++ b/src/python/s1interval_test.py @@ -0,0 +1,253 @@ +"""Tests for S1Interval pybind11 bindings.""" + +import unittest +import math +import s2geometry_pybind as s2 + + +class TestS1Interval(unittest.TestCase): + """Test cases for S1Interval bindings.""" + + # Constructors + + def test_default_constructor(self): + interval = s2.S1Interval() + self.assertTrue(interval.is_empty()) + + def test_constructor_with_bounds(self): + interval = s2.S1Interval(0.0, math.pi / 2) + self.assertAlmostEqual(interval.lo, 0.0) + self.assertAlmostEqual(interval.hi, math.pi / 2) + self.assertFalse(interval.is_empty()) + + def test_constructor_converts_minus_pi_to_pi(self): + # "The value -π is converted internally to π except for the Full() and Empty() intervals" + # (from s1interval.h constructor documentation) + interval = s2.S1Interval(-math.pi, 0.0) + self.assertEqual(interval.lo, math.pi) # -π converted to π + self.assertEqual(interval.hi, 0.0) + self.assertTrue(interval.is_valid()) + self.assertTrue(interval.is_inverted()) + + # Static factory methods + + def test_empty_constructor(self): + empty = s2.S1Interval.empty() + self.assertTrue(empty.is_empty()) + + def test_full_constructor(self): + full = s2.S1Interval.full() + self.assertTrue(full.is_full()) + + def test_from_point(self): + point_interval = s2.S1Interval.from_point(1.0) + self.assertEqual(point_interval.lo, 1.0) + self.assertEqual(point_interval.hi, 1.0) + + def test_from_point_pair(self): + pair_interval = s2.S1Interval.from_point_pair(0.5, 2.0) + self.assertTrue(pair_interval.contains(1.0)) + + # Properties + + def test_properties_lo_hi(self): + interval = s2.S1Interval(0.0, math.pi / 2) + self.assertAlmostEqual(interval.lo, 0.0) + self.assertAlmostEqual(interval.hi, math.pi / 2) + + def test_bounds(self): + interval = s2.S1Interval(0.5, 2.0) + bounds = interval.bounds() + self.assertEqual(bounds, (0.5, 2.0)) + self.assertAlmostEqual(bounds[0], interval.lo) + self.assertAlmostEqual(bounds[1], interval.hi) + + # Predicates + + def test_is_valid_with_normal_interval(self): + valid = s2.S1Interval(0.0, math.pi) + self.assertTrue(valid.is_valid()) + + def test_is_valid_with_empty_interval(self): + empty = s2.S1Interval.empty() + self.assertTrue(empty.is_valid()) + + def test_is_valid_with_full_interval(self): + full = s2.S1Interval.full() + self.assertTrue(full.is_valid()) + + def test_invalid_bounds_too_large(self): + invalid = s2.S1Interval(0.0, 4.0) # 4.0 > π + self.assertFalse(invalid.is_valid()) + + def test_invalid_bounds_too_small(self): + invalid = s2.S1Interval(-4.0, 0.0) # -4.0 < -π + self.assertFalse(invalid.is_valid()) + + def test_invalid_both_bounds_out_of_range(self): + invalid = s2.S1Interval(-4.0, 4.0) + self.assertFalse(invalid.is_valid()) + + def test_is_full_and_empty(self): + empty = s2.S1Interval.empty() + self.assertTrue(empty.is_empty()) + self.assertFalse(empty.is_full()) + + full = s2.S1Interval.full() + self.assertTrue(full.is_full()) + self.assertFalse(full.is_empty()) + + normal = s2.S1Interval(0.0, math.pi) + self.assertFalse(normal.is_empty()) + self.assertFalse(normal.is_full()) + + def test_is_inverted(self): + # Normal interval + normal = s2.S1Interval(0.0, math.pi) + self.assertFalse(normal.is_inverted()) + + # Inverted interval (wraps around) + inverted = s2.S1Interval(math.pi, 0.0) + self.assertTrue(inverted.is_inverted()) + + # Geometric operations + + def test_center(self): + interval = s2.S1Interval(0.0, math.pi) + self.assertAlmostEqual(interval.center(), math.pi / 2) + + def test_length(self): + interval = s2.S1Interval(0.0, math.pi) + self.assertAlmostEqual(interval.length(), math.pi) + + def test_complement_center(self): + interval = s2.S1Interval(0.0, math.pi / 2) + complement_center = interval.complement_center() + self.assertIsNotNone(complement_center) + + def test_contains_point(self): + interval = s2.S1Interval(0.0, math.pi) + self.assertTrue(interval.contains(math.pi / 2)) + self.assertTrue(interval.contains(0.0)) + self.assertTrue(interval.contains(math.pi)) + self.assertFalse(interval.contains(-math.pi / 2)) + + def test_interior_contains_point(self): + interval = s2.S1Interval(0.0, math.pi) + self.assertTrue(interval.interior_contains(math.pi / 2)) + self.assertFalse(interval.interior_contains(0.0)) + self.assertFalse(interval.interior_contains(math.pi)) + + def test_contains_interval(self): + interval1 = s2.S1Interval(0.0, math.pi) + interval2 = s2.S1Interval(0.5, 2.0) + interval3 = s2.S1Interval(-math.pi, -2.0) + self.assertTrue(interval1.contains(interval2)) + self.assertFalse(interval1.contains(interval3)) + + def test_interior_contains_interval(self): + interval1 = s2.S1Interval(0.0, math.pi) + interval2 = s2.S1Interval(0.5, 2.0) + self.assertTrue(interval1.interior_contains(interval2)) + + def test_intersects(self): + interval1 = s2.S1Interval(0.0, 2.0) + interval2 = s2.S1Interval(1.0, 3.0) + interval3 = s2.S1Interval(-3.0, -2.5) + self.assertTrue(interval1.intersects(interval2)) + self.assertFalse(interval1.intersects(interval3)) + + def test_interior_intersects(self): + interval1 = s2.S1Interval(0.0, 2.0) + interval2 = s2.S1Interval(1.0, 3.0) + interval3 = s2.S1Interval(-3.0, -2.5) + self.assertTrue(interval1.interior_intersects(interval2)) + self.assertFalse(interval1.interior_intersects(interval3)) + + def test_add_point(self): + interval = s2.S1Interval(0.0, 1.0) + interval.add_point(1.5) + self.assertTrue(interval.contains(1.5)) + + def test_project(self): + interval = s2.S1Interval(0.0, 1.5) + self.assertAlmostEqual(interval.project(1.0), 1.0) # Point inside interval + + def test_expanded(self): + interval = s2.S1Interval(0.5, 1.5) + expanded = interval.expanded(0.5) + self.assertAlmostEqual(expanded.length(), + interval.length() + 1.0) + + def test_union(self): + interval1 = s2.S1Interval(0.0, 1.0) + interval2 = s2.S1Interval(0.5, 1.5) + union = interval1.union(interval2) + self.assertTrue(union.contains(0.5)) + self.assertTrue(union.contains(1.2)) + + def test_intersection(self): + interval1 = s2.S1Interval(0.0, 2.0) + interval2 = s2.S1Interval(1.0, 3.0) + intersection = interval1.intersection(interval2) + self.assertAlmostEqual(intersection.lo, 1.0) + self.assertAlmostEqual(intersection.hi, 2.0) + + def test_complement(self): + interval = s2.S1Interval(0.0, math.pi / 2) + complement = interval.complement() + self.assertTrue(complement.is_inverted()) + self.assertFalse(complement.contains(math.pi / 4)) + self.assertTrue(complement.contains(math.pi)) + + def test_directed_hausdorff_distance(self): + interval1 = s2.S1Interval(0.0, 1.0) + interval2 = s2.S1Interval(0.5, 1.5) + distance = interval1.directed_hausdorff_distance(interval2) + self.assertIsNotNone(distance) + + def test_approx_equals(self): + interval1 = s2.S1Interval(0.0, 2.0) + interval2 = s2.S1Interval(0.0, 2.0) + interval3 = s2.S1Interval(0.0, 2.0 + 1e-16) # Very small difference + interval4 = s2.S1Interval(0.0, 2.1) # Larger difference + + # Test with default max_error (1e-15) + self.assertTrue(interval1.approx_equals(interval2)) + self.assertTrue(interval1.approx_equals(interval3)) # Within default tolerance + self.assertFalse(interval1.approx_equals(interval4)) # Outside default tolerance + + # Test with explicit max_error values + self.assertTrue(interval1.approx_equals(interval3, 1e-15)) + self.assertFalse(interval1.approx_equals(interval4, 0.05)) + self.assertTrue(interval1.approx_equals(interval4, 0.2)) + + # Operators + + def test_equality(self): + interval1 = s2.S1Interval(0.0, 2.0) + interval2 = s2.S1Interval(0.0, 2.0) + interval3 = s2.S1Interval(0.0, 3.0) + self.assertTrue(interval1 == interval2) + self.assertTrue(interval1 != interval3) + + # String representation + + def test_string_representation(self): + interval = s2.S1Interval(0.0, 2.0) + self.assertEqual(repr(interval), "S1Interval([0, 2])") + self.assertEqual(str(interval), "[0, 2]") + + empty = s2.S1Interval.empty() + # Empty interval has lo=pi, hi=-pi + self.assertEqual(repr(empty), "S1Interval([3.14159, -3.14159])") + self.assertEqual(str(empty), "[3.14159, -3.14159]") + + full = s2.S1Interval.full() + # Full interval spans from -pi to pi + self.assertEqual(repr(full), "S1Interval([-3.14159, 3.14159])") + self.assertEqual(str(full), "[-3.14159, 3.14159]") + + +if __name__ == "__main__": + unittest.main() diff --git a/src/python/s2point_bindings.cc b/src/python/s2point_bindings.cc index 88a0d454..97ea3f69 100644 --- a/src/python/s2point_bindings.cc +++ b/src/python/s2point_bindings.cc @@ -1,6 +1,8 @@ #include #include +#include + #include "s2/s2point.h" namespace py = pybind11; @@ -8,59 +10,77 @@ namespace py = pybind11; void bind_s2point(py::module& m) { py::class_(m, "S2Point") // Constructors - .def(py::init<>(), "Default constructor") + .def(py::init<>(), "Default constructor creates a zero vector") .def(py::init(), py::arg("x"), py::arg("y"), py::arg("z"), - "Construct S2Point from x, y, z coordinates") + "Constructor that accepts x, y, z coordinates") + .def(py::init([](py::tuple t) { + if (t.size() != 3) { + throw py::value_error("Tuple must have exactly 3 elements"); + } + return S2Point(t[0].cast(), t[1].cast(), t[2].cast()); + }), py::arg("coords"), "Constructor that accepts a tuple of (x, y, z) coordinates") - // Accessors + // Properties .def_property_readonly("x", py::overload_cast<>(&S2Point::x, py::const_), - "x coordinate") + "The x coordinate") .def_property_readonly("y", py::overload_cast<>(&S2Point::y, py::const_), - "y coordinate") + "The y coordinate") .def_property_readonly("z", py::overload_cast<>(&S2Point::z, py::const_), - "z coordinate") + "The z coordinate") + .def("data", [](const S2Point& self) { + return py::make_tuple(self.x(), self.y(), self.z()); + }, "Return coordinates as a tuple (x, y, z)") // Vector operations .def("norm", &S2Point::Norm, "Return the Euclidean norm (length)") - .def("norm2", &S2Point::Norm2, "Return the squared Euclidean norm") - .def("normalize", &S2Point::Normalize, "Return a normalized copy") + .def("norm2", &S2Point::Norm2, "Return the squared Euclidean norm (dot product with itself)") + .def("normalize", &S2Point::Normalize, + "Return a normalized copy of the vector.\n\n" + "Returns a unit vector if the norm is nonzero.") .def("dot_prod", [](const S2Point& self, const S2Point& other) { return self.DotProd(other); }, py::arg("other"), "Return the dot product with another point") .def("cross_prod", [](const S2Point& self, const S2Point& other) -> S2Point { return self.CrossProd(other); }, py::arg("other"), "Return the cross product with another point") - .def("angle", &S2Point::Angle, py::arg("other"), - "Return the angle to another point") + .def("angle", [](const S2Point& self, const S2Point& other) { + return self.Angle(other); + }, py::arg("other"), + "Return the angle between this and another point (radians).\n\n" + "The result is in the range [0, pi]. If either vector is zero-length,\n" + "or nearly zero-length, the result will be zero.") // Operators - .def(py::self + py::self, "Add two points") - .def(py::self - py::self, "Subtract two points") + .def(py::self + py::self, "Add two points (vector addition)") + .def(py::self - py::self, "Subtract two points (vector subtraction)") .def(py::self * double(), "Multiply by scalar") .def("__rmul__", [](const S2Point& self, double v) -> S2Point { return self * v; - }, "Multiply by scalar") + }, "Multiply by scalar (reversed operands)") .def(py::self / double(), "Divide by scalar") .def(-py::self, "Negate point") - .def(py::self == py::self, "Check equality") - .def(py::self != py::self, "Check inequality") + .def(py::self == py::self, "Return true if points are exactly equal") + .def(py::self != py::self, "Return true if points are not exactly equal") .def(py::self += py::self, "In-place addition") .def(py::self -= py::self, "In-place subtraction") .def("__imul__", [](S2Point& self, double v) -> S2Point& { return self *= v; - }, py::arg("v"), "In-place multiplication") + }, py::arg("v"), "In-place multiplication by scalar") .def("__itruediv__", [](S2Point& self, double v) -> S2Point& { return self /= v; - }, py::arg("v"), "In-place division") + }, py::arg("v"), "In-place division by scalar") // String representation + // __repr__ prefixes class name, __str__ delegates to C++ operator<< .def("__repr__", [](const S2Point& p) { - return "S2Point(" + std::to_string(p.x()) + ", " + - std::to_string(p.y()) + ", " + std::to_string(p.z()) + ")"; + std::ostringstream oss; + oss << "S2Point(" << p << ")"; + return oss.str(); }) .def("__str__", [](const S2Point& p) { - return "(" + std::to_string(p.x()) + ", " + - std::to_string(p.y()) + ", " + std::to_string(p.z()) + ")"; + std::ostringstream oss; + oss << p; + return oss.str(); }); } diff --git a/src/python/s2point_test.py b/src/python/s2point_test.py index a95fecfb..a7f9c64a 100644 --- a/src/python/s2point_test.py +++ b/src/python/s2point_test.py @@ -1,12 +1,14 @@ """Tests for S2Point pybind11 bindings.""" +import math import unittest import s2geometry_pybind as s2 - class TestS2Point(unittest.TestCase): """Test cases for S2Point bindings.""" + # Constructors + def test_default_constructor(self): p = s2.S2Point() self.assertEqual(p.x, 0.0) @@ -19,6 +21,39 @@ def test_constructor_with_coordinates(self): self.assertEqual(p.y, 2.0) self.assertEqual(p.z, 3.0) + def test_unnormalized_point(self): + # Unnormalized points are accepted by the constructor. + p = s2.S2Point(3.0, 4.0, 0.0) + self.assertAlmostEqual(p.x, 3.0) + self.assertAlmostEqual(p.y, 4.0) + self.assertAlmostEqual(p.z, 0.0) + self.assertAlmostEqual(p.norm(), 5.0) # Not on unit sphere + + # User may call normalize() to obtain a unit vector. + normalized = p.normalize() + self.assertAlmostEqual(normalized.norm(), 1.0) + + def test_constructor_from_tuple(self): + p = s2.S2Point((1.0, 2.0, 3.0)) + self.assertEqual(p.x, 1.0) + self.assertEqual(p.y, 2.0) + self.assertEqual(p.z, 3.0) + + def test_constructor_from_tuple_wrong_length(self): + with self.assertRaises(ValueError): + s2.S2Point((1.0, 2.0)) + with self.assertRaises(ValueError): + s2.S2Point((1.0, 2.0, 3.0, 4.0)) + + # Properties + + def test_data(self): + p = s2.S2Point(1.0, 2.0, 3.0) + coords = p.data() + self.assertEqual(coords, (1.0, 2.0, 3.0)) + + # Vector operations. + def test_norm(self): p = s2.S2Point(3.0, 4.0, 0.0) self.assertAlmostEqual(p.norm(), 5.0) @@ -47,6 +82,13 @@ def test_cross_product(self): self.assertAlmostEqual(cross.y, 0.0) self.assertAlmostEqual(cross.z, 1.0) + def test_angle(self): + p1 = s2.S2Point(1.0, 0.0, 0.0) + p2 = s2.S2Point(0.0, 1.0, 0.0) + self.assertAlmostEqual(p1.angle(p2), math.pi / 2) + + # Operators + def test_addition(self): p1 = s2.S2Point(1.0, 2.0, 3.0) p2 = s2.S2Point(4.0, 5.0, 6.0) @@ -94,9 +136,12 @@ def test_equality(self): self.assertEqual(p1, p2) self.assertNotEqual(p1, p3) - def test_repr(self): + # String representation + + def test_string_representation(self): p = s2.S2Point(1.0, 2.0, 3.0) - self.assertEqual(repr(p), "S2Point(1.000000, 2.000000, 3.000000)") + self.assertEqual(repr(p), "S2Point([1, 2, 3])") + self.assertEqual(str(p), "[1, 2, 3]") if __name__ == "__main__": diff --git a/src/s2/s1interval.h b/src/s2/s1interval.h index 9faed732..d8b5d46c 100644 --- a/src/s2/s1interval.h +++ b/src/s2/s1interval.h @@ -190,6 +190,7 @@ class S1Interval { // considered to start at an arbitrary point on the unit circle, thus any // interval with (length <= 2*max_error) matches the empty interval, and any // interval with (length >= 2*Pi - 2*max_error) matches the full interval. + // Note: default value is duplicated in Python bindings (s1interval_bindings.cc) bool ApproxEquals(const S1Interval& y, double max_error = 1e-15) const; // Low-level methods to modify one endpoint of an existing S1Interval. diff --git a/src/s2/s2point_test.cc b/src/s2/s2point_test.cc index 6d0f9a77..371ca87c 100644 --- a/src/s2/s2point_test.cc +++ b/src/s2/s2point_test.cc @@ -134,4 +134,18 @@ TEST(S2Point, FRoundWorks) { S2Point a(1.4, 1.5, 1.6); EXPECT_THAT(a.FRound(), Eq(S2Point(1, 2, 2))); } + +TEST(S2Point, UnnormalizedPointAccepted) { + // Unnormalized points are accepted by the constructor. + S2Point p(3.0, 4.0, 0.0); + EXPECT_EQ(p.x(), 3.0); + EXPECT_EQ(p.y(), 4.0); + EXPECT_EQ(p.z(), 0.0); + EXPECT_DOUBLE_EQ(p.Norm(), 5.0); // Not on unit sphere + + // User may call Normalize() to obtain a unit vector. + S2Point normalized = p.Normalize(); + EXPECT_DOUBLE_EQ(normalized.Norm(), 1.0); +} + } // namespace