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

Implement the traversal subcommands for the H3 cli #839

Merged
merged 13 commits into from
May 24, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 4 additions & 12 deletions CMakeTests.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -228,18 +228,10 @@ add_h3_test(testGridDistanceExhaustive src/apps/testapps/testGridDistanceExhaust
add_h3_test(testH3CellAreaExhaustive src/apps/testapps/testH3CellAreaExhaustive.c)
add_h3_test(testCellToBBoxExhaustive src/apps/testapps/testCellToBBoxExhaustive.c)

add_h3_cli_test(testCliCellToLatLng "cellToLatLng -c 8928342e20fffff" "POINT(-122.5003039349 37.5012466151)")
add_h3_cli_test(testCliLatLngToCell "latLngToCell --lat 20 --lng 123 -r 2" "824b9ffffffffff")
add_h3_cli_test(testCliCellToBoundary "cellToBoundary -c 8928342e20fffff" "POLYGON((-122.4990471431 37.4997389893, -122.4979805011 37.5014245698, -122.4992373065 37.5029321860, -122.5015607527 37.5027541980, -122.5026273256 37.5010686174, -122.5013705214 37.4995610248, -122.4990471431 37.4997389893))")
add_h3_cli_test(testCliGetResolution "getResolution -c 85283473fffffff" "5")
add_h3_cli_test(testCliGetBaseCellNumber "getBaseCellNumber -c 85283473fffffff" "20")
add_h3_cli_test(testCliStringToInt "stringToInt -c 85283473fffffff" "599686042433355775")
add_h3_cli_test(testCliIntToString "intToString -c 599686042433355775" "85283473fffffff")
add_h3_cli_test(testCliIsValidCell "isValidCell -c 85283473fffffff" "true")
add_h3_cli_test(testCliIsNotValidCell "isValidCell -c 85283473ffff" "false")
add_h3_cli_test(testCliIsResClassIII "isResClassIII -c 85283473fffffff" "true")
add_h3_cli_test(testCliIsPentagon "isPentagon -c 85283473fffffff" "false")
add_h3_cli_test(testCliGetIcosahedronFaces "getIcosahedronFaces -c 81743ffffffffff" "3, 8, 13, 9, 4")
file(GLOB cli_tests tests/cli/*.txt)
foreach(file ${cli_tests})
include(${file})
endforeach()

if(BUILD_ALLOC_TESTS)
add_h3_library(h3WithTestAllocators test_prefix_)
Expand Down
323 changes: 323 additions & 0 deletions src/apps/filters/h3.c
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,320 @@ SUBCOMMAND(getIcosahedronFaces,
return E_SUCCESS;
}

/// Traversal subcommands

SUBCOMMAND(
gridDisk,
"Returns a JSON array of a H3 cells within 'k' steps of the origin cell") {
DEFINE_CELL_ARG(cell, cellArg);
int k = 0;
Arg kArg = {.names = {"-k"},
.required = true,
.scanFormat = "%d",
.valueName = "distance",
.value = &k,
.helpText = "Maximum grid distance for the output set"};
Arg *args[] = {&gridDiskArg, &helpArg, &cellArg, &kArg};
PARSE_SUBCOMMAND(argc, argv, args);
int64_t len = 0;
H3Error err = H3_EXPORT(maxGridDiskSize)(k, &len);
if (err) {
return err;
}
H3Index *out = calloc(len, sizeof(H3Index));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need to check for failed memory allocation here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Technically yes, but the app is going to die if this happens no matter what if this happens. But I'd also assume anybody on a machine that cannot allocate 8 bytes is having a much harder time, which is why I left that error handling out?

Copy link
Collaborator

Choose a reason for hiding this comment

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

This could be much larger than 8 bytes with larger values of k. I think the program is better behaved if it explicitly tells the user that memory allocation failed rather than risk what happens with segfaulting, as in that case the user would have a much harder time tracking down what happened (they would need to attach a debugger to find out the problem is the k value.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Addressed. There's now a print to stderr and an exit for each if the pointer is NULL.

err = H3_EXPORT(gridDisk)(cell, k, out);
if (err) {
free(out);
return err;
}
printf(
"[ \"%" PRIx64 "\"",
out[0]); // TODO: Theoretically this may be zero, but I know it isn't
dfellis marked this conversation as resolved.
Show resolved Hide resolved
for (int64_t i = 1; i < len; i++) {
if (out[i] != 0) {
printf(", \"%" PRIx64 "\"", out[i]);
}
}
free(out);
printf(" ]");
return E_SUCCESS;
}

SUBCOMMAND(
gridDiskDistances,
"Returns a JSON array of arrays of H3 cells, each array containing cells "
"'k' steps away from the origin cell, based on the outer array index") {
DEFINE_CELL_ARG(cell, cellArg);
int k = 0;
Arg kArg = {.names = {"-k"},
.required = true,
.scanFormat = "%d",
.valueName = "distance",
.value = &k,
.helpText = "Maximum grid distance for the output set"};
Arg prettyArg = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

FWIW, I'd skip - interested callers have access to plenty of other pretty-printing tools

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Probably true. But I wasn't sure if we should rely on the users having jq, especially if they're more on the traditional geospatial/data analyst side of things and less familiar with the terminal.

.names = {"-p", "--pretty-print"},
.required = false,
.helpText =
"Determine if the JSON output should be pretty printed or not"};
Arg *args[] = {&gridDiskDistancesArg, &helpArg, &cellArg, &kArg,
&prettyArg};
PARSE_SUBCOMMAND(argc, argv, args);
bool pretty = prettyArg.found;
int64_t len = 0;
H3Error err = H3_EXPORT(maxGridDiskSize)(k, &len);
if (err) {
return err;
}
H3Index *out = calloc(len, sizeof(H3Index));
dfellis marked this conversation as resolved.
Show resolved Hide resolved
int *distances = calloc(len, sizeof(int));
err = H3_EXPORT(gridDiskDistances)(cell, k, out, distances);
if (err) {
free(out);
free(distances);
return err;
}
// Man, I wish JSON allowed trailing commas
printf("[%s", pretty ? "\n" : "");
for (int i = 0; i <= k; i++) {
printf("%s[%s", /* prefix */ pretty ? " " : "",
/* suffix */ pretty ? "\n" : "");
// We need to figure out how many cells are in each ring. Because of
// pentagons, we can't hardwire this, unfortunately
int count = 0;
for (int j = 0; j < len; j++) {
if (distances[j] == i && out[j] != 0) {
count++;
}
}
// On the second loop, we output cells with a comma except for the last
// one, which we now know
int cellNum = 0;
for (int j = 0; j < len; j++) {
if (distances[j] == i && out[j] != 0) {
cellNum++;
printf("%s\"%" PRIx64 "\"", pretty ? " " : "", out[j]);
if (cellNum == count) {
if (pretty) {
printf("\n");
}
} else {
printf(",%s", pretty ? "\n" : "");
}
}
}
// Similarly, we need to check which iteration of the outer array we're
// on and include a comma or not
if (i == k) {
printf("%s]%s", /* prefix */ pretty ? " " : "",
/* suffix */ pretty ? "\n" : "");
} else {
printf("%s],%s", /* prefix */ pretty ? " " : "",
/* suffix */ pretty ? "\n" : "");
}
}
printf("]\n"); // Always print the newline here so the terminal prompt gets
// its own line
free(out);
free(distances);
return E_SUCCESS;
}

