jbaileyh / geogrid

Turning geospatial polygons into regular or hexagonal grids. For other similar functionality see the tilemaps package https://github.com/kaerosen/tilemaps
Other
393 stars 31 forks source link

Coleman dev #37

Open aecoleman opened 4 years ago

aecoleman commented 4 years ago

Was able to remove the for loop in assign_polygons, which should make it much more efficient for larger datasets. Also transitioned assign_polygons to use sf primarily, and convert sp objects to sf (and back) when they are supplied.

I also fixed some lints (that probably weren't lints too long ago.

Finally, I added a function for generating samples of contiguous polygons from a larger sf object. I found it useful for benchmarking a larger geogrid job (hexgrid of all 3000+ CONUS Counties).

jbaileyh commented 4 years ago

Thanks for submitting! Will take a look as soon as possible.

aecoleman commented 4 years ago

Let me know if you have any questions, comments or suggestions. I hope I didn't break the lintr setup. Oh, and I just realized that I forgot to run it through R CMD Check, so it would be a good idea to make sure I didn't introduce any new issues there.

conorotompkins commented 4 years ago

Was this commit merged? I am running into issues with the exact task that @aecoleman mentioned ("hexgrid of all 3000+ CONUS Counties"). The package works very well, but it would be fantastic if it could handle larger jobs.

aecoleman commented 4 years ago

@conorotompkins I don't believe it has been merged. You can install the version on my github and see how that works for you. The command to do that would be remotes::install_github("aecoleman/geogrid", ref = "coleman-dev").

Note that I have not tested this since R 4.0.0 came out, so YMMV.

Also, when I did this, I had some issues with hierarchical integrity. Counter to my expectations, states would end up scattered about. Squaring the cost matrix might alleviate this, but I haven't had the chance to try it out, yet. (I mentioned how long it took to run.) I've also since seen faster implementations of the cost minimization algorithm, which I haven't had a chance to play around with (the clue package contains one, I think).

jbaileyh commented 4 years ago

@conorotompkins @aecoleman apologies - this hasn't been merged. I was going to suggest installing the branch kindly submitted by @aecoleman in the medium term.