You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sedona.apache.org by "iGN5117 (via GitHub)" <gi...@apache.org> on 2023/08/14 15:05:38 UTC

[GitHub] [sedona] iGN5117 opened a new pull request, #964: [DOCS] Refactor RS_AddBandFromArray

iGN5117 opened a new pull request, #964:
URL: https://github.com/apache/sedona/pull/964

   
   ## Did you read the Contributor Guide?
   
   - Yes, I have read [Contributor Rules](https://sedona.apache.org/latest-snapshot/community/rule/) and [Contributor Development Guide](https://sedona.apache.org/latest-snapshot/community/develop/)
   
   ## Is this PR related to a JIRA ticket?
   
   - No, this is a documentation update. The PR name follows the format `[DOCS] my subject`.
   
   
   ## What changes were proposed in this PR?
   - Refactor RS_BandFromArray to include typecasting info
   
   
   ## How was this patch tested?
   - Compiled documentation
   
   ## Did this PR include necessary documentation updates?
   - Yes, I have updated the documentation update.
   


-- 
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: dev-unsubscribe@sedona.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [sedona] iGN5117 commented on a diff in pull request #964: [DOCS] Refactor RS_AddBandFromArray

Posted by "iGN5117 (via GitHub)" <gi...@apache.org>.
iGN5117 commented on code in PR #964:
URL: https://github.com/apache/sedona/pull/964#discussion_r1293794670


##########
core/src/test/resources/raster/raster_with_no_data/test5.TIFF:
##########


Review Comment:
   Fixed



-- 
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: issues-unsubscribe@sedona.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [sedona] jiayuasu commented on a diff in pull request #964: [DOCS] Refactor RS_AddBandFromArray

Posted by "jiayuasu (via GitHub)" <gi...@apache.org>.
jiayuasu commented on code in PR #964:
URL: https://github.com/apache/sedona/pull/964#discussion_r1293782371


##########
core/src/test/resources/raster/raster_with_no_data/test5.TIFF:
##########


Review Comment:
   This file should not be added.



##########
docs/api/sql/Raster-operators.md:
##########
@@ -676,7 +676,7 @@ Introduction: Add a band to a raster from an array of doubles.
 
 Format: `RS_AddBandFromArray (raster: Raster, band: Array[Double])` | `RS_AddBandFromArray (raster: Raster, band: Array[Double], bandIndex:Int)` | `RS_AddBandFromArray (raster: Raster, band: Array[Double], bandIndex:Int, noDataValue:Double)`
 
-Since: `v1.4.1`
+Since: `v1.5.0`

Review Comment:
   You can separate the new APIs and old APIs by 
   
   ```
   Additional APIs since `v1.5.0`
   XXXX
   ```



##########
docs/api/sql/Raster-operators.md:
##########
@@ -692,6 +692,9 @@ In order to remove an existing noDataValue from an existing band, pass null as t
 
 Note that: `bandIndex == RS_NumBands(raster) + 1` is an experimental feature and might not lead to the loss of raster metadata and properties such as color models.
 
+!!!Note
+    RS_BandFromArray typecasts the double band values to the given datatype of the raster. This can lead to overflow values if values beyond the range of the raster's datatype are provided.

Review Comment:
   This should be `RS_AddBandFromArray`. What can users do to find the current data type in the raster?



##########
docs/api/sql/Raster-operators.md:
##########
@@ -692,6 +692,9 @@ In order to remove an existing noDataValue from an existing band, pass null as t
 
 Note that: `bandIndex == RS_NumBands(raster) + 1` is an experimental feature and might not lead to the loss of raster metadata and properties such as color models.

Review Comment:
   This should be `might lead to`



-- 
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: issues-unsubscribe@sedona.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [sedona] jiayuasu merged pull request #964: [DOCS] Refactor RS_AddBandFromArray

Posted by "jiayuasu (via GitHub)" <gi...@apache.org>.
jiayuasu merged PR #964:
URL: https://github.com/apache/sedona/pull/964


-- 
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: dev-unsubscribe@sedona.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [sedona] iGN5117 commented on a diff in pull request #964: [DOCS] Refactor RS_AddBandFromArray

Posted by "iGN5117 (via GitHub)" <gi...@apache.org>.
iGN5117 commented on code in PR #964:
URL: https://github.com/apache/sedona/pull/964#discussion_r1293785180


##########
docs/api/sql/Raster-operators.md:
##########
@@ -676,7 +676,7 @@ Introduction: Add a band to a raster from an array of doubles.
 
 Format: `RS_AddBandFromArray (raster: Raster, band: Array[Double])` | `RS_AddBandFromArray (raster: Raster, band: Array[Double], bandIndex:Int)` | `RS_AddBandFromArray (raster: Raster, band: Array[Double], bandIndex:Int, noDataValue:Double)`
 
-Since: `v1.4.1`
+Since: `v1.5.0`

Review Comment:
   This change is not version dependent, if a user loads a raster with some other data type, and tries to add a band, they will be typecasted.
   However, I changed the version because 1.5.0 onwards MakeEmptyRaster allows specifying data type.
   
   Also, once we can control supplying version's docs from the backend right, I didn't think I would need to specify version wise anything in the documentation



-- 
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: issues-unsubscribe@sedona.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [sedona] iGN5117 commented on a diff in pull request #964: [DOCS] Refactor RS_AddBandFromArray

Posted by "iGN5117 (via GitHub)" <gi...@apache.org>.
iGN5117 commented on code in PR #964:
URL: https://github.com/apache/sedona/pull/964#discussion_r1293792006


##########
docs/api/sql/Raster-operators.md:
##########
@@ -692,6 +692,9 @@ In order to remove an existing noDataValue from an existing band, pass null as t
 
 Note that: `bandIndex == RS_NumBands(raster) + 1` is an experimental feature and might not lead to the loss of raster metadata and properties such as color models.
 
+!!!Note
+    RS_BandFromArray typecasts the double band values to the given datatype of the raster. This can lead to overflow values if values beyond the range of the raster's datatype are provided.

Review Comment:
   Pushed the mistake fix.
   To get the data type for the given band, users can use RS_BandPixelType



-- 
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: issues-unsubscribe@sedona.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org