Skip to content

Conversation

ajfriend
Copy link
Collaborator

@ajfriend ajfriend commented Aug 16, 2025

Follow up from #1022: provides a function to create an H3 cell from (resolution, base cell number, and digit) components.

Relevant to #277

@ajfriend
Copy link
Collaborator Author

ajfriend commented Aug 16, 2025

Open questions:

  • Better function name?
  • How much validation do we want to do? I'm doing a few basic range checks on res, base cell, and digits, but do we also want to call isValidCell to ensure the output is valid? The downside would be the additional overhead of the check, and I expect that most users of this function will be creating cells from component data that they already know would produce a valid cell.
    • One potential use case for this function would be to create invalid cells (like deleted subsequences) for us to use in tests elsewhere, so we'd want this function (or maybe an internal-only version) to be able to create such cells. Thus, one option would be to restrict the domains of the inputs for this function to only those that are representable in the H3Index bit representation, even if it results in an invalid cell.
  • Errors: we have E_RES_DOMAIN, but the most specific domain error for base cell or digit being out of range is E_DOMAIN. Maybe we should create additional, more specific errors for these two cases.

@ajfriend ajfriend marked this pull request as draft August 16, 2025 19:42
@coveralls
Copy link

coveralls commented Aug 16, 2025

Coverage Status

coverage: 98.944% (+0.01%) from 98.934%
when pulling 3a456d5 on ajfriend:cell_from_components
into 50d4dbe on uber:master.

@ajfriend
Copy link
Collaborator Author

cc: @zachasme

@zachasme
Copy link
Contributor

This would definitely cover my use-case! 👍

Comment on lines 136 to 140
// Optional: isValidCells is a more expensive test. do we want to run it
// every time?
// if (!isValidCell(h)) {
// return E_CELL_INVALID;
// }
Copy link
Collaborator

Choose a reason for hiding this comment

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

My preference would be to allow users to specify invalid indexes if desired, as that could be used with getIndexDigit

Copy link
Collaborator

Choose a reason for hiding this comment

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

If you want to only generate valid cells, I think the only missing check is for the pentagon deleted subequence, so the full call to isValidCell might not be needed

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, I like the idea of this function only returning valid cells. I've reworked the code towards that.

@ajfriend
Copy link
Collaborator Author

Update:

  • The function should only be able to create valid H3 cells that pass isValidCell(). All other malformed inputs should raise an error.
  • Add a few new H3Error codes for more specific error information: E_BASE_CELL_DOMAIN, E_DIGIT_DOMAIN, E_DELETED_DIGIT

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants