Skip to content

Add Buffer Tile - #3419

Merged
pomadchin merged 5 commits into
locationtech:masterfrom
pomadchin:feature/buffer-tile
Oct 4, 2021
Merged

pomadchin merged 5 commits into
locationtech:masterfrom
pomadchin:feature/buffer-tile

Conversation

@pomadchin

@pomadchin pomadchin commented Sep 20, 2021 •

Copy link
Copy Markdown
Member

Overview

This PR adds BufferTile type to handle focal ops in RF.

Checklist

  • ./CHANGELOG.md updated, if necessary. Link to the issue if closed, otherwise the PR.
  • Unit tests added for bug-fix or new feature

}
}

def mapTile(f: Tile => Tile): BufferTile = BufferTile(f(sourceTile), gridBounds)

Copy link
Copy Markdown
Member Author

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))

Copy link
Copy Markdown
Member Author

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

Comment on lines +30 to +33
* - What should .map do? Map the buffer pixels or not?
* - toString method is friendly
* - mutable version makes sense
* - toBytes needs to encode padding size?

Copy link
Copy Markdown
Member Author

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:

  1. map would be applied to the window only (so it is consistent with any Tile behavior, with gets and foreaches)
  2. still need a friednly toString
  3. yes!
  4. 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.

@pomadchin pomadchin changed the title [WIP] Buffer Tiles [WIP] Add Buffer Tile Sep 23, 2021
@pomadchin pomadchin changed the title [WIP] Add Buffer Tile Add Buffer Tile Sep 23, 2021
@pomadchin

Copy link
Copy Markdown
Member Author

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

@pomadchin
pomadchin merged commit d690970 into locationtech:master Oct 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants