Skip to content

Conversions between GeoTools GridCoverage2D and GeoTrellis Raster types - #1502

Merged
lossyrob merged 28 commits into
locationtech:masterfrom
lossyrob:feature/jwm/geotools
Jun 6, 2016
Merged

lossyrob merged 28 commits into
locationtech:masterfrom
lossyrob:feature/jwm/geotools

Conversation

@lossyrob

@lossyrob lossyrob commented Jun 1, 2016

Copy link
Copy Markdown
Member

Supersedes #1444

James McClain and others added 27 commits May 16, 2016 10:14
According to the documentation[1], DataBuffer.TYPE_BYTE corresponds to
"unsigned byte" not "byte".

1. https://docs.oracle.com/javase/7/docs/api/java/awt/image/DataBuffer.html
Since all bands of Geotrellis MultibandTiles must all have the same
cellType and NODATA value, that particular type is not a good candidate
for representing the data in a GeoTools GridCoverage2D.  The best analog
of a GeoTools GridCoverage2D is an Array of Geotrellis Rasters.

Also, the converter now emits *ConstantNoDataCellType when it can.
The Raster[Tile] to GridCoverage2D transformation provides best-guess
ColorModels to the created GridCoverage2D objects.  The
Raster[MultibandTile] to GridCoverage2D transformation does not attempt
to provide ColorModel information.  NODATA metadata is now correct in
both the Tile case and MultibandTile case.
Since all bands of Geotrellis MultibandTiles must all have the same
cellType and NODATA value, that particular type is not a good candidate
for representing the data in a GeoTools GridCoverage2D.  The best analog
of a GeoTools GridCoverage2D is an Array of Geotrellis Rasters.

Also, the converter now emits *ConstantNoDataCellType when it can.
The Raster[Tile] to GridCoverage2D transformation provides best-guess
ColorModels to the created GridCoverage2D objects.  The
Raster[MultibandTile] to GridCoverage2D transformation does not attempt
to provide ColorModel information.  NODATA metadata is now correct in
both the Tile case and MultibandTile case.
@lossyrob lossyrob mentioned this pull request Jun 1, 2016
11 tasks done
@jamesmcclain

jamesmcclain commented Jun 3, 2016 •

Copy link
Copy Markdown
Member

+1 This looks good to me. There is quite a bit of test coverage and I was able to confirm that this produces GridCoverage2Ds which are compatible with GeoWave and viewable in the GeoServer.

I think that a valuable contribution would have been a concrete suggestion for compactifying some of the larger chunks of code, but unfortunately -- to the best of my knowledge -- the visual/textual similarity between different blocks of code is not necessarily easily addressed in a statically-typed language without using higher-order functions (or lisp-like macros).

ProjectedRaster(toRaster(bandIndex), crs)
case None =>
// Default LatLng
ProjectedRaster(toRaster(bandIndex), LatLng)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I wonder if it is worth emitting a warning here and/or giving a big warning in the source code that a CRS is being assumed.

@jamesmcclain

Copy link
Copy Markdown
Member

I think that geotools needs to be added to this list in order for the documentation to be buildable with the sbt doc command.

@jamesmcclain jamesmcclain mentioned this pull request Jun 6, 2016
4 tasks done
@lossyrob
lossyrob merged commit aa46532 into locationtech:master Jun 6, 2016
@lossyrob

lossyrob commented Jun 6, 2016

Copy link
Copy Markdown
Member Author

@jamesmcclain good call. Can you do that in you do that in your SimpleFeature PR?

@jamesmcclain

jamesmcclain commented Jun 7, 2016 •

Copy link
Copy Markdown
Member

@jamesmcclain good call. Can you do that in you do that in your SimpleFeature PR?

Indeed.

@lossyrob lossyrob added this to the 1.0 milestone Oct 18, 2016
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