Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue with exact equality and roundtrip of lat/lng coordinates #40

Open
jorisvandenbossche opened this issue Aug 19, 2024 · 1 comment

Comments

@jorisvandenbossche
Copy link
Collaborator

Something I ran into while testing #20 (xref #20 (comment)), is that it is not that easy to pass an exact equality test with equals().

Illustration:

# construct a point through centroid to get something that is not exactly Point(1, 0)
>>> point = spherely.centroid(spherely.LineString([(0, 0), (0, 2)]))
>>> point
POINT (0.9999999999999998 0)

# reconstruct this point through its lat/long coordinates
>>> point2 = spherely.Point(spherely.get_y(point), spherely.get_x(point))
>>> point2
POINT (0.9999999999999998 0)

# those two points (with the same WKT repr and equal coordinates in degrees) are not considered equal
>>> spherely.equals(point, point2)
False

What I assume is happening here is that the roundtrip from S2Point to S2LatLng and back is not exact (the conversion from S2Point to LatLng essentially happens in the WKT repr or in get_x/get_y, and the conversion from S2LatLng to S2Point is essentially happening in the Point(..) constructor.

While that might be expected based on knowledge of the S2 internals (how a S2Point is stored, which is not as its lon/lat coords), I suppose this will not be expected in general by users? And it also gives some usability issues (also when testing)

For equals() specifically, it might be possible to have an option there to snap round coordinates while checking equality.

@benbovy
Copy link
Owner

benbovy commented Aug 21, 2024

The implementation of equals ultimately relies on s2geometry's S2BooleanOperation::Equals, which provides a precision option. Unfortunately Precision.SNAPPED is not yet implemented, but I guess it would still be possible to add some tolerance by passing a snap function to the Options constructor (e.g., IntLatLngSnapFunction or IdentitySnapFunction with a given snap_radius). We could do that in Spherely by customizing

S2BooleanOperation::Options m_options;

In terms of API, I see that shapely also provides equals_exact. I'm not sure how they behave exactly for point geometries, though. Would it make sense in spherely for equals to have some internal minimal tolerance set by default so that the example above works, and provide a separate equals_exact function that accepts a user-defined tolerance?

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

No branches or pull requests

2 participants