fix: add 1:50k to valid grid sizes TDE-1010 TDE-1014 TDE-1015 #810
Conversation
amfage
changed the title
| @@ -14,7 +14,7 @@ import { CommandListArgs } from '../list/list.js'; | |||
|
|
|||
| const SHEET_MIN_X = MapSheet.origin.x + 4 * MapSheet.width; // The minimum x coordinate of a valid sheet / tile | |||
There was a problem hiding this comment.
I am not sure why we even have these, there is a MapSheet object with more precise min and max values, which also have included unit tests.
We should remove what ever is referencing these.
There was a problem hiding this comment.
We could consult with the Land team and see what their requirements are? Perhaps we need to step back and look at the whole of tileset validation as we have another ticket regarding logging https://toitutewhenua.atlassian.net/browse/TDE-1013
| @@ -89,7 +89,7 @@ export const MapSheet = { | |||
| gridSizeMax: 50000, | |||
| roundCorrection: 0.01, | |||
| /** Allowed grid sizes, these should exist in the LINZ Data service (meters) */ | |||
| gridSizes: [10_000, 5_000, 2_000, 1_000, 500], | |||
There was a problem hiding this comment.
When fixing bugs it would be nice to add a unit test.
There was a problem hiding this comment.
It wasn't clear what the best way to do this was, I think the tests might need to be reviewed.
There was a problem hiding this comment.
while this might hide the error, I don't think this fixes the bug, the tiles will not be named correctly when using 1:50k.
There was a problem hiding this comment.
| @@ -14,7 +14,7 @@ import { CommandListArgs } from '../list/list.js'; | |||
|
|
|||
| const SHEET_MIN_X = MapSheet.origin.x + 4 * MapSheet.width; // The minimum x coordinate of a valid sheet / tile | |||
| const SHEET_MAX_X = MapSheet.origin.x + 46 * MapSheet.width; // The maximum x coordinate of a valid sheet / tile | |||
| const SHEET_MIN_Y = MapSheet.origin.y - 41 * MapSheet.height; // The minimum y coordinate of a valid sheet / tile | |||
| const SHEET_MIN_Y = MapSheet.origin.y - 42 * MapSheet.height; // The minimum y coordinate of a valid sheet / tile | |||
There was a problem hiding this comment.
Now that's a magic number if ever I saw one. Did a test failure necessitate this? This should probably be explained in more detail, or be refactored out.
There was a problem hiding this comment.
That was my mistake counting tiles I guess. This is my original draft when I worked on this:
#### Motivation Cleans up miscellaneous issues found by JetBrains IDEA. #### Modification - Remove unreferenced example code - Remove unused variables, types, classes - Import type rather than relying on UMD global variable - Wait for writes to finish #### Checklist None of these are applicable. This is just a cleanup. - [ ] Tests updated - [ ] Docs updated - [ ] Issue linked in Title
amfage
changed the title
amfage
changed the title
|
Superseded. |
Will revisit in the New Year - should probably revisit the validation approach
Motivation
Allow the use of 50k scale imagery.
Note: it would be good to improve the tests and also take a look at how tileset-validation logs errors. I think these should be in a separate PR.
Modification
This is a simple fix to allow the use of 50k imagery. It also fixes a bug where the bottom row of map sheets were not considered valid.
Checklist
If not applicable, provide explanation of why.