mggg / maup

The geospatial toolkit for redistricting data.
https://maup.readthedocs.io/en/latest/
MIT License
65 stars 23 forks source link

Create "force_polygon" argument for topological repair #91

Closed coreyryanhanson closed 9 months ago

coreyryanhanson commented 10 months ago

I've experienced frequent issues with the topological repair functions when resolving errors from moving between certain projections. The functions tend to crash because of Line Segment fragments that are created after shapely's "make_valid" method. This fix adds an optional "force_polygon" argument that if enabled iterates through any resulting "GeometryCollection" objects and filters out any non-polygonal results.

peterrrock2 commented 9 months ago

Hey, thank you for adding this! Unfortunately, we didn't get around to changing this until the major update to version 2.0.0, but we have incorporated it into 2.0.1. Some other changes required a little bit of restructuring of this code, but I wanted to make sure that you received credit for the contribution, so I mentioned you in the full review of the 2.0.1 release PR#94. Let me know if that is all right, or if you would like any other additional accreditation for your contribution to the project.

jnclelland commented 9 months ago

Ditto to Peter's thanks - and here's a little more info. The new Maup version has a completely new repair function that's much more sophisticated than autorepair; it's called smart_repair, and there's more info in the README. (And if you want to see ALL the details, you can read all about it here: https://arxiv.org/abs/2312.11415.)

The new repair function uses Shapely's version of make_valid, so we had to give your version a different name; I called it make_valid_polygons and changed the default value of force_polygons to True. Then for the rest of the functions with a force_polygons argument, your version is used if force_polygons is True and Shapely's version is used if force_polygons is False.

I hope this is helpful - and feedback is very welcome!