pysal / tobler

Spatial interpolation, Dasymetric Mapping, & Change of Support
https://pysal.org/tobler
BSD 3-Clause "New" or "Revised" License
144 stars 30 forks source link

fix: addressing #146 h3fy not filling completely #162

Closed sjsrey closed 2 years ago

knaaptime commented 2 years ago

cool, thanks. I think the other way we considered handling this was following rasterio, where we give an all_touched kwarg that determines whether we do polygon intersection or centroid intersection. Might be useful to have that choice on top of the buffer

codecov[bot] commented 2 years ago

Codecov Report

Merging #162 (3a1b958) into master (66f3825) will increase coverage by 0.29%. The diff coverage is 43.75%.

@@            Coverage Diff             @@
##           master     #162      +/-   ##
==========================================
+ Coverage   74.24%   74.54%   +0.29%     
==========================================
  Files          20       20              
  Lines        1029     1041      +12     
==========================================
+ Hits          764      776      +12     
  Misses        265      265              
Impacted Files Coverage Δ
tobler/util/util.py 80.28% <43.75%> (-11.25%) :arrow_down:
tobler/_version.py 40.65% <0.00%> (+2.67%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 66f3825...3a1b958. Read the comment docs.

sjsrey commented 2 years ago

cool, thanks. I think the other way we considered handling this was following rasterio, where we give an all_touched kwarg that determines whether we do polygon intersection or centroid intersection. Might be useful to have that choice on top of the buffer

If I'm grokking all_touched, what this pr does is implement all_touched=True? And, all_touched=False may result in failing to address the original "bug" in that there still may be empty spaces internal to the source?

knaaptime commented 2 years ago

ah. I forgot we weren't doing the predicate ourselves. The polyfill is what determines that we select by hexagon centroids, so I dont think we'd have the option to do it the same as rasterio

martinfleis commented 2 years ago

We don't need to expose distance. For a user, that would be a wild guess what to use. We can automatically determine it from the h3 level as a radius of a hexagon.