georust / wkt

Rust read/write support for well-known text (WKT)
https://crates.io/crates/wkt
Apache License 2.0
50 stars 25 forks source link

3D point is spelled `POINT Z` not `POINTZ` #114

Closed michaelkirk closed 3 months ago

michaelkirk commented 3 months ago

I recently merged #113, but I think it contains a critical error. I do not use 3-d points, so it slipped past me.

@ariesdevil - do you have interest in fixing this?

kylebarron commented 3 months ago

I thought the WKT spec allowed either POINTZ or POINT Z. I got a separate error from geozero when I passed in POINTZ

michaelkirk commented 3 months ago

Oh - I truly don't know.

Trying to find an actual spec for WKT is making me laugh out loud. I guess we should probably just "do whatever JTS does".

I found this https://loc.gov/preservation/digital/formats/fdd/fdd000548.shtml, which links to some hundred page documents, but they seem to be about WKT CRS, and doesn't include the string "POINTZ" or "POINT Z"

kylebarron commented 3 months ago

Trying to find an actual spec for WKT is making me laugh out loud. I guess we should probably just "do whatever JTS does".

Yeah I was just searching for a spec and similarly couldn't find one. I suppose the ISO one is the "official" one. But I think this GEOS definition is enough.

I found this loc.gov/preservation/digital/formats/fdd/fdd000548.shtml, which links to some hundred page documents, but they seem to be about WKT CRS, and doesn't include the string "POINTZ" or "POINT Z"

Yeah it is confusing that there's both WKT for geometries and for CRS. Sadly the original simple features spec only defines WKB not WKT.

kylebarron commented 3 months ago

I recently merged #91

I think you're referring to https://github.com/georust/wkt/pull/113?

kylebarron commented 3 months ago
image

Shapely (GEOS) doesn't support POINTZ, it only supports POINT Z. So I think it's fine in this crate to only accept the latter

kylebarron commented 3 months ago

At first glance it looks like only point supports Z/M, and no other geometry types support it, so we should expand that support as well

michaelkirk commented 3 months ago

Ok, I'm working on making this change.

kylebarron commented 3 months ago

Sorry I should've given you a heads up that I was looking into this. I put up #115

kylebarron commented 3 months ago

FWIW while I was reading through the GEOS code here, I saw that they do support both POINTZ and POINT Z. It seems strange they support both in GEOS when Shapely only supports the latter. But it looks like it produces POINT Z by default https://github.com/libgeos/geos/blob/main/include/geos/io/WKTWriter.h#L165-L179

michaelkirk commented 3 months ago

Interesting. My understanding is that POINT Z is the "correct" way and POINTZ is a common enough misspelling that it might be worth supporting reading it (but not writing it) some day in the future.

kylebarron commented 3 months ago

This can be closed from https://github.com/georust/wkt/pull/115