SUBCOMMAND(gridRing,
"Returns a JSON array of H3 cells, each cell 'k' steps away from "
"the origin cell") {
DEFINE_CELL_ARG(cell, cellArg);
int k = 0;
Arg kArg = {.names = {"-k"},
.required = true,
.scanFormat = "%d",
.valueName = "distance",
.value = &k,
.helpText = "Maximum grid distance for the output set"};
Arg *args[] = {&gridRingArg, &helpArg, &cellArg, &kArg};
PARSE_SUBCOMMAND(argc, argv, args);
int64_t len = k == 0 ? 1 : 6 * k; // The length is fixed for gridRingUnsafe
// since it doesn't support pentagons
H3Index *out = calloc(len, sizeof(H3Index));
dfellis marked this conversation as resolved.
Show resolved Hide resolved
H3Error err = H3_EXPORT(gridRingUnsafe)(cell, k, out);
if (err) {
// For the CLI, we'll just do things less efficiently if there's an
// error here. If you use `gridDiskDistances` and only pay attention to
// the last array, it's equivalent to a "safe" gridRing call, but
// consumes a lot more temporary memory to do it
int64_t templen = 0;
err = H3_EXPORT(maxGridDiskSize)(k, &templen);
if (err) {
// But we abort if anything fails in here
free(out);
return err;
}
H3Index *temp = calloc(templen, sizeof(H3Index));
int *distances = calloc(templen, sizeof(int));
dfellis marked this conversation as resolved.
Show resolved Hide resolved
err = H3_EXPORT(gridDiskDistances)(cell, k, temp, distances);
if (err) {
free(out);
free(temp);
free(distances);
return err;
}
// Now, we first re-zero the `out` array in case there's garbage
// anywhere in it from the failed computation. Then we scan through the
// gridDisk output and copy the indexes that are the correct distance
// in. We *should* only be in this path when there's a pentagon
// involved, so we expect the true length of the array to be less than
// what was allocated for `out` in this scenario.
for (int i = 0; i < len; i++) {
out[i] = 0;
}
int64_t count = 0;
for (int64_t i = 0; i < templen; i++) {
if (distances[i] == k && temp[i] != 0) {
out[count] = temp[i];
count++;
}
}
len = count;
free(temp);
free(distances);
}
// Now that we have the correct data, however we got it, we can print it out
printf("[ \"%" PRIx64 "\"", out[0]);
for (int64_t i = 1; i < len; i++) {
if (out[i] != 0) {
printf(", \"%" PRIx64 "\"", out[i]);
}
}
free(out);
printf(" ]");
return E_SUCCESS;
}

