Conversation
| } | ||
|
|
||
| fn to_ewkt(&self, srid: Option<i32>) -> Result<String> { | ||
| self.to_wkt_with_opts(WktDialect::Ewkt, CoordDimensions::xyzm(), srid) |
There was a problem hiding this comment.
I'm not sure the right way to fix this, but as I understand it, setting CoordDimensions says "this output will have these given geometries", and so defaulting to xyzm seems incorrect.
| let geojson = r#"{"type": "LineString", "coordinates": [[1,1],[2,2]]}"#; | ||
| let mut wkt_data: Vec<u8> = Vec::new(); | ||
| let mut out = WktWriter::with_dims(&mut wkt_data, CoordDimensions::xyz()); | ||
| let mut out = WktWriter::with_dims(&mut wkt_data, CoordDimensions::xy()); |
There was a problem hiding this comment.
I'm not sure what this is supposed to test. Is it supposed to test that setting xyz doesn't matter? I thought at first below was a typo and that the LineString was meant to be 3d, but this entire test is otherwise the exact same as the one before it.
| let ewkt = Ewkb(blob) | ||
| .to_wkt_with_opts(WktDialect::Ewkt, CoordDimensions::xyz(), Some(4326)) | ||
| .unwrap(); | ||
| assert_eq!(ewkt, "SRID=4326;MULTIPOINT Z(10 -20 100,0 -0.5 101)") |
There was a problem hiding this comment.
Similarly, the API just isn't sufficient... There's no way to get the coordinate dimension from the EWKB without consuming it. And so there's no way right now to know the SRID and CoordDimensions of the input and pass that on to the writer at the beginning.
michaelkirk
left a comment
michaelkirk
left a comment
There was a problem hiding this comment.
These changes seem reasonable, but I didn't write the original code, so I'm not sure of the original motivations.
|
That failing test looks significant: |
|
My initial reaction is that it's really hard to fix ZM handling in isolation because it needs some bigger architectural changes to geozero |
I don't doubt it! It leaves me not sure what to do about this PR though. |
michaelkirk
dismissed
their stale review
Not sure what to do about that failing test.
### Change list - Convert geo-traits scalars to `wkt::Wkt` - This is prep work for direct transformation of geoarrow scalars to `wkt::Wkt` objects. - This does not go through geozero, partially because I hit issues with multidimensional writing in georust/geozero#237 - This supersedes #788, though it hasn't yet piped through to a full `ToWkt` trait implemented on arrays. cc @b4l
2 participants
No description provided.