Skip to content

pybind: Add S2Cell bindings#4

Closed
deustis wants to merge 6 commits into
masterfrom
deustis/s2cell_bindings
Closed

pybind: Add S2Cell bindings#4
deustis wants to merge 6 commits into
masterfrom
deustis/s2cell_bindings

Conversation

@deustis
Copy link
Copy Markdown
Owner

@deustis deustis commented May 7, 2026

Adds pybind11 bindings for S2Cell, covering constructors, factory methods, properties, geometric operations, containment/intersection tests, distance queries, and traversal.

The is_distance_less / is_distance_less_or_equal family is omitted — callers can use get_distance(...) < limit directly.

Naming: GetDistance is overloaded in C++ for S2Point, edge (a, b), and S2Cell arguments. Per the binding interface notes, the short name (get_distance) is used for the S2Point overload, and longer names (get_distance_to_edge, get_distance_to_cell) for the others. get_max_distance follows the same pattern.

_raw variants omitted: vertex_raw, edge_raw, and center_raw return unnormalized vectors intended for feeding into exact C++ predicates. Python callers won't do that, so these are dropped in favor of the normalized versions.

subdivide() returns a 4-tuple of S2Cell rather than an output array. get_cap_bound and get_rect_bound are deferred until S2Cap and S2LatLngRect are bound. Encode / Decode are intentionally omitted per the serialization policy in the pybind README.

Test plan

bazel test //python/... — all suites pass.

@deustis deustis mentioned this pull request May 7, 2026
1 task
@deustis deustis force-pushed the deustis/s2cell_bindings branch from c2cd642 to e212df1 Compare May 12, 2026 16:44
@deustis deustis changed the base branch from deustis/s2cell_id_bindings to master May 12, 2026 16:44
Comment thread src/python/s2cell_bindings.cc Outdated
Comment thread src/python/s2cell_bindings.cc
Comment thread src/python/s2cell_bindings.cc
Comment thread src/python/s2cell_id_bindings.cc Outdated
@deustis deustis force-pushed the deustis/s2cell_bindings branch from 9e7ace4 to a9a433c Compare May 12, 2026 19:34
@deustis deustis changed the base branch from master to deustis/s2cell_id_binding_fixes May 12, 2026 19:37
@deustis deustis force-pushed the deustis/s2cell_bindings branch from a9a433c to 1fc4db2 Compare May 12, 2026 22:52
@deustis deustis force-pushed the deustis/s2cell_id_binding_fixes branch from 32b3ab8 to 96ba727 Compare May 22, 2026 21:15
@deustis deustis force-pushed the deustis/s2cell_bindings branch from 1fc4db2 to 31d1af1 Compare May 22, 2026 21:15
deustis added 3 commits May 26, 2026 15:17
Binds S2Cell's constructors, factory methods, geometric accessors
(vertex, edge, center, area), boundary helpers, containment and
intersection predicates, and subdivide. Adds 43 unit tests.

Distance methods and the S2Cap/S2LatLngRect-returning region methods
are deferred with a TODO; they depend on S1ChordAngle/S2Cap/S2LatLngRect
bindings which are not yet in place.

(Stacked on deustis/s2cell_id_bindings / PR google#593.)
- Drop MaybeThrowNotValidCellId from S2Cell(S2CellId) constructor; all
  S2CellId objects reachable from Python are already validated
- Remove instance average_area(); keep only static average_area_for_level()
- Add S2CellId::kMaxPosition constant and validate pos in from_face_pos_level
  for both S2CellId and S2Cell bindings
- Switch constants to cls.attr() pattern in both S2CellId and S2Cell bindings;
  update README to document this style
- Fix BUILD: add absl/strings dep to s2cell_id_bindings
- Remove misleading encode/decode comment from s2cell_bindings.cc
- Fix magic constant 1<<30 -> 1<<S2CellId.MAX_LEVEL in s2cell_test.py
Consistent with other MaybeThrow* helpers in both binding files.
@deustis deustis force-pushed the deustis/s2cell_bindings branch from 31d1af1 to 677224f Compare May 26, 2026 15:17
@deustis deustis changed the base branch from deustis/s2cell_id_binding_fixes to master May 26, 2026 15:30
Comment thread src/python/module.cc Outdated
Comment thread src/python/s2cell_bindings.cc Outdated
Comment thread src/python/s2cell_bindings.cc Outdated
Comment thread src/python/s2cell_bindings.cc
- Fix s2cell deps comment in module.cc to include s1chord_angle
- Clarify orientation docstring (bitmask, values 0-3)
- Drop _raw variants (vertex_raw, edge_raw, center_raw): Python callers
  don't feed unnormalized vectors into exact C++ predicates
Comment thread src/python/s2cell_test.py Outdated
# Orientation is in [0, 3].
for f in range(6):
orient = s2.S2Cell.from_face(f).orientation
self.assertIn(orient, range(4))
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assert exact (or floating point approx) values in tests

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done — test now asserts face cells have orientation 0, with a range check on a non-face cell.

Comment thread src/python/s2cell_test.py Outdated
self.assertAlmostEqual(v0.y, v4.y)
self.assertAlmostEqual(v0.z, v4.z)

def test_vertex_raw(self):
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought we removed these _raw variants

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed.

Comment thread src/python/s2cell_test.py Outdated
# Face cell in (i,j) spans [0, 2^30].
bottom = face.ij_coord_of_edge(s2.S2Cell.BOTTOM_EDGE)
top = face.ij_coord_of_edge(s2.S2Cell.TOP_EDGE)
self.assertIn(bottom, (0, 1 << 30))
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assert a value

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done — now asserts BOTTOM=0, TOP=2^30, LEFT=0, RIGHT=2^30.

Comment thread src/python/s2cell_test.py Outdated
with self.assertRaises(ValueError):
s2.S2Cell.average_area_for_level(-1)

def test_approx_area_positive(self):
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this test is not useful, assert an approximate value within the error tolerance

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done — now checks the face cell area is within 3% of 4π/6.

Comment thread src/python/s2cell_bindings.cc Outdated
.def("may_intersect", &S2Cell::MayIntersect, py::arg("cell"),
"Return true if this cell may intersect the given cell")

// Traversal
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's get rid of traversal section and move this to geometric operations (also in tests)

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done — Traversal section removed, subdivide moved into Geometric operations.

Comment thread src/python/module.cc Outdated
// Deps: s1angle, s2point, s2latlng, r2point, r2rect
bind_s2cell_id(m);

// Deps: s2cell_id, s2point, s2latlng, s1chord_angle, r2rect
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make the deps order match the declaration order above

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — now r2rect, s1chord_angle, s2cell_id, s2latlng, s2point.

@deustis
Copy link
Copy Markdown
Owner Author

deustis commented May 26, 2026

Superseded by google#631.

@deustis deustis closed this May 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant