feat(rust/sedona-geometry): Let CrsTransform handle M coordinate#619
feat(rust/sedona-geometry): Let CrsTransform handle M coordinate#619paleolimbot merged 1 commit intoapache:mainfrom
Conversation
paleolimbot
left a comment
There was a problem hiding this comment.
Thank you!
I think this is correct...perhaps a good auxiliary check for this would be to ensure that ST_Translate has tests for the case where the XYM geometries are input and there is a deltaZ, ensuring that the M values are untouched (I think these tests already exist). I suppose we don't have a deltaM argument there, but maybe we should add one and test that Z values are untouched for XYZ and XYZM input (not for this PR since I see the tests here).
I also wonder if we've made the transform slower, but we have at least one optimization that we haven't implemented yet (transform arrays instead of scalar coordinates) and maybe that's the time to check in. Getting the default dimension math right is more important (we can always optimize easy cases later).
|
Thanks!
This was my main concern. But, I agree with you that we can optimize later. |
This is a followup of #606. Before addressing
ST_Force3DM()andST_Force4D(), I want to confirm if my approach is correct.This pull request changes
CrsTransformto have two additional methods to handle M coordinate (accordingly,transform_coord_3d()is renamed totransform_coord_xyz). Maybe I could create a separatetransform_m()method, but I thought it's better to do the transformation in a single call.transform_xym()transform_xyzm()This also requires
transform_and_write_coords()to extract and fill M coordinate.