SUBCOMMAND(gridPathCells,
"Returns a JSON array of H3 cells from the origin cell to the "
"destination cell (inclusive)") {
H3Index origin = 0;
Arg originArg = {.names = {"-o", "--origin"},
.required = true,
.scanFormat = "%" PRIx64,
.valueName = "cell",
.value = &origin,
.helpText = "The origin H3 cell"};
H3Index destination = 0;
Arg destinationArg = {.names = {"-d", "--destination"},
.required = true,
.scanFormat = "%" PRIx64,
.valueName = "cell",
.value = &destination,
.helpText = "The destination H3 cell"};
Arg *args[] = {&gridPathCellsArg, &helpArg, &originArg, &destinationArg};
PARSE_SUBCOMMAND(argc, argv, args);
int64_t len = 0;
H3Error err = H3_EXPORT(gridPathCellsSize)(origin, destination, &len);
if (err) {
return err;
}
H3Index *out = calloc(len, sizeof(H3Index));
dfellis marked this conversation as resolved.
Show resolved Hide resolved
err = H3_EXPORT(gridPathCells)(origin, destination, out);
if (err) {
free(out);
return err;
}
printf("[ \"%" PRIx64 "\"", out[0]);
for (int64_t i = 1; i < len; i++) {
if (out[i] != 0) {
printf(", \"%" PRIx64 "\"", out[i]);
}
}
free(out);
printf(" ]");
return E_SUCCESS;
}

SUBCOMMAND(gridDistance,
"Returns the number of steps along the grid to move from the origin "
"cell to the destination cell") {
H3Index origin = 0;
Arg originArg = {.names = {"-o", "--origin"},
.required = true,
.scanFormat = "%" PRIx64,
.valueName = "cell",
.value = &origin,
.helpText = "The origin H3 cell"};
H3Index destination = 0;
Arg destinationArg = {.names = {"-d", "--destination"},
.required = true,
.scanFormat = "%" PRIx64,
.valueName = "cell",
.value = &destination,
.helpText = "The destination H3 cell"};
Arg *args[] = {&gridDistanceArg, &helpArg, &originArg, &destinationArg};
PARSE_SUBCOMMAND(argc, argv, args);
int64_t distance = 0;
H3Error err = H3_EXPORT(gridDistance)(origin, destination, &distance);
if (err) {
return err;
}
printf("%" PRIx64, distance);
return E_SUCCESS;
dfellis marked this conversation as resolved.
Show resolved Hide resolved
}

SUBCOMMAND(cellToLocalIj,
"Returns the IJ coordinate for a cell anchored to an origin cell") {
DEFINE_CELL_ARG(cell, cellArg);
H3Index origin = 0;
Arg originArg = {.names = {"-o", "--origin"},
.required = true,
.scanFormat = "%" PRIx64,
.valueName = "cell",
.value = &origin,
.helpText = "The origin H3 cell"};
Arg *args[] = {&cellToLocalIjArg, &helpArg, &cellArg, &originArg};
PARSE_SUBCOMMAND(argc, argv, args);
CoordIJ out = {0};
H3Error err = H3_EXPORT(cellToLocalIj)(origin, cell, 0, &out);
if (err) {
return err;
}
printf("[%i, %i]\n", out.i, out.j);
return E_SUCCESS;
}

