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

Implement VincentyDistance and VincentyLength #213

Merged
merged 3 commits into from
May 19, 2018
Merged

Implement VincentyDistance and VincentyLength #213

merged 3 commits into from
May 19, 2018

Conversation

frewsxcv
Copy link
Member

@frewsxcv frewsxcv commented May 11, 2018

@frewsxcv frewsxcv force-pushed the vincenty branch 2 times, most recently from 98cf7d7 to 8cd4be4 Compare May 18, 2018 22:34
@frewsxcv
Copy link
Member Author

@urschrei Do you feel comfortable reviewing this?

@urschrei
Copy link
Member

Vincenty can fail to converge for certain antipodal points (see here: geopy/geopy#30), and Charles Karney has actually provided a method that's guaranteed to converge in Geopy. I think we should return a Result if we keep Vincenty, or switch to Karney's method (it doesn't look particularly more complex, but I understand if you don't want to re-implement now).

@frewsxcv
Copy link
Member Author

I think we should return a Result if we keep Vincenty

Done in bcbbbd9

or switch to Karney's method (it doesn't look particularly more complex, but I understand if you don't want to re-implement now)

I think we should add this in too, so I opened #234

@urschrei
Copy link
Member

lgtm!

@urschrei
Copy link
Member

bors r=urschrei

bors bot added a commit that referenced this pull request May 19, 2018
213: Implement VincentyDistance and VincentyLength r=urschrei a=frewsxcv

Fixes #205

Based off this implementation: https://github.com/janantala/GPS-distance/blob/master/java/Distance.java

Co-authored-by: Corey Farwell <coreyf@rwell.org>
@bors
Copy link
Contributor

bors bot commented May 19, 2018

Build succeeded

@bors bors bot merged commit b8b862a into master May 19, 2018
@frewsxcv frewsxcv deleted the vincenty branch May 19, 2018 17:34
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