uber / h3

Hexagonal hierarchical geospatial indexing system
https://h3geo.org
Apache License 2.0
4.84k stars 459 forks source link

order-dependent duplicate error in compact_cells #697

Closed jleedev closed 1 year ago

jleedev commented 1 year ago

This set of cells (which contains no duplicates) is rejected by compact_cells, depending on the order.

import h3

print(h3.versions())
s = [
    "851ec90ffffffff",
    "851ec907fffffff",
    "851ec90bfffffff",
    "851ec917fffffff",
    "851ec913fffffff",
    "851ec91bfffffff",
    "841ec95ffffffff",
    "841ec93ffffffff",
    "841ec99ffffffff",
    "841ec9dffffffff",
    "841ec97ffffffff",
    "841ec9bffffffff",
]
h3.compact_cells(s)
h3.compact_cells(reversed(s))
 ¶ python3 ./a.py 
{'c': '4.0.0', 'python': '4.0.0b1'}
Traceback (most recent call last):
  File "/home/josh/junk/h3-asdf/./a.py", line 19, in <module>
    h3.compact_cells(reversed(s))
  File "/home/josh/.local/share/virtualenvs/h3-asdf-b-rNoeuA/lib/python3.10/site-packages/h3/api/_api_template.py", line 369, in compact_cells
    hc = _cy.compact_cells(hu)
  File "cells.pyx", line 203, in h3._cy.cells.compact_cells
  File "cells.pyx", line 220, in h3._cy.cells.compact_cells
  File "error_system.pyx", line 221, in h3._cy.error_system.check_for_error
h3._cy.error_system.H3DuplicateInputError
ajfriend commented 1 year ago

That is odd. This seems to be a core library issue.

Can anyone reproduce in other bindings? And does this persist in v4.0.1?

ajfriend commented 1 year ago

So this seems to be happening because the input is mixed resolution. Currently, compact_cells only works for inputs of cells all sharing the same resolution.

It doesn't look like we currently mention this in the docs. We should update them!

Screen Shot 2022-09-21 at 8 05 09 PM
ajfriend commented 1 year ago

https://github.com/uber/h3/pull/700

ajfriend commented 1 year ago

https://github.com/uber/h3-py/pull/286

jleedev commented 1 year ago

Ah, yes. I had misread from https://github.com/uber/h3/issues/298 that it should be idempotent, but clearly it's not supposed to be.

jleedev commented 1 year ago

https://github.com/uber/h3-js#module_h3.compactCells does mention the same resolution, already, I just can't read.

The error code leaves a little to be desired, though.

ajfriend commented 1 year ago

A future version of compact_cells will be idempotent, able to handle duplicates, and handle mixed resolutions, but that's blocked on me getting back to https://github.com/uber/h3/pull/552