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

Add compat for unpickling shapely<2 geometries #1657

Merged
merged 6 commits into from Dec 11, 2022
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGES.txt
@@ -1,6 +1,13 @@
Changes
=======


2.0rc4 (2022-10-26)
-------------------

- Added temporary for unpickling shapely<2.0 geometries.


2.0rc1 (2022-10-26)
-------------------

Expand Down
84 changes: 82 additions & 2 deletions src/pygeom.c
Expand Up @@ -38,14 +38,14 @@ PyObject* GeometryObject_FromGEOS(GEOSGeometry* ptr, GEOSContextHandle_t ctx) {
} else {
self->ptr = ptr;
self->ptr_prepared = NULL;
self->weakreflist = (PyObject *)NULL;
self->weakreflist = (PyObject*)NULL;
return (PyObject*)self;
}
}

static void GeometryObject_dealloc(GeometryObject* self) {
if (self->weakreflist != NULL) {
PyObject_ClearWeakRefs((PyObject *)self);
PyObject_ClearWeakRefs((PyObject*)self);
}
if (self->ptr != NULL) {
// not using GEOS_INIT, but using global context instead
Expand Down Expand Up @@ -291,7 +291,87 @@ static PyObject* GeometryObject_richcompare(GeometryObject* self, PyObject* othe
return result;
}


static PyObject* GeometryObject_SetState(PyObject* self, PyObject* value) {
unsigned char* wkb = NULL;
Py_ssize_t size;
GEOSGeometry* geom = NULL;
GEOSWKBReader* reader = NULL;

PyErr_WarnFormat(PyExc_UserWarning, 0,
"Unpickling a shapely <2.0 geometry object. Please save the pickle "
"again; shapely 2.1 will not have this compatibility.");

/* Cast the PyObject bytes to char */
if (!PyBytes_Check(value)) {
PyErr_Format(PyExc_TypeError, "Expected bytes, found %s", value->ob_type->tp_name);
return NULL;
}
size = PyBytes_Size(value);
wkb = (unsigned char*)PyBytes_AsString(value);
if (wkb == NULL) {
return NULL;
}

PyObject* linearring_type_obj = PyList_GET_ITEM(geom_registry[0], 2);
if (linearring_type_obj == NULL) {
return NULL;
}
if (!PyType_Check(linearring_type_obj)) {
PyErr_Format(PyExc_RuntimeError, "Invalid registry value");
return NULL;
}
PyTypeObject* linearring_type = (PyTypeObject*)linearring_type_obj;

GEOS_INIT;

reader = GEOSWKBReader_create_r(ctx);
if (reader == NULL) {
errstate = PGERR_GEOS_EXCEPTION;
goto finish;
}
geom = GEOSWKBReader_read_r(ctx, reader, wkb, size);
if (geom == NULL) {
errstate = PGERR_GEOS_EXCEPTION;
goto finish;
}
if (Py_TYPE(self) == linearring_type) {
const GEOSCoordSequence* coord_seq = GEOSGeom_getCoordSeq_r(ctx, geom);
if (coord_seq == NULL) {
errstate = PGERR_GEOS_EXCEPTION;
goto finish;
}
geom = GEOSGeom_createLinearRing_r(ctx, (GEOSCoordSequence*)coord_seq);
if (geom == NULL) {
errstate = PGERR_GEOS_EXCEPTION;
goto finish;
}
}

if (((GeometryObject*)self)->ptr != NULL) {
GEOSGeom_destroy_r(ctx, ((GeometryObject*)self)->ptr);
}
((GeometryObject*)self)->ptr = geom;

finish:

if (reader != NULL) {
GEOSWKBReader_destroy_r(ctx, reader);
}

GEOS_FINISH;

if (errstate == PGERR_SUCCESS) {
Py_INCREF(Py_None);
return Py_None;
}
return NULL;
}


static PyMethodDef GeometryObject_methods[] = {
{"__setstate__", (PyCFunction)GeometryObject_SetState, METH_O,
"For unpickling pre-shapely 2.0 pickles"},
{NULL} /* Sentinel */
};

Expand Down
Binary file added tests/data/emptypoint_1.8.5.post1.pickle
Binary file not shown.
Binary file added tests/data/emptypolygon_1.7.1.pickle
Binary file not shown.
Binary file added tests/data/emptypolygon_1.8.5.post1.pickle
Binary file not shown.
Binary file added tests/data/geometrycollection_1.8.5.post1.pickle
Binary file not shown.
Binary file added tests/data/linearring_1.8.5.post1.pickle
Binary file not shown.
Binary file added tests/data/linestring_1.8.5.post1.pickle
Binary file not shown.
Binary file added tests/data/multilinestring_1.8.5.post1.pickle
Binary file not shown.
Binary file added tests/data/multipoint_1.8.5.post1.pickle
Binary file not shown.
Binary file added tests/data/multipolygon_1.8.5.post1.pickle
Binary file not shown.
Binary file added tests/data/point2d_1.8.5.post1.pickle
Binary file not shown.
Binary file added tests/data/point3d_1.8.5.post1.pickle
Binary file not shown.
Binary file added tests/data/polygon_1.8.5.post1.pickle
Binary file not shown.
97 changes: 83 additions & 14 deletions tests/test_pickle.py
@@ -1,24 +1,93 @@
import pathlib
import pickle
from pickle import dumps, loads, HIGHEST_PROTOCOL
import warnings

import shapely
from shapely.geometry import Point, LineString, LinearRing, Polygon, MultiLineString, MultiPoint, MultiPolygon, GeometryCollection, box
from shapely import wkt


import pytest
from shapely.geometry import Point, LineString, LinearRing, Polygon, MultiPoint

from pickle import dumps, loads, HIGHEST_PROTOCOL

TEST_DATA = {
"point2d": (Point, [(1.0, 2.0)]),
"point3d": (Point, [(1.0, 2.0, 3.0)]),
"linestring": (LineString, [(0.0, 0.0), (0.0, 1.0), (1.0, 1.0)]),
"linearring": (LinearRing, [(0.0, 0.0), (1.0, 0.0), (1.0, 1.0), (0.0, 0.0)]),
"polygon": (Polygon, [(0.0, 0.0), (1.0, 0.0), (1.0, 1.0), (0.0, 0.0)]),
"multipoint": (MultiPoint, [(1.0, 2.0), (3.0, 4.0), (5.0, 6.0)]),
"point2d": Point([(1.0, 2.0)]),
"point3d": Point([(1.0, 2.0, 3.0)]),
"linestring": LineString([(0.0, 0.0), (0.0, 1.0), (1.0, 1.0)]),
"linearring": LinearRing([(0.0, 0.0), (1.0, 0.0), (1.0, 1.0), (0.0, 0.0)]),
"polygon": Polygon([(0.0, 0.0), (1.0, 0.0), (1.0, 1.0), (0.0, 0.0)]),
"multipoint": MultiPoint([(1.0, 2.0), (3.0, 4.0), (5.0, 6.0)]),
"multilinestring": MultiLineString([[(0.0, 0.0), (1.0, 1.0)], [(1.0, 2.0), (3.0, 3.0)]]),
"multipolygon": MultiPolygon([box(0, 0, 1, 1), box(2, 2, 3, 3)]),
"geometrycollection": GeometryCollection([Point(1.0, 2.0), box(0, 0, 1, 1)]),
"emptypoint": wkt.loads("POINT EMPTY"),
"emptypolygon": wkt.loads("POLYGON EMPTY"),
}
TEST_NAMES, TEST_DATA = zip(*TEST_DATA.items())
@pytest.mark.parametrize("cls,coords", TEST_DATA, ids=TEST_NAMES)
def test_pickle_round_trip(cls, coords):
geom1 = cls(coords)
assert geom1.has_z == (len(coords[0]) == 3)
TEST_NAMES, TEST_GEOMS = zip(*TEST_DATA.items())


@pytest.mark.parametrize("geom1", TEST_GEOMS, ids=TEST_NAMES)
def test_pickle_round_trip(geom1):
data = dumps(geom1, HIGHEST_PROTOCOL)
geom2 = loads(data)
with warnings.catch_warnings():
warnings.simplefilter("error")
geom2 = loads(data)
assert geom2.has_z == geom1.has_z
assert type(geom2) is type(geom1)
assert geom2.geom_type == geom1.geom_type
assert geom2.wkt == geom1.wkt


SHAPELY_18_PICKLE = (
b'\x80\x04\x95\x8c\x00\x00\x00\x00\x00\x00\x00\x8c\x18shapely.geometry.polygon'
b'\x94\x8c\x07Polygon\x94\x93\x94)R\x94C]\x01\x03\x00\x00\x00\x01\x00\x00\x00'
b'\x05\x00\x00\x00\x00\x00\x00\x00\x00\x00$@\x00\x00\x00\x00\x00\x00\x00\x00'
b'\x00\x00\x00\x00\x00\x00$@\x00\x00\x00\x00\x00\x00$@\x00\x00\x00\x00\x00\x00'
b'\x00\x00\x00\x00\x00\x00\x00\x00$@\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00'
b'\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00$@\x00\x00\x00\x00\x00\x00\x00\x00'
b'\x94b.'
)


def test_unpickle_shapely_18():
from shapely.testing import assert_geometries_equal

with pytest.warns(UserWarning):
geom = loads(SHAPELY_18_PICKLE)
assert_geometries_equal(geom, box(0, 0, 10, 10))


HERE = pathlib.Path(__file__).parent

@pytest.mark.parametrize("fname", (HERE / "data").glob("*.pickle"), ids=lambda fname: fname.name)
def test_unpickle_pre_20(fname):
from shapely.testing import assert_geometries_equal

geom_type = fname.name.split("_")[0]
expected = TEST_DATA[geom_type]

with open(fname, "rb") as f:
with pytest.warns(UserWarning):
result = pickle.load(f)

if geom_type == "emptypolygon" and "1.7.1" in fname.name:
expected = wkt.loads("POLYGON Z EMPTY")
Copy link
Member

Choose a reason for hiding this comment

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

I could also just leave out this case from the files (so just test with the generated pickle data from 1.8.5)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That would be fine with me! I think these tests are already pretty thorough.
And my test above appears to be obsolete now? Feel free to remove it

Copy link
Member

Choose a reason for hiding this comment

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

Yes, cleaned up

assert_geometries_equal(result, expected)


if __name__ == "__main__":
HERE = pathlib.Path(__file__).parent
datadir = HERE / "data"
datadir.mkdir(exist_ok=True)

shapely_version = shapely.__version__
print(shapely_version)
print(shapely.geos.geos_version)

for name, geom in TEST_DATA.items():
if name == "emptypoint" and shapely.geos.geos_version < (3, 9, 0):
# Empty Points cannot be represented in WKB
continue
with open(datadir / f"{name}_{shapely_version}.pickle", "wb") as f:
pickle.dump(geom, f)