-
Notifications
You must be signed in to change notification settings - Fork 457
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
Version 3.2.0, add Local IJ to docs #149
Conversation
docs/api/localij.md
Outdated
axes are 120° apart. The origin index might not be at | ||
(0, 0) in the local coordinate system. | ||
|
||
LOcal IJ coordinates are only valid within a certain radius of the origin. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: LOcal
-> Local
docs/api/localij.md
Outdated
|
||
Produces local IJ coordinates for an index anchored by an origin. | ||
|
||
This function is experimental, and its output is not gauranteed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gauranteed
-> guaranteed
docs/api/localij.md
Outdated
|
||
Produces an index for local IJ coordinates anchored by an origin. | ||
|
||
This function is experimental, and its output is not gauranteed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gauranteed
-> guaranteed
docs/api/localij.md
Outdated
int experimentalLocalIjToH3(H3Index origin, const CoordIJ *ij, H3Index *out); | ||
``` | ||
|
||
Produces an index for local IJ coordinates anchored by an origin. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Might read better as Produces an H3 index from local IJ coordinates anchored by an origin.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor typos and one small suggestion.
docs/api/localij.md
Outdated
This function is experimental, and its output is not guaranteed | ||
to be compatible across different versions of H3. | ||
|
||
## h3Distance |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a little tricky. h3Distance
is a generally applicable function that many users may want without understanding a concept of local coordinates. I'm a little worried that we're burying it here because its implementation is based on IJ coords, rather than putting it somewhere that would make more sense to the reader.
Would it make more sense, for example, to rename Neighbors
to Traversal
and put this there?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, organizing this is not easy. While they are not connected from a use perspective, these functions have a similar set of limitations, which I wanted to make the user aware of.
I feel like if Neighbors
is renamed to Traversal
that could also encompass the Local IJ
functions. Maybe all of them should be on the same page?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I'd support that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Version 3.2.0, add Local IJ to docs
The Distance docs are replaced with docs for the Local IJ group of functions.
The coordinate systems documentation page is converted to use the expected single asterisk for italics, rather than underscores which aren't understood by Ocular.