Skip to content

GDALRasterSource should work consistent with BitCellType and ByteCellType - #3233

Merged
pomadchin merged 3 commits into
locationtech:masterfrom
pomadchin:fix/gdal-bit-byte-celltype
Apr 23, 2020
Merged

pomadchin merged 3 commits into
locationtech:masterfrom
pomadchin:fix/gdal-bit-byte-celltype

Conversation

@pomadchin

@pomadchin pomadchin commented Apr 22, 2020 •

Copy link
Copy Markdown
Member

Overview

This PR simplifies GDAL CellType derivation function and adds consistency specs

Checklist

Closes #3232

@pomadchin pomadchin self-assigned this Apr 22, 2020
@pomadchin
pomadchin force-pushed the fix/gdal-bit-byte-celltype branch from e250be5 to 3f21638 Compare April 22, 2020 01:40
@pomadchin
pomadchin requested a review from echeipesh April 22, 2020 01:41
@pomadchin

Copy link
Copy Markdown
Member Author

cc @metasim and @vpipkt

ct match {
case BitCellType =>
println("BitCellType requires a new GDALWarpBindings release")
// GDALRasterSource(path).cellType shouldBe UByteCellType

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.

I'll create a PR that would expose bitsPerSample through GDALWarp bindings.

@pomadchin
pomadchin force-pushed the fix/gdal-bit-byte-celltype branch from 3f21638 to 02d3d6e Compare April 22, 2020 01:44
Comment thread gdal/src/main/scala/geotrellis/raster/gdal/GDALUtils.scala
Comment thread gdal/src/main/scala/geotrellis/raster/gdal/GDALUtils.scala Outdated
@pomadchin
pomadchin force-pushed the fix/gdal-bit-byte-celltype branch from 0098528 to da02ce0 Compare April 22, 2020 02:42
@pomadchin pomadchin changed the title GDALRasterSource works inconsistenly with BitCellType and ByteCellType GDALRasterSource should work consistent with BitCellType and ByteCellType Apr 22, 2020
@pomadchin
pomadchin force-pushed the fix/gdal-bit-byte-celltype branch from da02ce0 to 5c9254f Compare April 22, 2020 20:48
@pomadchin
pomadchin force-pushed the fix/gdal-bit-byte-celltype branch from 5c9254f to 16d7ee0 Compare April 22, 2020 20:53
@pomadchin
pomadchin requested a review from metasim April 22, 2020 20:53
@pomadchin

Copy link
Copy Markdown
Member Author

To make these tests pass we need to merge #3232 and to file a CQ to merge that PR in.

@pomadchin
pomadchin force-pushed the fix/gdal-bit-byte-celltype branch from 16d7ee0 to f92743a Compare April 22, 2020 20:55
Comment on lines +293 to +296
lazy val bitsPerSample = md.get("NBITS").map(_.toInt)
/** To handle the [[ByteCellType]] it is possible to fetch information about the sampleFormat from the RasdterBand metadata. **/
lazy val signed = md.get("PIXELTYPE").contains("SIGNEDBYTE")

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.

That is a correct approach to retrieve the sampleFormat and bitsPerSample.

@pomadchin
pomadchin force-pushed the fix/gdal-bit-byte-celltype branch 3 times, most recently from 9fa5205 to 2067c86 Compare April 22, 2020 21:13
…te cellTypes, let's remove this part of the logic
@pomadchin
pomadchin force-pushed the fix/gdal-bit-byte-celltype branch from 2067c86 to 0ff52a9 Compare April 22, 2020 21:20
@pomadchin
pomadchin force-pushed the fix/gdal-bit-byte-celltype branch from e73c6e4 to db83894 Compare April 23, 2020 00:42
case _ => ByteCellType
}
if(!signedByte) noDataValue match {
case Some(nd) if nd.toInt > 0 && nd <= 255 => UByteUserDefinedNoDataCellType(nd.toByte)

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.

👍

@pomadchin

Copy link
Copy Markdown
Member Author

The CQ is still in process, I'll merge it as is, keeping an eye on that outstanding CQ. It is a dependency version up so there should be no problems with it.

@pomadchin
pomadchin merged commit 4e06fe4 into locationtech:master Apr 23, 2020
@pomadchin
pomadchin deleted the fix/gdal-bit-byte-celltype branch April 23, 2020 14:31
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.

GDALRasterSource works inconsistenly with BitCellType and ByteCellType

3 participants