-
Notifications
You must be signed in to change notification settings - Fork 362
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
Add Buffer Tile #3419
Add Buffer Tile #3419
Conversation
} | ||
} | ||
|
||
def mapTile(f: Tile => Tile): BufferTile = BufferTile(f(sourceTile), gridBounds) |
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.
@echeipesh how about making all functions to work within the window, but if user wants to apply smth beayond the window they need to do it explicitly. Mb the function name could be different though.
|
||
/** Computes the minimum value of a neighborhood */ | ||
def focalMin(n: Neighborhood, bounds: Option[GridBounds[Int]] = None, target: TargetCell = TargetCell.All): BufferTile = | ||
self.mapTile(_.focalMin(n, bounds, target)) |
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.
In this case focal ops are applied to the underlying tile cc @echeipesh
* - What should .map do? Map the buffer pixels or not? | ||
* - toString method is friendly | ||
* - mutable version makes sense | ||
* - toBytes needs to encode padding size? |
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.
If we're ok with the explicit work with the underlying tile:
- map would be applied to the window only (so it is consistent with any Tile behavior, with gets and foreaches)
- still need a friednly toString
- yes!
- toBytes encodes window only; we don't have any reasons to encode gridBounds to the output array 🤷, no interfaces that would support reading from it.
0143c68
to
25ce870
Compare
Going to merge it! We would still have a chance to change behavior and API if necessary. I would like to get it merged to remove BufferTile from RasterFrames in terms of locationtech/rasterframes#567 |
Overview
This PR adds BufferTile type to handle focal ops in RF.
Checklist