section-properties icon indicating copy to clipboard operation
section-properties copied to clipboard

Fix `CompoundGeometry` offset perimeter (dilation creates overlapping geometry)

Open robbievanleeuwen opened this issue 2 years ago • 1 comments

This PR addresses #331, in which dilated offsets create overlapping geometry.

Tasks:

  • [ ] Successfully run test_compound_rectangular_offset()

robbievanleeuwen avatar Oct 09 '23 11:10 robbievanleeuwen

@connorferster - seeing as you wrote the original code for this, wondering if you have time to have a quick look?

robbievanleeuwen avatar Oct 09 '23 11:10 robbievanleeuwen

@robbievanleeuwen For your review. The tests pass but mypy does not like my type annotations on one helper function. I am unsure of how to interpret the error messages in order to fix.

The function takes input the looks like this:

array([
    <GEOMETRYCOLLECTION (MULTILINESTRING EMPTY, MULTILINESTRING ((500 500, 500 0)))>,
    <GEOMETRYCOLLECTION (MULTILINESTRING EMPTY, MULTILINESTRING ((1000 0, 1000 5...>],
     dtype=object
)

I originally tried npt.ArrayLike[GeometryCollection] but then switched it to npt.ArrayLike because, from the error messages, it seemed to be the thing to do.

Any insight on what to be done to get this merged?

connorferster avatar Jul 18 '24 19:07 connorferster

Also, for fun:

The live-stream for doing this PR: https://www.youtube.com/live/hSfsojAAJjc?feature=shared

connorferster avatar Jul 18 '24 19:07 connorferster

Thoroughly enjoyed the stream - helps with the code review as well 😆 Shapely is such a fun and feature rich library!

mypy - happy to ignore these two lines, in this case shapely isn't being particularly helpful with what it's returning and I'm happy with what you're doing here.

I've also added that test you were working on in the live stream with 3 rectangles!

Thanks again for working on this one, bit of a curly one in the end! I also wouldn't lose too much sleep on all the edge cases as I'm sure there are many, and I've got a little note in the docs mentioning that the user should double check the output geometry in these cases!

robbievanleeuwen avatar Jul 19 '24 04:07 robbievanleeuwen