[SEDONA-428] Add RS_ZonalStats & RS_ZonalStatsAll #1124
Conversation
…ter's extent" This reverts commit b7a15b4.
common/src/main/java/org/apache/sedona/common/raster/RasterBandAccessors.java
Show resolved
Hide resolved
|
|
||
| for (int k = 0; k < rasterPixelData.length; k++) { | ||
|
|
||
| if (rasterizedPixelData[k] == 0 || excludeNoData && noDataValue != null && rasterPixelData[k] == noDataValue) { |
There was a problem hiding this comment.
Where does 0 come from? Why skip it?
There was a problem hiding this comment.
If you look at that line. The output from asRaster is that it will set 150 to the pixels that are under the geometry and will set 0 to all the other pixels. So I am skipping the 0 is because those pixels are not under the geometry.
|
@Kontinuation All raster functions that take a raster and a geometry as an input, we must perform implicit CRS transformation. What do you think? |
I think performing implicit CRS transformation could be better. Functions such as RS_Value, RS_Values may also need CRS transformation. The geometry needs to be transformed to the CRS of the raster. However, such transformations may be impossible or produce incorrect results due to the valid region of CRS. We have to assume that the geometry specified in the query is always able to be properly transformed to the raster's CRS. |
| @@ -26,6 +26,7 @@ | |||
| import org.geotools.gce.geotiff.GeoTiffReader; | |||
| import org.geotools.geometry.Envelope2D; | |||
| import org.geotools.geometry.jts.JTS; | |||
There was a problem hiding this comment.
Why does this PR involve changes related to AsRaster? Is this a API breaking change?
If this change is not related to zonal statistics. It must be placed in a separate PR.
There was a problem hiding this comment.
No, it doesn't affect the current RS_AsRaster function. This refactoring was done to remove the pixel coordinate translation and use the output from the AsRasterWithRasterExtent function instead. We talked about it in our tag-up last Thursday to improve performance.
It is related to Zonal statistics.
| String datatype = RasterBandAccessors.getBandType(raster, band); | ||
| Double noDataValue = RasterBandAccessors.getBandNoDataValue(raster, band); | ||
| // Adding an arbitrary value '150' for the pixels that are under the geometry. | ||
| GridCoverage2D rasterizedGeom = RasterConstructors.asRasterWithRasterExtent(roi, raster, datatype, 150, null); |
There was a problem hiding this comment.
It is just a placeholder value to remove the pixels that aren't under the given geometry roi
There was a problem hiding this comment.
@furqaankhan Will it collide with existing values in a raster? If not, we are good.
There was a problem hiding this comment.
No no. It just to determine which values to use for compute from the provided raster.
common/src/test/java/org/apache/sedona/common/raster/RasterBandAccessorsTest.java
Show resolved
Hide resolved
docs/api/sql/Raster-operators.md
Outdated
| !!!note | ||
| The following conditions will throw an `IllegalArgumentException` if they are not met: | ||
|
|
||
| - The `raster` and `zone` geometry should be in the same CRS, refer to [RS_SRID](#rs_srid) for raster, and [ST_SRID](../Function/#st_srid) for geometry to get the spatial reference identifier. |
There was a problem hiding this comment.
Please explain what happens if either side does not have CRS or different CRS.
There was a problem hiding this comment.
My bad, will change.
Did you read the Contributor Guide?
Is this PR related to a JIRA ticket?
[SEDONA-XXX] my subject.What changes were proposed in this PR?
How was this patch tested?
Did this PR include necessary documentation updates?