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/15 17:41:39 UTC
[GitHub] [sedona] iGN5117 opened a new pull request, #973: [SEDONA-364] Add RS_MinConvexHull
iGN5117 opened a new pull request, #973:
URL: https://github.com/apache/sedona/pull/973
## 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?
- Yes, the URL of the associated JIRA ticket is https://issues.apache.org/jira/browse/SEDONA-364. The PR name follows the format `[SEDONA-XXX] my subject`.
## What changes were proposed in this PR?
- Add RS_MinConvexHull
## How was this patch tested?
- Added new tests
## Did this PR include necessary documentation updates?
- Yes, I am adding a new API. I am using the [current SNAPSHOT version number](https://github.com/apache/sedona/blob/master/pom.xml#L29) in since `vX.Y.Z` format.
--
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 #973: [SEDONA-364] Add RS_MinConvexHull
Posted by "iGN5117 (via GitHub)" <gi...@apache.org>.
iGN5117 commented on code in PR #973:
URL: https://github.com/apache/sedona/pull/973#discussion_r1296045772
##########
common/src/main/java/org/apache/sedona/common/raster/GeometryFunctions.java:
##########
@@ -61,6 +63,70 @@ public static Geometry convexHull(GridCoverage2D raster) throws FactoryException
return geometryFactory.createPolygon(new Coordinate[] {coordOne, coordTwo, coordThree, coordFour, coordOne});
}
+ public static Geometry minConvexHull(GridCoverage2D raster, Integer band) throws FactoryException, TransformException {
+ boolean allBands = band == null;
+ if (!allBands) ensureSafeBand(raster, band);
+ int width = RasterAccessors.getWidth(raster), height = RasterAccessors.getHeight(raster), srid = RasterAccessors.srid(raster);
+ int minX = Integer.MAX_VALUE, maxX = Integer.MIN_VALUE, minY = Integer.MAX_VALUE, maxY = Integer.MIN_VALUE;
+ for (int j = 0; j < height; j++) {
+ for (int i = 0; i < width; i++) {
+ // get the value of the raster at the coordinates i, j
+ //if value is not no data, update variables to track minX, maxX, minY, maxY
+ GridCoordinates2D currGridCoordinate = new GridCoordinates2D(i, j);
+ double[] bandPixelValues = raster.evaluate(currGridCoordinate, (double[]) null);
+ if (allBands) {
Review Comment:
Great suggestion! I've refactored the code and pushed
--
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 #973: [SEDONA-364] Add RS_MinConvexHull
Posted by "iGN5117 (via GitHub)" <gi...@apache.org>.
iGN5117 commented on code in PR #973:
URL: https://github.com/apache/sedona/pull/973#discussion_r1296017699
##########
common/src/test/java/org/apache/sedona/common/raster/GeometryFunctionsTest.java:
##########
@@ -76,6 +77,49 @@ public void testConvexHullFromRasterFile() throws FactoryException, TransformExc
assertEquals(expectedCoordOne.getY(), coordinates[4].getY(), 0.5d);
}
+ @Test
+ public void testMinConvexHull() throws FactoryException, TransformException {
Review Comment:
Yes, all tests are done both on sedona and PostGIS
--
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 #973: [SEDONA-364] Add RS_MinConvexHull
Posted by "jiayuasu (via GitHub)" <gi...@apache.org>.
jiayuasu commented on code in PR #973:
URL: https://github.com/apache/sedona/pull/973#discussion_r1295524162
##########
common/src/main/java/org/apache/sedona/common/raster/GeometryFunctions.java:
##########
@@ -61,6 +63,70 @@ public static Geometry convexHull(GridCoverage2D raster) throws FactoryException
return geometryFactory.createPolygon(new Coordinate[] {coordOne, coordTwo, coordThree, coordFour, coordOne});
}
+ public static Geometry minConvexHull(GridCoverage2D raster, Integer band) throws FactoryException, TransformException {
+ boolean allBands = band == null;
+ if (!allBands) ensureSafeBand(raster, band);
+ int width = RasterAccessors.getWidth(raster), height = RasterAccessors.getHeight(raster), srid = RasterAccessors.srid(raster);
+ int minX = Integer.MAX_VALUE, maxX = Integer.MIN_VALUE, minY = Integer.MAX_VALUE, maxY = Integer.MIN_VALUE;
+ for (int j = 0; j < height; j++) {
+ for (int i = 0; i < width; i++) {
+ // get the value of the raster at the coordinates i, j
+ //if value is not no data, update variables to track minX, maxX, minY, maxY
+ GridCoordinates2D currGridCoordinate = new GridCoordinates2D(i, j);
+ double[] bandPixelValues = raster.evaluate(currGridCoordinate, (double[]) null);
+ if (allBands) {
Review Comment:
The duplicate code here can be merged. You can use the same for loop for both condition by manually controlling the start and end of this loop.
##########
common/src/test/java/org/apache/sedona/common/raster/GeometryFunctionsTest.java:
##########
@@ -76,6 +77,49 @@ public void testConvexHullFromRasterFile() throws FactoryException, TransformExc
assertEquals(expectedCoordOne.getY(), coordinates[4].getY(), 0.5d);
}
+ @Test
+ public void testMinConvexHull() throws FactoryException, TransformException {
Review Comment:
Do these tests yield the same results as PostGIS?
##########
docs/api/sql/Raster-operators.md:
##########
@@ -62,14 +62,70 @@ Since: `1.5.0`
Spark SQL example:
```sql
-SELECT RS_ConvexHull(SELECT RS_PixelAsPoint(RS_MakeEmptyRaster(1, 5, 10, 156, -132, 5, 10, 3, 5, 0)));
+SELECT RS_ConvexHull(RS_MakeEmptyRaster(1, 5, 10, 156, -132, 5, 10, 3, 5, 0));
```
Output:
```
POLYGON ((156 -132, 181 -107, 211 -7, 186 -32, 156 -132))
```
+### RS_MinConvexHull
+
+Introduction: Returns the min convex hull geometry of the raster **excluding** the NoDataBandValue band pixels, in the given band.
Review Comment:
Please also add that the produced geometry uses the world coordinate in the CRS specified by the raster.
##########
common/src/main/java/org/apache/sedona/common/raster/GeometryFunctions.java:
##########
@@ -61,6 +63,70 @@ public static Geometry convexHull(GridCoverage2D raster) throws FactoryException
return geometryFactory.createPolygon(new Coordinate[] {coordOne, coordTwo, coordThree, coordFour, coordOne});
}
+ public static Geometry minConvexHull(GridCoverage2D raster, Integer band) throws FactoryException, TransformException {
+ boolean allBands = band == null;
+ if (!allBands) ensureSafeBand(raster, band);
+ int width = RasterAccessors.getWidth(raster), height = RasterAccessors.getHeight(raster), srid = RasterAccessors.srid(raster);
+ int minX = Integer.MAX_VALUE, maxX = Integer.MIN_VALUE, minY = Integer.MAX_VALUE, maxY = Integer.MIN_VALUE;
+ for (int j = 0; j < height; j++) {
+ for (int i = 0; i < width; i++) {
+ // get the value of the raster at the coordinates i, j
+ //if value is not no data, update variables to track minX, maxX, minY, maxY
+ GridCoordinates2D currGridCoordinate = new GridCoordinates2D(i, j);
+ double[] bandPixelValues = raster.evaluate(currGridCoordinate, (double[]) null);
+ if (allBands) {
+ for (int currBand = 1; currBand <= RasterAccessors.numBands(raster); currBand++) {
+ double currBandValue = bandPixelValues[currBand - 1];
+ Double bandNoDataValue = RasterBandAccessors.getBandNoDataValue(raster, currBand);
+ if ((bandNoDataValue == null) || currBandValue != bandNoDataValue) {
Review Comment:
When there are multiple bands, is the overall minConvexHull the hull that has the minX, minY, maxX, maxY among all bands?
--
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 #973: [SEDONA-364] Add RS_MinConvexHull
Posted by "iGN5117 (via GitHub)" <gi...@apache.org>.
iGN5117 commented on code in PR #973:
URL: https://github.com/apache/sedona/pull/973#discussion_r1296045139
##########
docs/api/sql/Raster-operators.md:
##########
@@ -62,14 +62,70 @@ Since: `1.5.0`
Spark SQL example:
```sql
-SELECT RS_ConvexHull(SELECT RS_PixelAsPoint(RS_MakeEmptyRaster(1, 5, 10, 156, -132, 5, 10, 3, 5, 0)));
+SELECT RS_ConvexHull(RS_MakeEmptyRaster(1, 5, 10, 156, -132, 5, 10, 3, 5, 0));
```
Output:
```
POLYGON ((156 -132, 181 -107, 211 -7, 186 -32, 156 -132))
```
+### RS_MinConvexHull
+
+Introduction: Returns the min convex hull geometry of the raster **excluding** the NoDataBandValue band pixels, in the given band.
Review Comment:
Good point, I added this in the docs
--
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 #973: [SEDONA-364] Add RS_MinConvexHull
Posted by "iGN5117 (via GitHub)" <gi...@apache.org>.
iGN5117 commented on code in PR #973:
URL: https://github.com/apache/sedona/pull/973#discussion_r1296018941
##########
common/src/main/java/org/apache/sedona/common/raster/GeometryFunctions.java:
##########
@@ -61,6 +63,70 @@ public static Geometry convexHull(GridCoverage2D raster) throws FactoryException
return geometryFactory.createPolygon(new Coordinate[] {coordOne, coordTwo, coordThree, coordFour, coordOne});
}
+ public static Geometry minConvexHull(GridCoverage2D raster, Integer band) throws FactoryException, TransformException {
+ boolean allBands = band == null;
+ if (!allBands) ensureSafeBand(raster, band);
+ int width = RasterAccessors.getWidth(raster), height = RasterAccessors.getHeight(raster), srid = RasterAccessors.srid(raster);
+ int minX = Integer.MAX_VALUE, maxX = Integer.MIN_VALUE, minY = Integer.MAX_VALUE, maxY = Integer.MIN_VALUE;
+ for (int j = 0; j < height; j++) {
+ for (int i = 0; i < width; i++) {
+ // get the value of the raster at the coordinates i, j
+ //if value is not no data, update variables to track minX, maxX, minY, maxY
+ GridCoordinates2D currGridCoordinate = new GridCoordinates2D(i, j);
+ double[] bandPixelValues = raster.evaluate(currGridCoordinate, (double[]) null);
+ if (allBands) {
+ for (int currBand = 1; currBand <= RasterAccessors.numBands(raster); currBand++) {
+ double currBandValue = bandPixelValues[currBand - 1];
+ Double bandNoDataValue = RasterBandAccessors.getBandNoDataValue(raster, currBand);
+ if ((bandNoDataValue == null) || currBandValue != bandNoDataValue) {
Review Comment:
Yes, by multiple bands I mean all bands, we do not have an option of supporting a subset of all bands. We either do 1 band or all bands
--
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 #973: [SEDONA-364] Add RS_MinConvexHull
Posted by "jiayuasu (via GitHub)" <gi...@apache.org>.
jiayuasu merged PR #973:
URL: https://github.com/apache/sedona/pull/973
--
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