Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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-428] Add RS_ZonalStats & RS_ZonalStatsAll #1124
[SEDONA-428] Add RS_ZonalStats & RS_ZonalStatsAll #1124
Changes from all commits
b7a15b4
247ca5d
28ba03d
e0da982
e03be38
427a671
b35886e
46b9ef5
a175260
ac788ae
37bd597
ca3dd5c
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why add
150
here?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sedona/common/src/main/java/org/apache/sedona/common/raster/RasterBandAccessors.java
Lines 253 to 257 in ac788ae
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@furqaankhan Will it collide with existing values in a raster? If not, we are good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No no. It just to determine which values to use for compute from the provided raster.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where does
0
come from? Why skip it?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sedona/common/src/main/java/org/apache/sedona/common/raster/RasterBandAccessors.java
Line 237 in b35886e
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 set0
to all the other pixels. So I am skipping the0
is because those pixels are not under the geometry.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 theAsRasterWithRasterExtent
function instead. We talked about it in our tag-up last Thursday to improve performance.It is related to Zonal statistics.