Skip to content

Rename distance* functions to greatCircleDistance*#622

Merged
nrabinowitz merged 5 commits into
uber:masterfrom
nrabinowitz:lat-lng-distance
Jul 14, 2022
Merged

Rename distance* functions to greatCircleDistance*#622
nrabinowitz merged 5 commits into
uber:masterfrom
nrabinowitz:lat-lng-distance

Conversation

@nrabinowitz

Copy link
Copy Markdown
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

coveralls commented Jul 5, 2022

Copy link
Copy Markdown

Coverage Status

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

Comment thread src/h3lib/include/h3api.h.in Outdated
*/
/** @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
Copy Markdown
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
Copy Markdown
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

ajfriend commented Jul 6, 2022

Copy link
Copy Markdown
Collaborator

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

ajfriend commented Jul 6, 2022

Copy link
Copy Markdown
Collaborator

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.

5 participants