Conversation
Implement a general validation function that checks if an H3 index is valid for any mode (cell, directed edge, or vertex). Changes: - Add isValidIndex() function declaration to h3api.h.in - Implement isValidIndex() in h3Index.c - Add includes for directedEdge.h and vertex.h in h3Index.c - Add comprehensive test coverage in testH3Index.c The function returns true if the index is valid as a cell, directed edge, or vertex, providing a single validation function for any H3 index type. Fixes uber#1043
|
|
src/apps/testapps/testH3Index.c
Outdated
| // Test with valid directed edge | ||
| H3Index neighbor; | ||
| t_assertSuccess(H3_EXPORT(latLngToCell)(&coord, 5, &neighbor)); | ||
| neighbor++; // Get a neighboring cell |
There was a problem hiding this comment.
This won't work. Simply incrementing will almost never give you a neighboring cell.
There was a problem hiding this comment.
@ajfriend I just pushed a commit to fix this. Thanks for the feedback. I used gridRingUnsafe to call the neighboring cells. Let me know what you think.
Replace incorrect neighbor++ with proper gridRingUnsafe call to get an actual neighboring cell for directed edge testing. Simply incrementing an H3Index will almost never produce a valid neighboring cell. This addresses the review feedback from ajfriend on lines 224-227.
|
Thanks for this! It looks like you may need to run/test this locally to get the formatting that the CI is expecting. |
Apply clang-format style to match project formatting conventions: - Put first two conditions on same line - Wrap long comment to 80 characters
|
@ajfriend happy to contribute. I fixed the formatting manually based on the logs then confirmed using clang-format. CI should hopefully pass now. |
Apply clang-format style to isValidIndex documentation comment: - Wrap comment across 3 lines for proper formatting
|
CI formatting failed on the header file, I re-formatted it with clang-format. |
isaacbrodsky
left a comment
There was a problem hiding this comment.
Thanks! Please note we will need to add this to fuzzers (e.g. https://github.com/uber/h3/blob/master/src/apps/fuzzers/fuzzerCellProperties.c) & docs as well.
|
I'll land this now. Thanks @cnaples79! I've added #1058, #1059, #1060, which anyone should feel free to take as a follow-up. |
Summary
Implements a general validation function
isValidIndex()that checks if an H3 index is valid for any mode (cell, directed edge, or vertex), as requested in #1043.Changes Made
isValidIndex()function declaration toh3api.h.inh3Index.cthat checks all three validation types:directedEdge.handvertex.htoh3Index.ctestH3Index.ccovering:API Usage
Test Plan
isValidIndexintestH3Index.cFixes #1043