Exposing index inspection/manipulation macros/functions

[zachasme]

I'm implementing (SP-)GiST index support for our PostgreSQL bindings and I find myself copying over macros for inspecting and manipulating index digits and resolution. I need these because I'm encoding each index digit as a branch in a tree, and reconstructing H3 indexes as the path from root to leaf node.

Have you considered exposing some of the tools for low-level manipulation of H3 indexes as part of the public API or maybe in a separate header file?

[dfellis]

Is there a problem with the h3-pg project depending on the h3Index.h file? I think everything you need is defined in that header file.

[zachasme]

Yes those are exactly the things I need. The only problem was that I couldn't figure out a way to install h3Index.h alongside h3api.h, and I would like to support linking against H3 built as a shared library at some point.

[isaacbrodsky]

To install h3Index.h, you can modify CMakeLists.txt before generating and make installing:

Change (around line 609):

install(
    FILES "${CMAKE_CURRENT_BINARY_DIR}/src/h3lib/include/h3api.h"
    DESTINATION "${include_install_dir}/h3"
)

To:

install(
    FILES "${CMAKE_CURRENT_BINARY_DIR}/src/h3lib/include/h3api.h" "${CMAKE_CURRENT_SOURCE_DIR}/src/h3lib/include/h3Index.h"
    DESTINATION "${include_install_dir}/h3"
)

I don't think that linking to H3 as a shared library should have an impact on using the macros or using internal H3 functions.

[dfellis]

Should that be a default, then @isaacbrodsky ?

[zachasme]

Unless it is on by default I cannot assume existence of h3Index.h even if h3 is already installed on the system.

For example on Arch Linux h3 might have been installed manually or using the aur package but I will have to instruct users to rebuild it using the changed CMakeLists.

I fully understand if it doesn't make sense for you to expose that file by default in it's current form.

[isaacbrodsky]

Only h3api.h was installed because that's the public API (edit: for semantic versioning). I think it would be fine to add the internal headers with clarity that they're internal. The drawback of this is that different library versions may break your build. Perhaps we could initially add something like:

# (First, define internal_header_files)
install(
    FILES ${internal_header_files}
    DESTINATION "${include_install_dir}/h3/internal"
)

Alternately maybe the macros could be promoted to part of the public API, and add them to h3api.h or a new, non-internal, header that's installed. The bitwise layout of the indexes is already part of the public API so that would be a small change.

[dfellis]

I think moving the macros to h3api.h is the best solution here.

[zachasme]

Would you accept a PR moving some of the macros into h3api.h?

[isaacbrodsky]

Would you accept a PR moving some of the macros into h3api.h?

Yes, I think that would be accepted.

[dfellis]

No blocking from me, sorry that I forgot about this.