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

Upgrade go-spatial/geom to latest #952

Merged
merged 2 commits into from
Nov 9, 2023

Conversation

jchamberlain
Copy link
Contributor

This addresses the second point in #951. The main changes impacting this project are in the slippy package, and include:

  • Using a grid for calculating extent, rather than relying on slippy.Tile directly.
  • Removal of SRID-specific extent functions (Extent3857(), Extent4326()), thus requiring use a another tool for transformations.
  • Buggy behavior in new slippy.RangeFamilyAt(), thus requiring keeping around the old version of that function.

@@ -345,7 +345,7 @@ func TestEncode(t *testing.T) {
},
},
},
tile: slippy.NewTile(2, 3, 4),
tile: slippy.NewTile(2, 3, 3),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

4 is not a valid y-value at zoom 2, and the geom upgrade makes this test fail unless a valid coordinate is used.

@ARolek
Copy link
Member

ARolek commented Sep 29, 2023

@jchamberlain could you please split this PR into 2 commits: 1 for the vendoring of geom (and go.mod updates) and then a second one for the code changes? Makes review a bit easier.

Copy link
Member

@gdey gdey left a comment

Choose a reason for hiding this comment

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

LGTM

There are some minor code cleanups; that we can get to later. And we should make an issue around renaming slippy.PixelsToNative.

cmd/tegola/cmd/cache/slippy.go Show resolved Hide resolved
cmd/tegola/cmd/cache/slippy_test.go Show resolved Hide resolved
provider/provider.go Show resolved Hide resolved
@ARolek
Copy link
Member

ARolek commented Nov 2, 2023

@jchamberlain can you rebase against master? I just fixed the docker hub issue (and upgraded Go in the CI) so we can get the CI to pass. Then I can merge this in.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 4ce028e55-PR-952

  • 51 of 85 (60.0%) changed or added relevant lines in 8 files are covered.
  • 3 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.05%) to 46.855%

Changes Missing Coverage Covered Lines Changed/Added Lines %
cmd/tegola/cmd/cache/slippy.go 18 20 90.0%
cmd/tegola/cmd/cache/seed_purge.go 9 16 56.25%
server/handle_map_layer_zxy.go 12 24 50.0%
provider/provider.go 4 17 23.53%
Files with Coverage Reduction New Missed Lines %
provider/provider.go 1 54.29%
cmd/tegola/cmd/cache/cache.go 2 23.56%
Totals Coverage Status
Change from base Build 4d93140a4: -0.05%
Covered Lines: 6571
Relevant Lines: 14024

💛 - Coveralls

@ARolek ARolek merged commit a6f1b0c into go-spatial:master Nov 9, 2023
9 of 11 checks passed
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.

4 participants