Skip to content
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

Slippy tile issue 997 #1007

Merged
merged 2 commits into from
Aug 15, 2024
Merged

Slippy tile issue 997 #1007

merged 2 commits into from
Aug 15, 2024

Conversation

gdey
Copy link
Member

@gdey gdey commented Jul 30, 2024

No description provided.

@gdey gdey requested a review from ARolek as a code owner July 30, 2024 19:10
Copy link
Member

@ARolek ARolek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lots of great clean up. Thanks for tackling this!

@@ -79,7 +79,7 @@ func TestReplaceTokens(t *testing.T) {
sql: "SELECT id, !pixel_width! as width, !pixel_height! as height, !scale_denominator! as scale_denom FROM foo WHERE !BBOX!",
layer: Layer{srid: tegola.WebMercator, geomField: "geom"},
tile: provider.NewTile(11, 1070, 676, 64, tegola.WebMercator),
expected: `SELECT id, 76.43702827 as width, 76.43702827 as height, 272989.38669477 as scale_denom FROM foo WHERE "geom".ST_IntersectsRectPlanar(NEW ST_POINT($1, $3), NEW ST_POINT($2, $3)) = 1`,
expected: `SELECT id, 76.43702829 as width, 76.43702829 as height, 272989.38673277 as scale_denom FROM foo WHERE "geom".ST_IntersectsRectPlanar(NEW ST_POINT($1, $3), NEW ST_POINT($2, $3)) = 1`,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these the old values before the change happened in the last version, or did you generate new values?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I generated new values. Did not conform the values.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, maybe check the old values to see how close they line up.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked at it's all within 0.00000001 of each other. Some of the number are close to what was there, but the numbers after are different. I think this is again due to the drift in floating numbers.

go.mod Outdated Show resolved Hide resolved
@coveralls
Copy link

coveralls commented Aug 7, 2024

Pull Request Test Coverage Report for Build e5a736695

Details

  • 34 of 67 (50.75%) changed or added relevant lines in 12 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.07%) to 42.418%

Changes Missing Coverage Covered Lines Changed/Added Lines %
atlas/atlas.go 0 1 0.0%
cmd/tegola/cmd/cache/format.go 2 3 66.67%
cmd/tegola/cmd/cache/tile_list.go 5 6 83.33%
cmd/tegola/cmd/cache/tile_name.go 5 6 83.33%
server/handle_map_layer_zxy.go 5 6 83.33%
tile.go 0 1 0.0%
atlas/map.go 9 12 75.0%
cmd/tegola/cmd/cache/cache.go 1 4 25.0%
cmd/tegola/cmd/cache/seed_purge.go 5 9 55.56%
provider/provider.go 0 8 0.0%
Files with Coverage Reduction New Missed Lines %
provider/provider.go 1 54.55%
Totals Coverage Status
Change from base Build a58cd8965: -0.07%
Covered Lines: 6893
Relevant Lines: 16250

💛 - Coveralls

@ARolek
Copy link
Member

ARolek commented Aug 15, 2024

@gdey one nit on this PR, can you rename the commit titles? They're currently the same, but one commit is for vendoring the geom package, the other is for adopting it.

Updated to use slippy tile from geom v0.1.0.
This fixes the issues with slippy tile.
@ARolek ARolek merged commit e5a7366 into master Aug 15, 2024
16 of 18 checks passed
@ARolek ARolek deleted the slippy_tile_issue_997 branch August 15, 2024 18:39
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.

3 participants