-
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
RasterSource.resolutions becomes List[CellSize] #3124
Conversation
Add Dimensions and use them for intersection check
Other changes not documented because they were changes from 3.0-SNAPSHOT rather than 2.3
While preparing this PR following issue was discovered: #3123 |
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.
💯 beatiful!
@@ -317,7 +317,7 @@ class GDALRasterSourceRDDSpec extends FunSpec with TestEnvironment with BeforeAn | |||
val actual = RasterSourceRDD.read(readingSources, floatingLayout).stitch() | |||
|
|||
// random chip to test agains, to speed up tests | |||
val gridBounds = RasterExtent(randomExtentWithin(actual.extent), actual.cellSize).gridBounds | |||
val gridBounds = actual.rasterExtent.gridBoundsFor(randomExtentWithin(actual.extent)) |
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.
I'm surprised how much better it looks!
@@ -26,7 +26,7 @@ case class GDALMetadata( | |||
bandCount: Int, | |||
cellType: CellType, | |||
gridExtent: GridExtent[Long], | |||
resolutions: List[GridExtent[Long]], | |||
resolutions: List[CellSize], |
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.
👍 finally these are resolutions indeed.
} | ||
|
||
object Dimensions { | ||
implicit def apply[N: Integral](tup: (N, N)) = new Dimensions(tup._1, tup._2) |
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.
Just a notion: boxing happens here, but we don't care too much about it.
Overview
This PR is intended to address usability issues around resampling RasterSource described in #3100
RasterSource.resolutions is not
List[CellSize]
. As discussed in the issue this removes the ability to provideGridBounds
fromRasterSource.resolutions
to theRasterSource.read
method of the same instance. The judgement call is that if each overview somehow has differing extent this is considered very unusual and can be diagnosed by further digging intoRasterMetadata
interface or through additional specialized API.Grid.gridBounds are removed. This added a very confusing case where
GridBounds
could either be a window in some raster or describe the whole footprint of raster. RemovingGrid.gridBounds
goes a long way to remove that confusion. This is a recently added member to the class, so impact is judged to be small.GridBounds.intersects(dimensions) - For those cases when we're checking for intersection with a raster we're using the new
Dimensions
class.Dimensions
can be checked for intersection vsGridBounds
and intersection ofGridBounds
andDimensions
is smallerOption[GridBounds]
.Grid.dimensions becomes
Dimensions[N]
Instead of(N, N)
- with addition of specialized API described above. Giving this a class goes a long way to clarify its usage in API. Note that this change results in fix-ups to tests that could be be caught by compiler due to checking for equality with "==". This is likely to happen in client code but should be easily fixed.Checklist
docs/CHANGELOG.rst
updated, if necessarydocs
guides update, if necessaryNotes
Closes #3100