[Contribution] Serialization support for `s2_geography`

[tzakharko]

In the current implementation, s2_geography objects are wrappers over external pointers. This means that serializing and unserializing the data (e.g. when doing saveRDS()) will result in a segfault (see also #259). While s2 data is originally intended to be an intermediate format for processing, the ability to store and load s2 objects would make the package more useful and prevent confusing bugs.

I would like to implement an initial implementation of serialization support and wanted to inquire whether the maintainers would be interesting in accepting such a contribution. The idea would be to wrap s2_geography as a shallow ALTREP class implementing serialization methods.

[paleolimbot]

Thank you for opening!

Reprex:

library(s2)

geog <- as_s2_geography("POINT (0 1)")
unserialize(serialize(geog, NULL))
# crash!

The failure mode should probably be an error and not a crash:

library(geos)

geom <- as_geos_geometry("POINT (0 1)")
unserialize(serialize(geom, NULL))
#> <geos_geometry[1]>
#> Error in geos_num_coordinates(x): External pointer is not valid [i=1]

I wonder if it would work for your use case to serialize a different object?

library(s2)

geog <- as_s2_geography("POINT (0 1)")
geog_to_serialize <- wk::as_wkb(geog)
as_s2_geography(unserialize(serialize(geog_to_serialize, NULL)))
#> <geodesic s2_geography[1] with CRS=OGC:CRS84>
#> [1] POINT (0 1)

We could also implement a refHook for readRDS or something fanicer like the ALTREP you described; however, that's a bit complicated/verbose for use here I think. Perhaps you could share your use case?

[tzakharko]

My use case is that we extensively rely on s2 routines for spherical geometry manipulation and it is cumbersome to always convert the data to a serializable format. In particular, the inability to deserialize s2 data interferes with caching or storing them in nested structures. I'd like to improve the user experience when working with s2 geometry as a primary data format.

Refhooks don't address the issue unfortunately, since the user needs to specify them manually, so the user experience does not change much. What we really need is serializable extptrs, but that would require an overhaul of the base R implementation, which is a much larger effort.

Would you be opposed to me submitting a pull request adding serialization support to s2_geography via ALTREP? This seems like the way to go, and the patch itself should be fairly straightforward. There are only a handful of trivial methods to implement. For serialization one could convert the data into wkb.

[paleolimbot]

Very cool!

Would you be opposed to me submitting a pull request adding serialization support to s2_geography via ALTREP?

I'm certainly happy to review, although I admit that I am skeptical. Perhaps it can be done in a way that the implementation lives in its own files (e.g., s2-geography-altrep.R, s2-geography-altrep.cpp) and marked as experimental?

[tzakharko]

I finally had a bit of time to look into this more seriously this morning. I have submitted an initial implementation in #283, would be great if you could have a look. All tests are passing and I am unable to detect any measurable performance overhead.