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

Support pickling Geography objects #54

Open
jorisvandenbossche opened this issue Oct 9, 2024 · 3 comments
Open

Support pickling Geography objects #54

jorisvandenbossche opened this issue Oct 9, 2024 · 3 comments
Milestone

Comments

@jorisvandenbossche
Copy link
Collaborator

At the moment we didn't implement this, so you will get TypeError: cannot pickle 'spherely.Point' object, but it would be useful to support it.

To provide a full exact roundtrip through pickling, we should maybe pickle the raw unit vector (x, y, z) of the S2Point, instead of going through latitude/longitude like other conversions (eg WKB), because there can always be some floating point difference in this conversion.

@benbovy
Copy link
Owner

benbovy commented Oct 10, 2024

S2geometry provides Encoder and Decoder objects as well as Encode() and Decode() methods from most of its data types (point, polyline, polygon) so we might be able to just reuse that. For a point it serializes the raw unit vector. It is also possible to encode S2ShapeIndex objects with lazy decoding.

Probably cleanest would be to expose this API via s2geography::Geography wrapper classes rather than dealing with it directly in spherely (same for other methods like Clone(), which would be better than this function implemented in #51).

@benbovy
Copy link
Owner

benbovy commented Oct 10, 2024

Actually, I think we could already access Encode() and Decode() via s2geography::Geography::Shape().

@benbovy benbovy added this to the 0.1.0 milestone Nov 14, 2024
@benbovy
Copy link
Owner

benbovy commented Nov 14, 2024

paleolimbot/s2geography#40 is now merged so adding support for pickle should be easier.

That s2geography PR also added the GeographyKind enum tag, which will allow us to clean up some parts of the code in Spherely (e.g., remove some dynamic_cast).

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