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

upgrade to h3 v4.2.0 #432

Merged
merged 5 commits into from
Dec 29, 2024
Merged

upgrade to h3 v4.2.0 #432

merged 5 commits into from
Dec 29, 2024

Conversation

isaacbrodsky
Copy link
Collaborator

No description provided.

Comment on lines 480 to 481
flags : int
Containment mode flags
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

flags seems like it needs an enum

@isaacbrodsky
Copy link
Collaborator Author

Coverage seems to be getting 403 which I don't think is related to this change.

@ajfriend
Copy link
Contributor

I was looking for docs on the containment modes and realized that we don't have it documented. As part of this effort, I think we should do:

@@ -467,7 +467,7 @@ def uncompact_cells(cells, res):
return _out_collection(hu)


def h3shape_to_cells(h3shape, res):
def h3shape_to_cells(h3shape, res, flags=0, experimental=False):
Copy link
Contributor

@ajfriend ajfriend Dec 22, 2024

Choose a reason for hiding this comment

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

I'm not sure if this should be the final form of the API. For example, we might not use integers for the flags, and we may drop the experimental option.

But it would be nice to get this functionality out so folks can start playing with it.

For the time being, could we leave h3shape_to_cells unchanged, and just create a h3shape_to_cells_experimental where we're free to change the API in the future?

@ajfriend
Copy link
Contributor

This looks great. thanks!

@ajfriend ajfriend merged commit ef6e897 into uber:master Dec 29, 2024
41 of 42 checks passed
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.

3 participants