Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SEDONA-421] Add RS_Clip #1078

Merged
merged 12 commits into from
Nov 9, 2023
Merged

[SEDONA-421] Add RS_Clip #1078

merged 12 commits into from
Nov 9, 2023

Conversation

furqaankhan
Copy link
Contributor

Did you read the Contributor Guide?

Is this PR related to a JIRA ticket?

What changes were proposed in this PR?

  • Add RS_Clip
  • Add support to RasterUtils.getDataType to work with RS_BandPixelType

How was this patch tested?

  • Passed new test cases

Did this PR include necessary documentation updates?

  • Yes, I have updated the documentation update.

// * @param crop Specifies to keep the original extent or not
// * @return A clip Raster with defined ROI by the geometry
// */
// public static GridCoverage2D clip(GridCoverage2D raster, int band, Geometry geometry, boolean crop) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove comments

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought to keep it when we can support same arity functions. We can uncomment it and use it. Do let me know if you still want them to be removed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. You can leave it there.

// this will shrink the extent of the raster to the geometry
Crop cropObject = new Crop();
ParameterValueGroup parameters = cropObject.getParameters();
parameters.parameter("Source").setValue(singleBandRaster);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this have to be a single band?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can be either single or all bands. How do you want me to implement it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A single band is ok for now.

@@ -69,6 +73,23 @@ public void testSetBandNoDataValueWithEmptyRasterMultipleBand() throws FactoryEx
assertEquals(444, (double) RasterBandAccessors.getBandNoDataValue(grid, 2), 0.1d);
}

@Test
public void testClip() throws IOException, FactoryException, TransformException, ParseException {
GridCoverage2D raster = rasterFromGeoTiff(resourceFolder + "raster_geotiff_color/FAA_UTM18N_NAD83.tif");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add tests to try with crop == true. In addition, please also test if the pixels of a raster are successfully masked.

@jiayuasu jiayuasu merged commit a996f8b into apache:master Nov 9, 2023
40 checks passed
@furqaankhan furqaankhan deleted the rs-clip branch July 16, 2024 01:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants