All remaining subcommands for the h3 cli program #924
Conversation
|
Oh, this currently includes the region subcommands in this PR, since it depends on that one, but once it gets approved and merged, this'll be reduced to only the new commands. I did (almost) every command in a separate commit, for easier reviewing. |
dfellis
force-pushed
the
h3-cli-remaining-subcommands
branch
3 times, most recently
from
cd00c06
to
de30ae0
Compare
Co-authored-by: Isaac Brodsky <[email protected]>
Co-authored-by: Isaac Brodsky <[email protected]>
dfellis
force-pushed
the
h3-cli-remaining-subcommands
branch
from
d5b44ad
to
d63accf
Compare
| SUBCOMMAND(pentagonCount, "Returns 12") { | ||
| Arg *args[] = {&pentagonCountArg, &helpArg}; | ||
| PARSE_SUBCOMMAND(argc, argv, args); | ||
| printf("12\n"); | ||
| return E_SUCCESS; | ||
| } |
There was a problem hiding this comment.
not sure if we really need to bind this, but ok
There was a problem hiding this comment.
I personally find it hilarious, so I want it in. ;)
| Arg *args[] = {&isValidDirectedEdgeArg, &helpArg, &cellArg}; | ||
| PARSE_SUBCOMMAND(argc, argv, args); | ||
| bool isValid = H3_EXPORT(isValidDirectedEdge)(cell); | ||
| printf("%s", isValid ? "true\n" : "false\n"); |
There was a problem hiding this comment.
No strong opinion here, but would 1 and 0 be more flexible outputs?
| return err; | ||
| } | ||
| // Using WKT formatting for the output. TODO: Add support for JSON | ||
| // formatting |
There was a problem hiding this comment.
Is it weird that we use WKT here but raw lat,lng polygons for cellsToMultiPolygon?
| if (err) { | ||
| return err; | ||
| } | ||
| printf("%" PRIx64 "\n", out); |
There was a problem hiding this comment.
Nit: Should we use h3Println here?
| .helpText = "Angle in radians"}; | ||
| Arg *args[] = {&radsToDegsArg, &radArg, &helpArg}; | ||
| PARSE_SUBCOMMAND(argc, argv, args); | ||
| printf("%.10lf\n", H3_EXPORT(radsToDegs)(rad)); |
There was a problem hiding this comment.
Nit: Worth making a common printDouble function to help us always print with the same format?
| hasPrinted = true; | ||
| } | ||
| } | ||
| printf("]\n"); |
There was a problem hiding this comment.
For polygonToCells we print newline-delimited cells, but most of the multi-cell outputs here use JSON arrays. Should this be consistent?
| "to use greatCircleDistanceKm"); | ||
| exit(1); | ||
| } | ||
| FILE *fp = 0; |
There was a problem hiding this comment.
Nit: It feels like we could share a lot of code between greatCircleDistanceKm and greatCircleDistanceRads and greatCircleDistanceM
|
Discussed offline, some of the inconsistencies will be taken care of in a follow-up PR. |
After implementing the region subcommands, all of the rest documented on h3geo.org were "trivial" as all patterns of subcommand had been implemented, so I just slogged through writing all of them while I had the drive to do it. ;)
There may be some test failures with
degsToRadsand friends due to floating point rounding errors on different CPU architectures. I hope not, but if so, then I'll have to figure out how to test them better.