pybind: Add S1Interval bindings (#524)

src/python/BUILD.bazel

@@ -26,10 +26,19 @@ pybind_extension(
     name = "s2geometry_bindings",
     srcs = ["module.cc"],
     deps = [
+        ":s1interval_bindings",
         ":s2point_bindings",
     ],
 )
 
+pybind_library(
+    name = "s1interval_bindings",
+    srcs = ["s1interval_bindings.cc"],
+    deps = [
+        "//:s2",
+    ],
+)
+
 pybind_library(
     name = "s2point_bindings",
     srcs = ["s2point_bindings.cc"],
@@ -42,6 +51,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"],

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`
+- Properties are always read-only. To create a modified object, use a constructor or factory method.
+- Other functions are not properties: `angle.radians()`, `angle.degrees()`, `interval.length()`
+
+**Invalid Values:**
+- Invalid inputs to constructions or functions raises `ValueError`.
+- Example: `S1Interval(0.0, 4.0)` raises `ValueError` because `4.0 > π`.
+- Note: In C++, these conditions trigger `ABSL_DCHECK` assertions. The bindings prevent these assertions from firing by pre-validating inputs.
+- Note: Python bindings check for invalid inputs and throw C++ exceptions which are caught by
+  pybind and converted to Python exceptions. Exceptions are normally prohibited by the C++
+  style guide, but this is the preferred approach for pybind.
+
+**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 `<classname>_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 `<classname>_test.py`
+4. Create tests in `<classname>_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)

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);
 }

src/python/s1interval_bindings.cc

@@ -0,0 +1,150 @@
+#include <pybind11/pybind11.h>
+#include <pybind11/operators.h>
+
+#include <sstream>
+
+#include "s2/s1interval.h"
+
+namespace py = pybind11;
+
+namespace {
+
+void MaybeThrowInvalidPoint(double p) {
+  if (!S1Interval::IsValidPoint(p)) {
+     throw py::value_error("Invalid S1 point: " + std::to_string(p));
+  }
+}
+
+}  // namespace
+
+void bind_s1interval(py::module& m) {
+  py::class_<S1Interval>(m, "S1Interval")
+      // Constructors
+      .def(py::init<>(), "Default constructor creates an empty interval")
+      .def(py::init([](double lo, double hi) {
+             MaybeThrowInvalidPoint(lo);
+             MaybeThrowInvalidPoint(hi);
+             return S1Interval(lo, hi);
+           }),
+           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.\n"
+           "Raises ValueError if either bound is outside that range.")
+
+      // 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",
+                  [](double p1, double p2) {
+                    MaybeThrowInvalidPoint(p1);
+                    MaybeThrowInvalidPoint(p2);
+                    return S1Interval::FromPointPair(p1, p2);
+                  },
+                  py::arg("p1"), py::arg("p2"),
+                  "Constructs the minimal interval containing two points")
+
+      // Properties
+      .def_property_readonly("lo", &S1Interval::lo, "Lower bound")
+      .def_property_readonly("hi", &S1Interval::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_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", [](const S1Interval& self, double p) {
+               MaybeThrowInvalidPoint(p);
+               return self.Contains(p);
+             }, py::arg("p"),
+           "Return true if the interval (which is closed) contains the point 'p'")
+      .def("interior_contains", [](const S1Interval& self, double p) {
+               MaybeThrowInvalidPoint(p);
+               return self.InteriorContains(p);
+             }, py::arg("p"),
+           "Return true if the interior of the interval contains the point 'p'")
+      .def("contains", py::overload_cast<const S1Interval&>(
+               &S1Interval::Contains, py::const_),
+           py::arg("other"),
+           "Return true if the interval contains the given interval 'y'")
+      .def("interior_contains", py::overload_cast<const S1Interval&>(
+               &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& self, double p) {
+               MaybeThrowInvalidPoint(p);
+               self.AddPoint(p);
+             }, 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", [](const S1Interval& self, double p) {
+               if (self.is_empty()) throw py::value_error("Invalid S1Interval");
+               MaybeThrowInvalidPoint(p);
+               return self.Project(p);
+             }, 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();
+      });
+}