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

Rename distance* functions to greatCircleDistance* #622

Merged
merged 5 commits into from
Jul 14, 2022

Conversation

nrabinowitz
Copy link
Collaborator

Per offline discussion: The intent of this change is to disambiguate the point-to-point distance functions from the grid distance functions, particularly in binding contexts where the unit suffix may be lost.

This was just a straight search-and-replace, no other code changes.

@coveralls
Copy link

coveralls commented Jul 5, 2022

Coverage Status

Coverage increased (+0.0004%) to 98.66% when pulling 57ac6c7 on nrabinowitz:lat-lng-distance into 1e661dc on uber:master.

@@ -319,14 +319,14 @@ DECLSPEC double H3_EXPORT(radsToDegs)(double radians);
* @{
*/
/** @brief "great circle distance" between pairs of LatLng points in radians*/
DECLSPEC double H3_EXPORT(distanceRads)(const LatLng *a, const LatLng *b);
DECLSPEC double H3_EXPORT(latLngDistanceRads)(const LatLng *a, const LatLng *b);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please update the docs - API docs, upgrade instructions, and CHANGELOG.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done, thanks. When upgrading the docs I assumed that JS and Java would use latLngDistance and Python would use latlng_distance.

@ajfriend
Copy link
Contributor

ajfriend commented Jul 6, 2022

Sorry, I'm waffling on this, but I'm currently leaning toward haversineDistance, even if that might refer to a specific formula. I think it's less ambiguous than LatLng and seems to be generally accepted as the right name for this sort of thing:

Too late to consider this version?

@ajfriend
Copy link
Contributor

ajfriend commented Jul 6, 2022

I guess "haversine" isn't used in the the OGC names. Its STDistance: https://docs.microsoft.com/en-us/previous-versions/sql/sql-server-2008/bb933808(v=sql.100)

...but i don't think we're considering calling it STDistance here :)

@nrabinowitz nrabinowitz changed the title Rename distance* functions to latLngDistance* Rename distance* functions to greatCircleDistance* Jul 13, 2022
@nrabinowitz nrabinowitz merged commit 3983476 into uber:master Jul 14, 2022
@nrabinowitz nrabinowitz deleted the lat-lng-distance branch July 14, 2022 01:07
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.

None yet

5 participants