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.

Sign up for GitHub

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

GeoTrellisRasterSource should return None on empty reads #3240

Merged
merged 2 commits into from
May 14, 2020
Merged

GeoTrellisRasterSource should return None on empty reads #3240

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3e791a9
Remove GeoTrellisRasterSource.sparseStitch
pomadchin May 13, 2020
42c94b4
Cover sparseStitch with an empty input test case
pomadchin May 14, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Fix S3RDDLayerReader partitioning [#3231](https://github.com/locationtech/geotrellis/pull/3231)
- GDALRasterSource works inconsistenly with BitCellType and ByteCellType [#3232](https://github.com/locationtech/geotrellis/issues/3232)
- rasterizeWithValue accepts only topologically valid polygons [#3236](https://github.com/locationtech/geotrellis/pull/3236)
- Rasterizer.rasterize should be consistent with rasterizeWithValue [#3238](https://github.com/locationtech/geotrellis/pull/3238)
- Rasterizer.rasterize should be consistent with rasterizeWithValue [#3238](https://github.com/locationtech/geotrellis/pull/3238)
pomadchin marked this conversation as resolved.
Show resolved Hide resolved
- GeoTrellisRasterSource should return None on empty reads [#3240](https://github.com/locationtech/geotrellis/pull/3240)

## [3.3.0] - 2020-04-07

Expand Down
2 changes: 1 addition & 1 deletion layer/src/main/scala/geotrellis/layer/stitch/StitchCollectionMethods.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ abstract class SpatialTileLayoutCollectionStitchMethods[
* @return The stitched Raster, otherwise None if the collection is empty or the extent does not intersect
*/
def sparseStitch(extent: Extent): Option[Raster[V]] = {
if (self.headOption.isEmpty) {
if (self.isEmpty) {
None
} else {
val tile = self.head._2
Expand Down
2 changes: 1 addition & 1 deletion spark/src/main/scala/geotrellis/spark/stitch/StitchRDDMethods.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ abstract class SpatialTileLayoutRDDStitchMethods[
val tiles = self.toCollection
// From here down, this code duplicates SpatialTileLayoutCollectionStitchMethods.sparseStitch,
// replacing self with tiles
if (tiles.headOption.isEmpty) {
if (tiles.isEmpty) {
None
} else {
val tile = tiles.head._2
Expand Down
31 changes: 2 additions & 29 deletions store/src/main/scala/geotrellis/store/GeoTrellisRasterSource.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,43 +258,16 @@ object GeoTrellisRasterSource {

def readIntersecting(reader: CollectionLayerReader[LayerId], layerId: LayerId, metadata: TileLayerMetadata[_], extent: Extent, bands: Seq[Int], time: Option[ZonedDateTime]): Option[Raster[MultibandTile]] = {
val tiles = readTiles(reader, layerId, extent, bands, time)
sparseStitch(tiles, extent)
tiles.sparseStitch(extent)
}

def read(reader: CollectionLayerReader[LayerId], layerId: LayerId, metadata: TileLayerMetadata[_], extent: Extent, bands: Seq[Int], time: Option[ZonedDateTime]): Option[Raster[MultibandTile]] = {
val tiles = readTiles(reader, layerId, extent, bands, time)
metadata.extent.intersection(extent) flatMap { intersectionExtent =>
sparseStitch(tiles, intersectionExtent).map(_.crop(intersectionExtent))
tiles.sparseStitch(intersectionExtent).map(_.crop(intersectionExtent))
}
}

/**
* The stitch method in gtcore is unable to handle missing spatialkeys correctly.
* This method works around that problem by attempting to infer any missing tiles
**/
def sparseStitch(
tiles: Seq[(SpatialKey, MultibandTile)] with Metadata[TileLayerMetadata[SpatialKey]],
extent: Extent
): Option[Raster[MultibandTile]] = {
val md = tiles.metadata
Copy link
Member Author

@pomadchin pomadchin May 13, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

There was no tiles.isEmpty check, and anyway we don't need this function anymore, see #2903

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you comment a bit further on why we don't need this anymore? That linked issue + its links don't show where the original problem handling missing spatialkeys is addressed in the original sparsestitch function.

Copy link
Member Author

@pomadchin pomadchin May 14, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@CloudNiner if you will look a bit above (at the place where it was used):

sparseStitch(tiles, extent) we can use the inbuilt function now tiles.sparseStitch(extent)

The issue #2903 was addressed in your PR here #3017 which is called "Add sparse stitch method to StitchCollectionMethods".

Copy link
Contributor

Choose a reason for hiding this comment

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

lol embarrassing -- I do in fact remember this now. 👍

val expectedKeys = md
.mapTransform(extent)
.coordsIter
.map { case (x, y) => SpatialKey(x, y) }
.toList
val actualKeys = tiles.map(_._1)
val missingKeys = expectedKeys diff actualKeys

val missingTiles = missingKeys.map { key =>
(key, MultibandTile(ArrayTile.empty(md.cellType, md.tileLayout.tileCols, md.tileLayout.tileRows)))
}
val allTiles = tiles.withContext { collection =>
collection.toList ::: missingTiles
}
if (allTiles.isEmpty) None
else Some(allTiles.stitch())
}

def apply(dataPath: GeoTrellisPath): GeoTrellisRasterSource = GeoTrellisRasterSource(dataPath, None, None)

def apply(dataPath: GeoTrellisPath, time: Option[ZonedDateTime]): GeoTrellisRasterSource = GeoTrellisRasterSource(dataPath, time, None)
Expand Down