Skip to content

Updating "README.md" to avoid confusion#1054

Merged
ajfriend merged 4 commits intouber:masterfrom
benguild:patch-1
Oct 13, 2025
Merged

Updating "README.md" to avoid confusion#1054
ajfriend merged 4 commits intouber:masterfrom
benguild:patch-1

Conversation

@benguild
Copy link
Copy Markdown
Contributor

@benguild benguild commented Oct 6, 2025

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Oct 6, 2025

CLA assistant check
All committers have signed the CLA.

@coveralls
Copy link
Copy Markdown

coveralls commented Oct 6, 2025

Coverage Status

coverage: 98.934%. remained the same
when pulling 3861da0 on benguild:patch-1
into 4c56ad1 on uber:master.

@ajfriend
Copy link
Copy Markdown
Collaborator

ajfriend commented Oct 6, 2025

Thanks for this! I'm trying to think through the best way to represent this in the docs. What about adding a note about this under the hierarchical bullet in the Highlights section, and then linking to the docs for more details, including Nicks Observable page. I'm also wondering if the /docs/highlights/indexing page might be better named as something like /docs/highlights/hierarchical_containment. What do folks think? @isaacbrodsky @dfellis @nrabinowitz

Maybe something like:

Highlights

@benguild
Copy link
Copy Markdown
Contributor Author

benguild commented Oct 6, 2025

That's not clear enough, honestly. You could say that but it's probably best to reinforce it with an even simpler explanation of the limitations afterward… in that the children are not an exact fit to the parent, and vice versa, etc.

As noted in my comment #1053 (comment), this should really be introduced as a limitation and clarification as early in the discovery process as possible.

@ajfriend
Copy link
Copy Markdown
Collaborator

ajfriend commented Oct 6, 2025

I agree this should be as obvious as quickly as possible. I'm just trying to craft a good, short warning up front that would fit well in an readme page, that we can then expand on immediately in the linked page. And we may want to also update the linked page.

How about:

Highlights

  • H3 is a hierarchical geospatial index, providing exact logical containment across the cell hierarchy, and approximate geometric containment (e.g., children are not entirely contained in their parents).

@isaacbrodsky
Copy link
Copy Markdown
Collaborator

I agree this should be as obvious as quickly as possible. I'm just trying to craft a good, short warning up front that would fit well in an readme page, that we can then expand on immediately in the linked page. And we may want to also update the linked page.

How about:

Highlights

* H3 is a [hierarchical geospatial index](/docs/highlights/indexing), providing **exact logical** containment across the cell hierarchy, and **approximate geometric** containment (e.g., children are not entirely contained in their parents).

This looks good but I think linking "approximate geometric" to Nick's Observable may be helpful. That phrase may not be immediately obvious what it means, and having a graphic example for it could be very informative.

We definitely also want to improve the docs pages :)

@benguild
Copy link
Copy Markdown
Contributor Author

benguild commented Oct 6, 2025

  • H3 is a hierarchical geospatial index, providing exact logical containment across the cell hierarchy, and approximate geometric containment (e.g., children are not entirely contained in their parents).

How about something like:

  • H3 is a hierarchical geospatial index, which provides exact logical containment but approximate geometric containment across the cell hierarchy. ⚠️ Children are not entirely contained by their parent cells, and points within them may in fact map to adjacent parents instead.

@ajfriend
Copy link
Copy Markdown
Collaborator

ajfriend commented Oct 8, 2025

Based off the last few comments, how about something like this for the intro/readme page:

That being said, I still think @benguild is correct in that we should document this better (and improve upon the meager attempt above), as it seems to be a common gotcha for new users coming to the library. I'd be in favor of a new top-level docs page on "hierarchical containment" that makes it vividly clear to new users what the potential problems might be, while also explaining how to avoid or work with them. Alternatively, we can rename/rework the https://h3geo.org/docs/highlights/indexing page to make this issue more obvious.

What do folks think? I can create a task for it and give it a stab.

@isaacbrodsky
Copy link
Copy Markdown
Collaborator

Based off the last few comments, how about something like this for the intro/readme page:

* H3 is a [hierarchical geospatial index](/docs/highlights/indexing), which provides [**exact logical** containment but only **approximate geometric** containment](https://observablehq.com/@nrabinowitz/h3-hierarchical-non-containment) across the cell hierarchy. **That is, child cells are not always geometrically contained by their parent cells, so points within them may map to other (adjacent) parent cells.**

This text seems overall good to me. I think the word "map" may be a little unclear (what does "map to other ... cells" mean?) Maybe "be logically contained by other (adjacent) parent cells"?

That being said, I still think @benguild is correct in that we should document this better (and improve upon the meager attempt above), as it seems to be a common gotcha for new users coming to the library. I'd be in favor of a new top-level docs page on "hierarchical containment" that makes it vividly clear to new users what the potential problems might be, while also explaining how to avoid or work with them. Alternatively, we can rename/rework the https://h3geo.org/docs/highlights/indexing page to make this issue more obvious.

What do folks think? I can create a task for it and give it a stab.

I would love to see more and improved docs pages to guide new users through H3! I find we frequently direct people to @nrabinowitz 's great notebook visualizations to explain things, so it would be great to have all of that in one place.

@ajfriend
Copy link
Copy Markdown
Collaborator

Good point on "map". How about:

@isaacbrodsky
Copy link
Copy Markdown
Collaborator

Good point on "map". How about:

I think that's a good way to put it.

@benguild
Copy link
Copy Markdown
Contributor Author

I think this is important enough that it should probably be its own bulletpoint. Placing it in the second sentence frames it as a minor detail, when in fact people keep hitting this issue.

Simply put, it’s unusual that the child cells don’t fit the parents. While there are advantages in this design, you should highlight this as a key difference aside from the fact that this offers a "hierarchical geospatial index" when it's not strictly so. Anybody who looks at this for the first time needs to know that, and I'm concerned about the pattern of folks not knowing it.

I'm just trying to be fairly direct here, since the purpose of the PR is to highlight this key detail to avoid others falling into the same difference in expectation again. We can work on the language, but we should agree on whether it's 1 or 2 bulletpoints first.

@ajfriend
Copy link
Copy Markdown
Collaborator

Honestly, I think that's splitting hairs. In the context of an intro page, the language above makes it quite clear, especially if we bold the second sentence (and we should), which calls it out better than making a new bullet, IMO.

I'd propose the language above, with the second sentence in bold.

We can see how that goes, and make further changes if necessary. But my guess is that further changes on the intro page will have less effect than a new top-level docs page on "hierarchical containment" that we can link to, and that's where we should focus our time.

@ajfriend
Copy link
Copy Markdown
Collaborator

And here's a ticket for what I think is the more important docs issue: #1057

@benguild
Copy link
Copy Markdown
Contributor Author

the language above makes it quite clear, especially if we bold the second sentence (and we should)

I think with the bolding… @ajfriend, @isaacbrodsky, and I are in reasonable agreement here. I tweaked the language in the diff based on folks' thoughts, and would suggest that latest version. Can revise further if folks think it's less clear.

@ajfriend
Copy link
Copy Markdown
Collaborator

Looks great. And thanks again for raising! We're always interested in suggestions to improve the docs (even if there's some back and forth on how to do it 😀).

@benguild
Copy link
Copy Markdown
Contributor Author

No problem. Can merge today if there are enough reviews completed.

@ajfriend ajfriend merged commit 5af8ee5 into uber:master Oct 13, 2025
45 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.

5 participants