Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Pythagorean distance is confusing and not useful #27

Closed
moveson opened this issue Nov 5, 2017 · 5 comments
Closed

Pythagorean distance is confusing and not useful #27

moveson opened this issue Nov 5, 2017 · 5 comments

Comments

@moveson
Copy link
Contributor

moveson commented Nov 5, 2017

The TrackPoint#pythagorean_distance_from method returns units in radians, although the comment line above it indicates it returns units in km.

Even if the result is multiplied by RADIUS to return km, the distance is highly inaccurate at most latitudes, because the method assumes a degree of longitude is equal to ~111 km regardless of latitude. For example, at 37 degrees latitude (the latitude of the points in the test file), calculations between points with significant difference in longitude are off by about 26%.

I would suggest the TrackPoint#pythagorean_distance_from method be deleted to avoid confusion. This would require removal of Segment#pythagorean_distance as well. Comments note that this method is not currently in use.

@moveson moveson changed the title Pythagorean distance is not useful Pythagorean distance is confusing and not useful Nov 6, 2017
@andrewhao
Copy link
Collaborator

Hi @moveson - thanks for your feedback. I believe you're correct - pythagorean distance is not useful for nearly all use cases (unless someone here chimes in otherwise). I'll take that into consideration that we may want to delete the method, or at least update the documentation.

@moveson
Copy link
Contributor Author

moveson commented Nov 15, 2017

That sounds good. To be clear, though, the method in its current state is not useful in any case. I spent a bit of time figuring out that it was broken, and then I spent quite a bit more time figuring out why it was broken. I'd like to avoid others spending that unnecessary time.

I'm happy to make the changes and submit a PR if that would be helpful.

@andrewhao
Copy link
Collaborator

If you could make a PR, that would be great. Thanks!

@moveson
Copy link
Contributor Author

moveson commented Nov 20, 2017

If you would authorize me to push to a new branch, I'll make a PR.

@andrewhao
Copy link
Collaborator

Hey @moveson - typically we accept PRs from forks - please feel free to fork this repo, make a change on a branch and open a PR from there. Thank you!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants