Skip to content

Commit

Permalink
Replace _out_unordered and _out_ordered with _out_collection (#339)
Browse files Browse the repository at this point in the history
* Replace _out_unordered and _out_ordered with _out_collection

* update batch of tests

* lint

* few more tests

* group tests around APIs

* few more tests

* few more tests

* couple few

* final tests

* test_icosahedron_faces

* lint

* remove redundant tests

* consolidate same_set

* same_set

* update makefile

* remove the .so files

* Add note indicating that there is no guaranteed ordering of cells for many functions

* bump to pypa/[email protected]
  • Loading branch information
ajfriend authored Mar 10, 2024
1 parent 158f52d commit e64ff67
Show file tree
Hide file tree
Showing 25 changed files with 216 additions and 189 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/wheels.yml
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ jobs:

## Build

- uses: pypa/[email protected].2
- uses: pypa/[email protected].5
env:
CIBW_TEST_REQUIRES: pytest numpy
CIBW_TEST_COMMAND: pytest {project}/tests
Expand Down
4 changes: 3 additions & 1 deletion makefile
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@ clear:
-./env/bin/pip uninstall -y h3
-@rm -rf MANIFEST
-@rm -rf annotations
-@rm -rf .pytest_cache tests/__pycache__ __pycache__ _skbuild dist .coverage
-@rm -rf .pytest_cache _skbuild dist .coverage build
-@find . -type d -name '__pycache__' | xargs rm -r
-@find . -type d -name '*.egg-info' | xargs rm -r
-@find . -type f -name '*.pyc' | xargs rm -r
-@find . -type f -name '*.so' | xargs rm -r
-@find . -type d -name '*.ipynb_checkpoints' | xargs rm -r
-@find ./tests -type f -name '*.c' | xargs rm -r

Expand Down
2 changes: 1 addition & 1 deletion src/h3/_cy/cells.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ cpdef get_icosahedron_faces(H3int h):

# todo: wait? do faces start from 0 or 1?
# we could do this check/processing in the int_mv object
out = {f for f in faces if f >= 0}
out = [f for f in faces if f >= 0]

return out

Expand Down
61 changes: 46 additions & 15 deletions src/h3/api/basic_int/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@
_in_scalar,
_out_scalar,
_in_collection,
_out_unordered,
_out_ordered,
_out_collection,
)


Expand Down Expand Up @@ -258,7 +257,7 @@ def cell_to_boundary(h):

def grid_disk(h, k=1):
"""
Return unordered set of cells with H3 distance ``<= k`` from ``h``.
Return unordered collection of cells with H3 distance ``<= k`` from ``h``.
That is, the 'filled-in' disk.
Parameters
Expand All @@ -270,15 +269,19 @@ def grid_disk(h, k=1):
Returns
-------
unordered collection of H3Cell
Notes
-----
There is currently no guaranteed order of the output cells.
"""
mv = _cy.grid_disk(_in_scalar(h), k)

return _out_unordered(mv)
return _out_collection(mv)


def grid_ring(h, k=1):
"""
Return unordered set of cells with H3 distance ``== k`` from ``h``.
Return unordered collection of cells with H3 distance ``== k`` from ``h``.
That is, the "hollow" ring.
Parameters
Expand All @@ -290,15 +293,19 @@ def grid_ring(h, k=1):
Returns
-------
unordered collection of H3Cell
Notes
-----
There is currently no guaranteed order of the output cells.
"""
mv = _cy.grid_ring(_in_scalar(h), k)

return _out_unordered(mv)
return _out_collection(mv)


def cell_to_children(h, res=None):
"""
Children of a cell.
Children of a cell as an unordered collection.
Parameters
----------
Expand All @@ -310,10 +317,14 @@ def cell_to_children(h, res=None):
Returns
-------
unordered collection of H3Cell
Notes
-----
There is currently no guaranteed order of the output cells.
"""
mv = _cy.cell_to_children(_in_scalar(h), res)

return _out_unordered(mv)
return _out_collection(mv)


# todo: nogil for expensive C operation?
Expand All @@ -330,12 +341,16 @@ def compact_cells(cells):
Returns
-------
unordered collection of H3Cell
Notes
-----
There is currently no guaranteed order of the output cells.
"""
# todo: does compact_cells work on mixed-resolution collections?
hu = _in_collection(cells)
hc = _cy.compact_cells(hu)

return _out_unordered(hc)
return _out_collection(hc)


def uncompact_cells(cells, res):
Expand All @@ -359,11 +374,15 @@ def uncompact_cells(cells, res):
todo: add test to make sure an error is returned when input
contains cell smaller than output res.
https://github.com/uber/h3/blob/master/src/h3lib/lib/h3Index.c#L425
Notes
-----
There is currently no guaranteed order of the output cells.
"""
hc = _in_collection(cells)
hu = _cy.uncompact_cells(hc, res)

return _out_unordered(hu)
return _out_collection(hu)


def h3shape_to_cells(h3shape, res):
Expand Down Expand Up @@ -396,6 +415,10 @@ def h3shape_to_cells(h3shape, res):
'862830947ffffff',
'862830957ffffff',
'86283095fffffff'}
Notes
-----
There is currently no guaranteed order of the output cells.
"""

# todo: not sure if i want this dispatch logic here. maybe in the objects?
Expand All @@ -410,7 +433,7 @@ def h3shape_to_cells(h3shape, res):
else:
raise ValueError('Unrecognized type: ' + str(type(h3shape)))

return _out_unordered(mv)
return _out_collection(mv)


def cells_to_h3shape(cells, tight=True):
Expand Down Expand Up @@ -457,6 +480,10 @@ def geo_to_cells(geo, res):
Both H3Poly and H3MultiPoly implement the interface.
res : int
Resolution of desired output cells.
Notes
-----
There is currently no guaranteed order of the output cells.
"""
h3shape = geo_to_h3shape(geo)
return h3shape_to_cells(h3shape, res)
Expand Down Expand Up @@ -643,7 +670,7 @@ def origin_to_directed_edges(origin):
"""
mv = _cy.origin_to_directed_edges(_in_scalar(origin))

return _out_unordered(mv)
return _out_collection(mv)


def directed_edge_to_boundary(edge):
Expand All @@ -667,7 +694,7 @@ def grid_path_cells(start, end):
"""
mv = _cy.grid_path_cells(_in_scalar(start), _in_scalar(end))

return _out_ordered(mv)
return _out_collection(mv)


def is_res_class_III(h):
Expand Down Expand Up @@ -715,7 +742,7 @@ def get_pentagons(res):
"""
mv = _cy.get_pentagons(res)

return _out_unordered(mv)
return _out_collection(mv)


def get_res0_cells():
Expand All @@ -729,10 +756,14 @@ def get_res0_cells():
Returns
-------
unordered collection of H3Cell
Notes
-----
There is currently no guaranteed order of the output cells.
"""
mv = _cy.get_res0_cells()

return _out_unordered(mv)
return _out_collection(mv)


def cell_to_center_child(h, res=None):
Expand Down
3 changes: 1 addition & 2 deletions src/h3/api/basic_int/_convert.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,4 @@ def _in_collection(cells):
return _cy.iter_to_mv(it)


_out_unordered = set
_out_ordered = list
_out_collection = list
8 changes: 1 addition & 7 deletions src/h3/api/basic_str/_convert.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,5 @@ def _in_collection(cells):
return _cy.iter_to_mv(it)


def _out_unordered(mv):
# todo: should this be an (immutable) frozenset?
return set(_cy.int_to_str(h) for h in mv)


def _out_ordered(mv):
# todo: should this be an (immutable) tuple?
def _out_collection(mv):
return list(_cy.int_to_str(h) for h in mv)
3 changes: 1 addition & 2 deletions src/h3/api/memview_int/_convert.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,4 @@ def _id(x):
_in_scalar = _id
_out_scalar = _id
_in_collection = _id
_out_unordered = _id
_out_ordered = _id
_out_collection = _id
3 changes: 1 addition & 2 deletions src/h3/api/numpy_int/_convert.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,4 @@ def _in_collection(x):
return np.asarray(x, dtype='uint64')


_out_unordered = _in_collection
_out_ordered = _in_collection
_out_collection = _in_collection
Empty file added tests/polyfill/__init__.py
Empty file.
18 changes: 13 additions & 5 deletions tests/polyfill/test_h3.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import h3
import pytest

