pybind: Add S2LatLngRect bindings#655
Open
deustis wants to merge 10 commits into
Open
Conversation
Binds S2LatLngRect to Python with full coverage of the public API: constructors, static factories (from_center_size, from_point, from_point_pair, empty, full, full_lat, full_lng), properties (lat, lng, lat_lo/hi, lng_lo/hi, lo, hi), predicates (is_valid, is_empty, is_full, is_point, is_inverted), geometric accessors (vertex, center, size, get_area, get_centroid), containment and intersection methods, mutation (add_point), set operations (expanded, polar_closure, union, intersection, expanded_by_distance), distance methods (distance_to_rect, distance_to_latlng, directed_hausdorff_distance, hausdorff_distance), the S2Region interface (get_rect_bound, cell_union_bound), and operators (==, !=, approx_equals, approx_equals_latlng, __hash__, __repr__, __str__). Also resolves the TODO in s2cell_bindings.cc by wiring up S2Cell.get_rect_bound() now that S2LatLngRect is bound after S2Cell in the module initialization order. get_cap_bound() on S2LatLngRect and S2Cell remains deferred pending S2Cap bindings.
- Fix __repr__ to prefix class name per README convention - Rename get_area/get_centroid to area/centroid (drop Get prefix) - Rename get_rect_bound to rect_bound (drop Get prefix per convention) - Add inverted rect test (antimeridian-spanning) - Improve boundary_intersects test coverage - Move test_s2cell_rect_bound to s2cell_test.py
Mutating methods are inconsistent with the otherwise-immutable API. from_latlngs provides the same bounding-rect-from-collection pattern without mutating the receiver.
The R1Interval+S1Interval constructor now raises ValueError for out-of-range lat or empty/non-empty mismatch between lat and lng. is_valid() is removed as it would always return True for objects reachable from Python.
Static factories -> Factory methods; Geometric accessors -> Geometric operations; remove S2Region interface label.
- Extract throw conditions into MaybeThrowInvalidLatInterval and MaybeThrowEmptyMismatch helpers, matching the pattern in other bindings - Rename distance_to_rect -> distance, distance_to_latlng -> distance_latlng to match the same-type-short-name convention used by contains/intersects - Drop approx_equals_latlng (the S2LatLng-tolerance overload is not compelling enough to expose under a separate name) - Collapse non-README subsection headers into Geometric operations; add // String representation section comment - Add comment on from_latlngs explaining it replaces the mutable AddPoint pattern (the Python interface is immutable) - Add comment on cell_union_bound explaining the output-parameter conversion - Add test class docstring; fix test_str to assert content; cross-check both == and != in test_equality/test_inequality; rename test methods to match renamed bindings; use plain # Section style throughout
- Move inline comments from before .def() calls into lambda bodies - Replace assertIsInstance calls with concrete value assertions
deustis
marked this pull request as ready for review
July 17, 2026 21:20
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds pybind11 bindings for
S2LatLngRect, covering all key C++ methods including the S2Region interface methods (cap_bound,rect_bound,cell_union_bound).Design choices:
from_latlngs: custom factory replacing the mutableAddPointpattern — callers pass a list ofS2LatLngand receive a new bounding rectangledistance/distance_latlng: renamed from theGetDistanceoverloads (dropping_to_rect/_to_latlngper naming convention)cell_union_bound: wrapsGetCellUnionBound's output parameter, returning a list insteaddirected_hausdorff_distance: usespy::overload_castto resolve the public/private overload ambiguity in C++approx_equals: only the(S2LatLngRect, S1Angle)overload is bound; the interval-level overload is omittedTest plan
bazel test //python:s2latlng_rect_test //python:s2cell_test— all tests pass.