-
Notifications
You must be signed in to change notification settings - Fork 360
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
Minor issues fixes #2027
Minor issues fixes #2027
Conversation
Signed-off-by: Grigory Pomadchin <gr.pomadchin@gmail.com>
3b5840a
to
cb110f1
Compare
Signed-off-by: Grigory Pomadchin <gr.pomadchin@gmail.com>
cb110f1
to
67e93c6
Compare
@@ -46,6 +46,13 @@ object TileLayout { | |||
* @param tileRows number of pixel rows in each tile, North to South | |||
*/ | |||
case class TileLayout(layoutCols: Int, layoutRows: Int, tileCols: Int, tileRows: Int) { | |||
|
|||
assert( |
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.
should use require
in these cases, that gives more relevant exception.
assert( | ||
layoutCols >= 0 && layoutRows >= 0 && tileCols >= 0 && tileRows >= 0, | ||
s"TileLayout should contain cols and rows >= 0: " + | ||
s"TileLayout(layoutCols = $layoutCols, layoutRows = $layoutRows, tileCols = $tileCols, tileRows = $tileRows)" |
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 think its fine to use .toString
in these cases even though it doesn't have field labels its a little more tenable as a pattern.
#2021 work would be done in a separate PR; only descriptive failure messages were introduced in this PR at least to reduce pain. |
@@ -116,15 +116,23 @@ class ZoomedLayoutScheme(val crs: CRS, val tileSize: Int, resolutionThreshold: D | |||
|
|||
def zoomOut(level: LayoutLevel) = { | |||
val layout = level.layout | |||
|
|||
if(layout.layoutCols < 2 || layout.layoutRows < 2) | |||
sys.error( |
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.
This is too late to give this exception. At this point a couple of pyramid levels would have already been saved and all that work would have been wasted.
Its possible to give this exception before we start the pyramid but now the nature of this error depends on work in the broken out PR. I would say lets remove this exception here and deal with the issue fully in separate PR.
11339f6
to
3632a61
Compare
@@ -46,6 +46,12 @@ object TileLayout { | |||
* @param tileRows number of pixel rows in each tile, North to South | |||
*/ | |||
case class TileLayout(layoutCols: Int, layoutRows: Int, tileCols: Int, tileRows: Int) { | |||
|
|||
require( | |||
layoutCols >= 0 && layoutRows >= 0 && tileCols >= 0 && tileRows >= 0, |
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.
>
not >=
, need at least one pixel or a tile in the tile/layout.
Signed-off-by: Grigory Pomadchin <gr.pomadchin@gmail.com>
Signed-off-by: Grigory Pomadchin <gr.pomadchin@gmail.com>
Signed-off-by: Grigory Pomadchin <gr.pomadchin@gmail.com>
feee027
to
e504cad
Compare
Signed-off-by: Grigory Pomadchin <gr.pomadchin@gmail.com>
This seems fine for |
Resolves #2022
Resolves #2012
Resolves #2026