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

more efficient unary_union in h3fy #126

Closed knaaptime closed 3 years ago

codecov-io commented 3 years ago

Codecov Report

Merging #126 (179fe90) into master (399ea20) will increase coverage by 0.20%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #126      +/-   ##
==========================================
+ Coverage   82.32%   82.52%   +0.20%     
==========================================
  Files          15       15              
  Lines         758      767       +9     
==========================================
+ Hits          624      633       +9     
  Misses        134      134              
Impacted Files Coverage Δ
tobler/tests/test_utils.py 100.00% <100.00%> (ø)
tobler/util/util.py 89.47% <100.00%> (+0.14%) :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 399ea20...e401c16. Read the comment docs.

martinfleis commented 3 years ago

This does not help in case of multipolygons. We do unary_union, then fail -> except: and do it again.

I'd do unary_union, save it separately and pass it directly to _to_hex and that loop in case of multipolygon instead of passing gdf.

knaaptime commented 3 years ago

gotcha. the new strategy should handle it this way

martinfleis commented 3 years ago

Hey @knaaptime, I have pushed a bit of a cleanup. There's no need to go via geopandas, explode and such in this. Feel free to revert if you don't like this way but it seems to be cleaner and more straightforward to my eyes like this.

knaaptime commented 3 years ago

excellent, thank you! I was just in the middle of doing a portion of this because my crs handling was ridiculous. Looking at the code for too long and jumping through too many hoops :)

knaaptime commented 3 years ago

@martinfleis i had to move the clip up above the reprojection otherwise the gdfs are misaligned, otherwise i think this is set