from .. import util as u


class MockGeoInterface:
def __init__(self, dictionary):
Expand Down Expand Up @@ -111,9 +113,12 @@ def test_polyfill_with_hole():
out = h3.h3shape_to_cells(poly, res=9)
assert len(out) == 1214

foo = lambda x: h3.h3shape_to_cells(h3.H3Poly(x), 9)
# todo: foo = lambda x: h3.H3Poly(x).to_cells(9)
assert out == foo(sf_7x7) - foo(sf_hole1)
foo = lambda x: set(h3.h3shape_to_cells(h3.H3Poly(x), 9))

assert u.same_set(
out,
foo(sf_7x7) - foo(sf_hole1)
)


def test_polyfill_with_two_holes():
Expand All @@ -122,8 +127,11 @@ def test_polyfill_with_two_holes():
out = h3.h3shape_to_cells(poly, 9)
assert len(out) == 1172

foo = lambda x: h3.h3shape_to_cells(h3.H3Poly(x), 9)
assert out == foo(sf_7x7) - (foo(sf_hole1) | foo(sf_hole2))
foo = lambda x: set(h3.h3shape_to_cells(h3.H3Poly(x), 9))
assert u.same_set(
out,
foo(sf_7x7) - (foo(sf_hole1) | foo(sf_hole2))
)


def test_polyfill_geo_json_compliant():
Expand Down
21 changes: 13 additions & 8 deletions tests/polyfill/test_polyfill.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

from h3 import H3ResDomainError

from .. import util as u


def get_us_box_coords():

Expand Down Expand Up @@ -72,7 +74,7 @@ def test_h3shape_to_cells():
poly = h3.H3Poly(maine)
out = h3.h3shape_to_cells(poly, 3)

assert out == expected
assert u.same_set(out, expected)


def test_h3shape_to_cells2():
Expand Down Expand Up @@ -100,20 +102,23 @@ def test_h3shape_to_cells_holes():

for res in 1, 2, 3, 4, 5:
cells_all = h3.h3shape_to_cells(h3.H3Poly(outer), res)
cells_holes = h3.h3shape_to_cells(h3.H3Poly(outer, hole1, hole2), res=res)
cells_holes = set(h3.h3shape_to_cells(h3.H3Poly(outer, hole1, hole2), res=res))

cells_1 = h3.h3shape_to_cells(h3.H3Poly(hole1), res)
cells_2 = h3.h3shape_to_cells(h3.H3Poly(hole2), res)
cells_1 = set(h3.h3shape_to_cells(h3.H3Poly(hole1), res))
cells_2 = set(h3.h3shape_to_cells(h3.H3Poly(hole2), res))

assert len(cells_all) == len(cells_holes) + len(cells_1) + len(cells_2)
assert cells_all == set.union(cells_holes, cells_1, cells_2)
assert u.same_set(
cells_all,
set.union(cells_holes, cells_1, cells_2)
)


def test_resolution():
poly = h3.H3Poly([])

assert h3.h3shape_to_cells(poly, 0) == set()
assert h3.h3shape_to_cells(poly, 15) == set()
assert h3.h3shape_to_cells(poly, 0) == []
assert h3.h3shape_to_cells(poly, 15) == []

with pytest.raises(H3ResDomainError):
h3.h3shape_to_cells(poly, -1)
Expand Down Expand Up @@ -158,4 +163,4 @@ def test_cells_to_geo():
assert len(coord[0]) == 7
assert coord[0][0] == coord[0][-1]

assert h3.geo_to_cells(geo, res) == {h}
assert h3.geo_to_cells(geo, res) == [h]
4 changes: 2 additions & 2 deletions tests/polyfill/test_polygon_class.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ def test_repr():
a = '8928308280fffff'
b = h3.grid_ring(a, 5).pop()

cells1 = h3.grid_ring(b, 2) | {a}
cells2 = cells1 | {b}
cells1 = h3.grid_ring(b, 2) + [a]
cells2 = cells1 + [b]

mpoly1 = h3.cells_to_h3shape(cells1)
mpoly2 = h3.cells_to_h3shape(cells2)
Expand Down
Loading

0 comments on commit e64ff67

Please sign in to comment.