encoding/wkt: replace wkt parser with goyacc parser #197
Conversation
| @@ -57,5 +59,10 @@ func Marshal(g geom.T, applyOptFns ...EncodeOption) (string, error) { | |||
|
|
|||
| // Unmarshal translates a WKT to the corresponding geometry. | |||
| func Unmarshal(wkt string) (geom.T, error) { | |||
There was a problem hiding this comment.
i think in go-geom, we want to relax things we check in CockroachDB (subject to @twpayne guidance!) as go-geom takes a more "OGC spec" view of the world, namely:
a) geometrycollections can have mixed dimensions.
b) linestrings/polygons do not have to have 2/4 points respectively, and polygon rings don't have to be closed.
c) i think empty can be mixed willy nilly in MultiPoint.
we can choose to optionally allow these checks (but have them off by default). this should be done with options -- see how we did the EncodeOption for Marshal above. probably DecodeOptionRunPostGISCompatibilityChecks which enables all the checks we did above.
There was a problem hiding this comment.
a) geometrycollections can have mixed dimensions.
If the PostGIS compatibility option is not specified:
- Should a
GEOMETRYCOLLECTION{M, Z, ZM}be allowed to have geometries with different dimensions or does that only apply toGEOMETRYCOLLECTION? - Should the layout for a
GEOMETRYCOLLECTIONalways begeom.NoLayout? - If the answer to 1 is no, should the layout of a
*GeometryCollectionbe set if it is aGEOMETRYCOLLECTION{M, Z, ZM}or should it remain asgeom.NoLayout?
There was a problem hiding this comment.
Very good points! I've opened #200 to continue the discussion.
|
Thank you @andyyang890 - this is a fantastic contribution! I just back from a few days away but will review in detail in the next couple of days. |
|
Thank you so much for this @andyyang890! The code is excellent, and thank you in particularly for the detail in handling edge cases. I'll patch up the lint failure in a separate PR. |
|
Sounds good, thanks for the detailed review! |
Fixes #151.
Fixes #180.