georust / geozero

Zero-Copy reading and writing of geospatial data.
Apache License 2.0
336 stars 33 forks source link

wkt reader #20

Closed michaelkirk closed 2 years ago

michaelkirk commented 2 years ago

Fixes #19

I have a couple things I was hoping to get input on before submitting for actual review... questions inline. Update: OK! Ready for review!

Definitely some weirdness around handling POINT EMPTY, but I went with what felt to me like a reasonable compromise of trying to:

pka commented 2 years ago

Looks like we can't avoid going into that rabbit hole. At least we're not alone (https://github.com/locationtech/jts/pull/567 https://trac.osgeo.org/geos/ticket/1005). Looks like your solution with an explicit empty_point is the cleanest one. But in the end we would probably need this for all geometry types?

Anyway let's merge this and improve handling of empty geometries seperately. Thanks a lot!

michaelkirk commented 2 years ago

in the end we would probably need this for all geometry types?

Luckily, I think POINT EMPTY is kind of an exception. It seems like we can more reasonably handle other empty geometries like MULTIPOINT EMPTY, LINESTRING EMPTY, etc. without exploding.

I've expounded on this here: https://github.com/georust/geozero/pull/21