SUBCOMMAND(localIjToCell,
"Returns the H3 index from a local IJ coordinate anchored to an "
"origin cell") {
H3Index origin = 0;
Arg originArg = {.names = {"-o", "--origin"},
.required = true,
.scanFormat = "%" PRIx64,
.valueName = "cell",
.value = &origin,
.helpText = "The origin H3 cell"};
int i, j;
Arg iArg = {.names = {"-i"},
.required = true,
.scanFormat = "%d",
.valueName = "i",
.value = &i,
.helpText = "The I dimension of the IJ coordinate"};
Arg jArg = {.names = {"-j"},
.required = true,
.scanFormat = "%d",
.valueName = "j",
.value = &j,
.helpText = "The J dimension of the IJ coordinate"};
Arg *args[] = {&localIjToCellArg, &helpArg, &originArg, &iArg, &jArg};
PARSE_SUBCOMMAND(argc, argv, args);
CoordIJ in = {.i = i, .j = j};
H3Index out = 0;
H3Error err = H3_EXPORT(localIjToCell)(origin, &in, 0, &out);
if (err) {
return err;
}
h3Println(out);
return E_SUCCESS;
}

// TODO: Is there any way to avoid this particular piece of duplication?
SUBCOMMANDS_INDEX

Expand All @@ -366,6 +680,15 @@ SUBCOMMAND_INDEX(isResClassIII)
SUBCOMMAND_INDEX(isPentagon)
SUBCOMMAND_INDEX(getIcosahedronFaces)

/// Traversal subcommands
SUBCOMMAND_INDEX(gridDisk)
SUBCOMMAND_INDEX(gridDiskDistances)
SUBCOMMAND_INDEX(gridRing)
SUBCOMMAND_INDEX(gridPathCells)
SUBCOMMAND_INDEX(gridDistance)
SUBCOMMAND_INDEX(cellToLocalIj)
SUBCOMMAND_INDEX(localIjToCell)

END_SUBCOMMANDS_INDEX

int main(int argc, char *argv[]) {
Expand Down
1 change: 1 addition & 0 deletions tests/cli/cellToBoundary.txt
dfellis marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
add_h3_cli_test(testCliCellToBoundary "cellToBoundary -c 8928342e20fffff" "POLYGON((-122.4990471431 37.4997389893, -122.4979805011 37.5014245698, -122.4992373065 37.5029321860, -122.5015607527 37.5027541980, -122.5026273256 37.5010686174, -122.5013705214 37.4995610248, -122.4990471431 37.4997389893))")
1 change: 1 addition & 0 deletions tests/cli/cellToLatLng.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
add_h3_cli_test(testCliCellToLatLng "cellToLatLng -c 8928342e20fffff" "POINT(-122.5003039349 37.5012466151)")
1 change: 1 addition & 0 deletions tests/cli/getBaseCellNumber.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
add_h3_cli_test(testCliGetBaseCellNumber "getBaseCellNumber -c 85283473fffffff" "20")
1 change: 1 addition & 0 deletions tests/cli/getIcosahedronFaces.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
add_h3_cli_test(testCliGetIcosahedronFaces "getIcosahedronFaces -c 81743ffffffffff" "3, 8, 13, 9, 4")
1 change: 1 addition & 0 deletions tests/cli/getResolution.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
add_h3_cli_test(testCliGetResolution "getResolution -c 85283473fffffff" "5")
1 change: 1 addition & 0 deletions tests/cli/intToString.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
add_h3_cli_test(testCliIntToString "intToString -c 599686042433355775" "85283473fffffff")
1 change: 1 addition & 0 deletions tests/cli/isPentagon.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
add_h3_cli_test(testCliIsPentagon "isPentagon -c 85283473fffffff" "false")
1 change: 1 addition & 0 deletions tests/cli/isResClassIII.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
add_h3_cli_test(testCliIsResClassIII "isResClassIII -c 85283473fffffff" "true")
2 changes: 2 additions & 0 deletions tests/cli/isValidCell.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
add_h3_cli_test(testCliIsValidCell "isValidCell -c 85283473fffffff" "true")
add_h3_cli_test(testCliIsNotValidCell "isValidCell -c 85283473ffff" "false")
1 change: 1 addition & 0 deletions tests/cli/latLngToCell.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
add_h3_cli_test(testCliLatLngToCell "latLngToCell --lat 20 --lng 123 -r 2" "824b9ffffffffff")
1 change: 1 addition & 0 deletions tests/cli/stringToInt.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
add_h3_cli_test(testCliStringToInt "stringToInt -c 85283473fffffff" "599686042433355775")
Loading