You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Happy to pick this up. Looks like the wiring already existed in f895a59 on #646 before it got pulled back out in ebb7012 for scope reasons, I'll base the S2Cell.cap_bound() binding on that, updated for the get_ prefix drop from the later review round, plus the s2cell_bindings to s2cap_bindings BUILD dep and a matching test in s2cell_test.py.
Thanks for the feedback! I know your S2Region base-class PR will end up binding this too, through inheritance, so once deustis#11 lands, #654's .def() line becomes redundant. Rather than block a live bug fix on a bigger unreviewed refactor, I'd like to suggest implementing #654 now and let you drop the now-duplicate line when you rebase on top of master. However, I'm happy to close the PR and let you take the reigns if you like.
Activity
Shaurya2k06 commentedon Jul 17, 2026
Happy to pick this up. Looks like the wiring already existed in f895a59 on #646 before it got pulled back out in ebb7012 for scope reasons, I'll base the S2Cell.cap_bound() binding on that, updated for the get_ prefix drop from the later review round, plus the s2cell_bindings to s2cap_bindings BUILD dep and a matching test in s2cell_test.py.
cc @jmr
Shaurya2k06 commentedon Jul 17, 2026
Let me know if I've correctly identified this bug, and if its a required change!
cc @jmr @deustis
deustis commentedon Jul 17, 2026
@Shaurya2k06, happy for the help! I do have a draft PR for this over here:
deustis#11
The strategy there is to bind S2Region and declare it as the base class for S2Cell, S2LatLngRect, and S2Cap.
However this is currently blocked on #646
Edit: #646 just landed
Shaurya2k06 commentedon Jul 17, 2026
Thanks for the feedback! I know your S2Region base-class PR will end up binding this too, through inheritance, so once deustis#11 lands, #654's .def() line becomes redundant. Rather than block a live bug fix on a bigger unreviewed refactor, I'd like to suggest implementing #654 now and let you drop the now-duplicate line when you rebase on top of master. However, I'm happy to close the PR and let you take the reigns if you like.
deustis commentedon Jul 17, 2026
Oh, is there a live bug here? I didn't realize. By all means we can ago ahead with #654 if that's the case (I thought this was just a TODO).
Separately, I just put up #655 for the S2LatLngRect bindings, which would be the final pre-req before deustis#11