pybind: Add S1ChordAngle bindings#5
Closed
deustis wants to merge 3 commits into
Closed
Conversation
deustis
force-pushed
the
deustis/s2cell_bindings
branch
3 times, most recently
from
a9a433c to
1fc4db2
Compare
Binds S1ChordAngle with constructors, factory methods (from_radians, from_degrees, from_e5/6/7, fast_upper_bound_from, from_length2, zero, right, straight, infinity, negative), properties (length2, radians, degrees, e5/6/7), predicates (is_zero, is_negative, is_infinity, is_special, is_valid), geometric ops (to_angle, sin/cos/tan, sin2, successor, predecessor, plus_error, error bounds), arithmetic (+, -, +=, -=) with pre-validation that rejects Negative()/Infinity() operands, comparisons, hash, and __repr__/__str__. Unblocks the deferred distance methods on S2Cell (GetDistance, GetMaxDistance, IsDistanceLess, etc.) which can be added in a follow-up. Stacked on deustis/s2cell_bindings.
deustis
force-pushed
the
deustis/s1chord_angle_bindings
branch
from
9fbf526 to
7f453b3
Compare
deustis
commented
May 13, 2026
- Use S2::IsUnitLength for unit-length validation, matching the C++ DCHECK exactly; add s2pointutil.h include - Drop fast_upper_bound_from, from_length2, from_e5/e6/e7 factory methods - Drop length2 property, sin2, successor, predecessor, plus_error, and error-bound accessor bindings - Update tests accordingly, replacing length2 assertions with radians/degrees/comparisons
deustis
commented
May 13, 2026
- Guard sin/cos/tan against special values (negative/infinity) to prevent DCHECK in C++; add test_sin_cos_tan_special_raises - Guard e5/e6/e7 properties against special values; negative() would silently return a meaningless integer without this guard; add test_e5_e6_e7_special_raises - Rename MaybeThrowSpecialForArithmetic -> MaybeThrowIfSpecial - Update error messages to use Python-style lowercase names (negative(), infinity()) - Switch __hash__ from std::hash<double> to absl::Hash<double>; add absl/hash BUILD dep - Fix test_str and test_repr to assert exact values - Fix test_constructor_from_antipodal_points to use assertEqual(straight()) - Add test_to_angle_special_values and test_str_negative_sentinel
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.
Summary
S1ChordAngle: default / two-point /S1Angleconstructors, factory methods (from_radians,from_degrees,from_e5/6/7,fast_upper_bound_from,from_length2,zero,right,straight,infinity,negative), properties (length2,radians,degrees,e5/6/7), predicates (is_zero,is_negative,is_infinity,is_special,is_valid), geometric ops (to_angle,sin/cos/tan,sin2,successor,predecessor,plus_error,s2_point_constructor_max_error,s1_angle_constructor_max_error).==,!=,<,>,<=,>=,+,-,+=,-=,__hash__,__repr__,__str__. Arithmetic pre-validates that neither operand isNegative()orInfinity(), matching the C++!is_special()precondition.ValueError(in C++ this is anABSL_DCHECK; the pybind convention is to surface preconditions as exceptions).Unblocks
Once this lands, the distance methods on
S2Cellthat were deferred in #4 (PR google#593 stack) can be filled in as a follow-up:get_distance(point),get_distance(a, b),get_distance(cell),get_boundary_distance,get_max_distance(*),is_distance_less*,is_max_distance_less*.Stack
Stacked on
deustis/s2cell_bindings(draft PR #4), which is stacked ondeustis/s2cell_id_bindings(upstream PR google#593).Test plan
bazel test //python/...— all 10 suites pass (46 new S1ChordAngle tests, existing suites still green).