Support 3d geometry #12
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
There are still quite some tests failing. In the |
|
Yeah, sorry for the tests, I will rewrite them without having to use |
visr
approved these changes
Dec 19, 2022
|
Thanks. There's on thing to improve and that is to create real ArchGDAL Z geometries to test with (skipped for now). Actually passing three coordinates per point doesn't do the trick. |
asinghvi17
reviewed
Mar 19, 2024
Comment on lines
+20
to
+33
| ("Point", ArchGDAL.createpoint(coord), false, false), | ||
| ("PointZ", ArchGDAL.createpoint(coord3), true, true), | ||
| ("LineString", ArchGDAL.createlinestring(coord, lcoord), false, false), | ||
| ("LineStringZ", ArchGDAL.createlinestring(coord3, lcoord3, lcoord3), true, true), | ||
| ("Polygon", ArchGDAL.createpolygon(coords), false, false), | ||
| ("PolygonZ", ArchGDAL.createpolygon(coords3), true, true), | ||
| ("MultiPoint", ArchGDAL.createmultipoint([coord, lcoord]), false, false), | ||
| ("MultiPointZ", ArchGDAL.createmultipoint([coord3, lcoord3]), true, true), | ||
| ("MultiLineString", ArchGDAL.createmultilinestring([[coord, lcoord], [coord, lcoord]]), false, false), | ||
| ("MultiLineStringZ", ArchGDAL.createmultilinestring([[coord3, lcoord3], [coord3, lcoord3]]), true, true), | ||
| ("MultiPolygon", ArchGDAL.createmultipolygon([coords, coords]), false, false), | ||
| ("MultiPolygonZ", ArchGDAL.createmultipolygon([coords3, coords3]), true, true), | ||
| ("Empty", ArchGDAL.createpoint(), false, false), | ||
| ("Empty Multi", ArchGDAL.createmultipolygon(), false, false) |
There was a problem hiding this comment.
Could these be replaced by GeoInterface wrapper types, since they encode directly whether Z and M are specifically present?
There was a problem hiding this comment.
They could be, but I mostly want to compare to GDAL and GEOS WKB/WKT functionality, so that would only introduce a layer of conversions.
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
However, it's not straightforward to be compatible with 3th-party packages.
There are several flavors of 2d+ WKT/WKB. WKT should print Z, M, ZM in their type, but GDAL omits this when possible and only provides it for M. ArchGDAL doesn't fully Z/M geometries, so this is sometimes hard to test. WKB has two ways of providing the geometry type, one with bitflags, one with x-thousands added to the type. We parse both, but only write the flagged one, which seems to be default. Interestingly, LibGEOS(.jl) doesn't seem to support either.
Also improved parsing of type with regex.
Closes #6