paleolimbot commented on code in PR #363: URL: https://github.com/apache/sedona-db/pull/363#discussion_r2561314205
########## c/sedona-geos/src/lib.rs: ########## @@ -37,7 +37,6 @@ mod st_minimumclearance_line; mod st_perimeter; mod st_polygonize; mod st_polygonize_agg; -mod st_reverse; Review Comment: Peter is right that for all the other ones we kept the original around. I think for this one either is OK because I'm fairly sure that the geo-traits version will be faster and I am not worried about correctness issues popping up (i.e., I'm confident our tests can cover the full range of expected input). > Shouldn't we move the sedona-functions to the bottom, since they are generally the fastest That's a great point. I don't think we've had a kernel yet that we implemented in both sedona-functions and sedona-geos, but I think we have to add sedona-functions first for the Documentation (i.e., the functions live there and in theory everything else is just kernels). We could split up sedona-functions so that sedona-geo-traits has the actual kernels and then we register it last; however, I think for today maybe we just keep what this PR did (remove the st_reverse we're pretty sure is slower). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
