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

Fix wrong coverage around antimeridian #49

Merged
merged 2 commits into from
Sep 5, 2021

Conversation

guilhem-lk
Copy link
Contributor

We cannot use GeoHash.coverBoundingBox method with a bounding box around antimeridian (180/-180) because it does not allow bottomRightLon to be lower than topLeftLon (Google Maps behavior) or it does not handle correctly longitude outside [-180, 180] range (Leaflet behavior).

This pull request addresses this use case and also fixes issues in #25 and #34.

Now the GeoHash.coverBoundingBoxLongs method allow more longitudes values in input. For instance, these values are accepted (see unit tests):

  • topLeftLon = 156 and bottomRightLon = -118
  • topLeftLon = -204 and bottomRightLon = -121
  • topLeftLon = -703 and bottomRightLon = 624

@davidmoten
Copy link
Owner

Cool, thanks. I'll have a look now.

Copy link
Owner

@davidmoten davidmoten left a comment

Choose a reason for hiding this comment

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

Looks great, thanks! I'll merge now, upgrade a few libraries then release probably in a matter of hours. thanks for the contribution.

@davidmoten davidmoten merged commit 3ba1a44 into davidmoten:master Sep 5, 2021
@davidmoten
Copy link
Owner

a bit of work to updating plugins etc but i'll release within the next day or so

@davidmoten
Copy link
Owner

0.8 on Maven Central now

@guilhem-lk guilhem-lk deleted the fix-antimeridian-coverage branch September 6, 2021 07:43
@guilhem-lk
Copy link
Contributor Author

Great, thank you very much for this super fast review! :)

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.

2 participants