Tile grid #105
Tile grid #105
Conversation
…a check for the SRID (L25-27) from the grid to allow for increased coverage. Adjusted the test areas from directional (north, east, south, west) to quadrants [1-4].
BerryDaniel
force-pushed
the
tile-grid
branch
from
34863a0 to
e8c68ce
Compare
slippy/tile_grid_test.go
Outdated
| srid uint | ||
| zoom uint | ||
| expected uint | ||
| expected [2]uint |
There was a problem hiding this comment.
please change this (and the test cases) to geom.Point
There was a problem hiding this comment.
My bad, these are tile coordinates. Here, the zoom and expected can be joined into a single field with type *Tile (NewTile returns a pointer).
Note: structs can be compared like *tc.expected == *tile
There was a problem hiding this comment.
Wouldn't you need the grid first before creating the tile?
slippy/tile_grid_test.go
Outdated
| srid uint | ||
| zoom uint | ||
| expected float64 | ||
| expected [2]float64 |
There was a problem hiding this comment.
please change this (and the test cases) to geom.Point
slippy/tile_grid_test.go
Outdated
| "3857_south": { | ||
| lat: -85.0511, | ||
| "3857_z0_random": { | ||
| point: geom.Point{0, 0 + rand.Float64() * 85.0511}, |
There was a problem hiding this comment.
Go's math/rand generator is deterministic, so this will always be the same number. The same behavior is good for tests, but the use of the package is misleading. Use a literal instead like 12.345.
slippy/tile_grid_test.go
Outdated
| x uint | ||
| y uint | ||
| srid uint | ||
| zoom uint |
There was a problem hiding this comment.
The zoom, x, and y here should also be contained into a tile
slippy/tile_grid_test.go
Outdated
|
|
||
| if !cmp.Float(pt.X(), tc.expected) { | ||
| t.Errorf("got %v expected %v", pt.X(), tc.expected) | ||
| if pt != tc.expected { |
There was a problem hiding this comment.
You should use cmp.PointEqual(pt, tc.expected) (from github.com/go-spatial/geom/cmp). This is the package we use for floating point comparisons since, plain == is unstable.
There was a problem hiding this comment.
The test cases can be left as is btw
There was a problem hiding this comment.
modified as requested
|
@BerryDaniel LGTM thank you! |
Consolidated X and Y tests for FromNative and ToNative. I also added a check for the SRID (L25-27) from the grid to allow for increased coverage. Adjusted the test areas from directional (north, east, south, west) to quadrants [1-4].
rebased to address merge conflict