[SEDONA-388] Add RS_AsRaster #1011

furqaankhan · 2023-09-07T23:40:36Z

Did you read the Contributor Guide?

Yes, I have read Contributor Rules and Contributor Development Guide

Is this PR related to a JIRA ticket?

Yes, the URL of the associated JIRA ticket is https://issues.apache.org/jira/browse/SEDONA-388. The PR name follows the format [SEDONA-XXX] my subject.

What changes were proposed in this PR?

Add RS_AsRaster
Fix alphabetic order of Raster writer docs
Add gt-process-feature dependency to maven
Add new function RasterOutputs.toGeotiff for vizualizing a raster internally (Not exposed to the public)

How was this patch tested?

Passed added tests

Did this PR include necessary documentation updates?

Yes, I have updated the documentation update.

# Conflicts: # common/src/main/java/org/apache/sedona/common/raster/RasterConstructors.java

common/src/test/java/org/apache/sedona/common/raster/RasterConstructorsTest.java

spark/common/pom.xml

common/src/main/java/org/apache/sedona/common/raster/RasterConstructors.java

jiayuasu · 2023-09-08T01:53:28Z

common/src/main/java/org/apache/sedona/common/raster/RasterOutputs.java

+     * */
+    public static boolean toGeoTiff(GridCoverage2D raster, String filePath) {
+        File outputFile = new File(filePath);
+        GeoTiffFormat format = new GeoTiffFormat();


Please do not use the GeoTiff format here. Use the asGeoTiff function to get the bytes and directly write the bytes to the disk file.

And this function is not tested. Please add a test.

docs/api/sql/Raster-writer.md

common/src/main/java/org/apache/sedona/common/raster/RasterConstructors.java

common/src/test/java/org/apache/sedona/common/raster/RasterConstructorsTest.java

Kontinuation · 2023-09-09T02:13:47Z

common/src/main/java/org/apache/sedona/common/raster/RasterConstructors.java

+        double scaleX = Math.abs(metadata[4]), scaleY = Math.abs(metadata[5]);
+        if (geom.getGeometryType().equalsIgnoreCase(Geometry.TYPENAME_POINT)) {
+            bound = new Envelope2D(bound.getCoordinateReferenceSystem(), bound.getCenterX() - scaleX * 0.5, bound.getCenterY() - scaleY * 0.5, scaleX, scaleY);
+        }
+
+        int width = (int) bound.getWidth(), height = (int) bound.getHeight();
+
+        if (geom.getGeometryType().equalsIgnoreCase(Geometry.TYPENAME_POINT)) {
+            width = 1;
+            height = 1;
+        } else {
+            // To preserve scale of reference raster
+            width = (int) (width / scaleX);
+            height = (int) (height / scaleY);
+        }


There are some special treatments for points. I think it is also needed for horizontal and vertical line strings. We also need to add tests for these cases.

Actually, any kind of geometry can have a sub-pixel size. I suggest simply checking the width/height of the raster to determine if we need to extend its width or height to at least 1 pixel, so that we don't need special treatments for different types of geometries.

Kontinuation · 2023-09-09T02:17:02Z

common/src/test/java/org/apache/sedona/common/raster/RasterConstructorsTest.java

+        expected = new double[] {0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 0.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 0.0, 0.0, 0.0, 0.0, 0.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 0.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 0.0, 0.0, 0.0, 0.0, 0.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 0.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 0.0, 0.0, 0.0, 0.0, 0.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 0.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 0.0, 0.0, 0.0, 0.0, 0.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 0.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 0.0, 0.0, 0.0, 0.0, 0.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 0.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 0.0, 0.0, 0.0, 0.0, 0.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 0.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 0.0, 0.0, 0.0, 0.0, 0.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 0.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 0.0, 0.0, 0.0, 0.0, 0.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 0.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 0.0, 0.0, 0.0, 0.0, 0.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 0.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 0.0, 0.0, 0.0, 0.0, 0.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 0.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 0.0, 0.0, 0.0, 0.0, 0.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 0.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 0.0, 0.0, 0.0, 0.0, 0.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 0.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 0.0, 0.0, 0.0, 0.0, 0.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 0.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 0.0, 0.0, 0.0, 0.0, 0.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 0.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 0.0, 0.0, 0.0, 0.0, 0.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 0.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 3093151.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0};
+        assertArrayEquals(expected, actual, 0.1d);
+
+        rasterized = RasterConstructors.asRaster(geom, raster, "d");
+        actual = MapAlgebra.bandAsArray(rasterized, 1);
+        expected = new double[] {0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 0.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 0.0, 0.0, 0.0, 0.0, 0.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 0.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 0.0, 0.0, 0.0, 0.0, 0.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 0.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 0.0, 0.0, 0.0, 0.0, 0.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 0.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 0.0, 0.0, 0.0, 0.0, 0.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 0.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 0.0, 0.0, 0.0, 0.0, 0.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 0.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 0.0, 0.0, 0.0, 0.0, 0.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 0.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 0.0, 0.0, 0.0, 0.0, 0.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 0.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 0.0, 0.0, 0.0, 0.0, 0.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 0.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 0.0, 0.0, 0.0, 0.0, 0.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 0.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 0.0, 0.0, 0.0, 0.0, 0.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 0.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 0.0, 0.0, 0.0, 0.0, 0.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 0.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 0.0, 0.0, 0.0, 0.0, 0.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 0.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 0.0, 0.0, 0.0, 0.0, 0.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 0.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 0.0, 0.0, 0.0, 0.0, 0.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 0.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 0.0, 0.0, 0.0, 0.0, 0.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 0.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0};
+        assertArrayEquals(expected, actual, 0.1d);


These walls of texts as expected results are very confusing.
I suggest rendering smaller rasters, or picking some representative points and only testing them.

I agree, altering RN.

jiayuasu · 2023-09-09T03:43:44Z

common/src/main/java/org/apache/sedona/common/raster/RasterOutputs.java

+     * @return true if file is created, otherwise throws IOException
+     * @throws IOException
+     * */
+    public static boolean toGeoTiff(GridCoverage2D raster, String filePath) {


This function should be called writeToDiskFile. It takes a byte[] array as input and write the file to disk. This is a generic function that works with the result of asGeoTiff, asArcGrid, and so on.

jiayuasu · 2023-09-09T03:46:07Z

common/src/test/java/org/apache/sedona/common/raster/RasterOutputTest.java

+    @Test
+    public void testToGeoTiff() throws IOException {
+        GridCoverage2D rasterOg = rasterFromGeoTiff(resourceFolder + "raster/test1.tiff");
+        String filePath = System.getProperty("user.dir") + "/../spark/common/src/test/resources/testToGeoTiffFunction/test1.tiff";


The file path is wrong. It should be System.getProperty("user.dir") + "/target/estToGeoTiffFunction/test1.tiff";. Do not add content to the test resource folder.

jiayuasu · 2023-09-09T03:48:03Z

spark/common/src/test/resources/testToGeoTiffFunction/test1.tiff

This file is generated by your toGeoTiff. It should not be committed. And again, the test should not write result back to the resources folder.

furqaankhan added 8 commits August 30, 2023 14:34

feat: add initial implementation of RS_AsRaster

7c85c9d

Merge remote-tracking branch 'origin/rs-as-raster' into rs-as-raster

c378bb9

# Conflicts: # common/src/main/java/org/apache/sedona/common/raster/RasterConstructors.java

feat: add implementation of RS_AsRaster

26b16c6

temp: test for RS_AsRaster

b0b14c3

feat: RS_AsRaster Implementation with test

d6e2c5b

feat: port RS_AsRaster to spark sql and add test

418b7ad

docs: add docs for RS_AsRaster

bbbc295

docs: correct alphabetic order

4f57e45

Kontinuation reviewed Sep 8, 2023

View reviewed changes

common/src/test/java/org/apache/sedona/common/raster/RasterConstructorsTest.java Outdated Show resolved Hide resolved

jiayuasu requested changes Sep 8, 2023

View reviewed changes

jiayuasu added affect public APIs sedona-common sedona-spark labels Sep 8, 2023

jiayuasu added this to the sedona-1.5.0 milestone Sep 8, 2023

jiayuasu requested changes Sep 8, 2023

View reviewed changes

common/src/test/java/org/apache/sedona/common/raster/RasterConstructorsTest.java Outdated Show resolved Hide resolved

furqaankhan and others added 7 commits September 8, 2023 09:27

Merge branch 'apache:master' into rs-as-raster

9e21090

feat: change toGeoTiff implementation, add test

94c71c8

feat: add dependency in spark-shaded

bc194f8

docs: add pixeltype explanation, clearly defined the flavors supported

7f9601b

feat: add a new convenience flavor to spark sql

be3e9d1

fix: add point extent formula and tests for each geometry type

8f387fc

feat: add test for new flavor in spark sql

644b445

Kontinuation reviewed Sep 9, 2023

View reviewed changes

furqaankhan added 4 commits September 8, 2023 22:23

fix: kept the test tiff file

787717f

feat: add support for LineString special case

016ac7b

feat: add LineString special test case in spark sql

0dfce39

feat: check width and height rather than geometry types

d9d63d8

jiayuasu requested changes Sep 9, 2023

View reviewed changes

furqaankhan added 2 commits September 9, 2023 00:02

fix: change path for test geotiff and remove .tiff from resource folder

ab9c713

fix: FileNotFoundError

d37925f

fix: changed function name

d7adb99

Kontinuation approved these changes Sep 11, 2023

View reviewed changes

jiayuasu merged commit 8ab6e91 into apache:master Sep 11, 2023
39 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SEDONA-388] Add RS_AsRaster #1011

[SEDONA-388] Add RS_AsRaster #1011

furqaankhan commented Sep 7, 2023

jiayuasu Sep 8, 2023

jiayuasu Sep 8, 2023

Kontinuation Sep 9, 2023

Kontinuation Sep 9, 2023

Kontinuation Sep 9, 2023

furqaankhan Sep 9, 2023

jiayuasu Sep 9, 2023

jiayuasu Sep 9, 2023

jiayuasu Sep 9, 2023

[SEDONA-388] Add RS_AsRaster #1011

[SEDONA-388] Add RS_AsRaster #1011

Conversation

furqaankhan commented Sep 7, 2023

Did you read the Contributor Guide?

Is this PR related to a JIRA ticket?

What changes were proposed in this PR?

How was this patch tested?

Did this PR include necessary documentation updates?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment