Skip to content

Added srid keyword to wkb.dumps. Closes #592.#593

Merged
sgillies merged 2 commits intoshapely:masterfrom
snorfalorpagus:wkb_srid
May 28, 2018
Merged

Added srid keyword to wkb.dumps. Closes #592.#593
sgillies merged 2 commits intoshapely:masterfrom
snorfalorpagus:wkb_srid

Conversation

@snorfalorpagus
Copy link
Copy Markdown
Member

Added the srid keyword argument to wkb.dumps to include an SRID in WKB data, as suggested in #592.

I've added some tests for WKB in test_wkb.py (to match test_wkt.py). There are already some in test_persist.py which I've left alone. Perhaps this could be expanded in the future, but it's not essential.

An observation is that pickle uses wkb.dumps to store data. This defaults to not exporting the SRID (even if one is present in the geometry after loading with loads. This means that the round-trip for a geometry with an SRID isn't perfect. However I think this is acceptable as generally we're ignoring the SRID in Shapely.

@snorfalorpagus snorfalorpagus requested a review from sgillies May 28, 2018 15:36
@coveralls
Copy link
Copy Markdown

coveralls commented May 28, 2018

Coverage Status

Coverage increased (+1.9%) to 80.384% when pulling 89bd078 on snorfalorpagus:wkb_srid into ad53583 on Toblerity:master.

geom = lgeos.GEOSGeom_clone(ob._geom)
lgeos.GEOSSetSRID(geom, srid)
ob = geom_factory(geom)
kw["include_srid"] = True
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Copy Markdown
Contributor

@sgillies sgillies left a comment

Choose a reason for hiding this comment

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

@snorfalorpagus can you add an entry to the change log? This is a feature that interested users shouldn't overlook.

@sgillies sgillies merged commit 4433a84 into shapely:master May 28, 2018
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

Successfully merging this pull request may close these issues.

3 participants