zhangfengcdt commented on code in PR #1524:
URL: https://github.com/apache/sedona/pull/1524#discussion_r1681871245


##########
docs/api/sql/Raster-operators.md:
##########
@@ -452,14 +452,14 @@ SELECT RS_GeoReferrence(ST_MakeEmptyRaster(1, 3, 4, 
100.0, 200.0,2.0, -3.0, 0.1,
 
 ### RS_GeoTransform
 
-Introduction: Returns an array of parameters that represent the 
GeoTransformation of the raster. The array contains the following values:
+Introduction: Returns a struct of parameters that represent the 
GeoTransformation of the raster. The struct has the following schema:
 
-- 0: pixel width along west-east axis (x-axis)
-- 1: pixel height along north-south axis (y-axis)
-- 2: Rotation of the raster
-- 3: Angular separation between x-axis and y-axis
-- 4: X ordinate of upper-left coordinate
-- 5: Y ordinate of upper-left coordinate
+- magnitudeI: pixel width along west-east axis (x-axis)

Review Comment:
   Looks like the original (before this change) descriptions are not correct 
and consistent with the comments in the code for geoTransform function. 
   
   I am updating the document to use the ones from the comments.
   
   ```
   - magnitudeI: size of a pixel along the transformed i axis
   - magnitudeJ: size of a pixel along the transformed j axis
   - thetaI: angle by which the raster is rotated (Radians positive clockwise)
   - thetaIJ: angle from transformed i axis to transformed j axis (Radians 
positive counter-clockwise)
   - offsetX: X ordinate of the upper-left corner of the upper-left pixel
   - offsetY: Y ordinate of the upper-left corner of the upper-left pixel
   ```
   
   



##########
spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/raster/RasterFunctions.scala:
##########
@@ -0,0 +1,259 @@
+/*

Review Comment:
   The reason why I create new scala wrappers is that if we change the Java 
function (RS_Metadata), since they are under the sedona-common module, it does 
not have dependency for spark. However, in order to return struct, we need 
   ```
   import org.apache.spark.sql.types.{AbstractDataType, BooleanType, DataType, 
DoubleType, IntegerType, StructField, StructType}
   ```
   
   I think it is not appropriate to introduce spark dependencies to the 
sedona-common module. The wrapper is also consistent with some other RS 
functions in PixelFunctions for example.



-- 
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]

Reply via email to