Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix CompoundGeometry offset perimeter (dilation creates overlapping geometry) #332

Merged
merged 10 commits into from
Jul 19, 2024

Conversation

robbievanleeuwen
Copy link
Owner

@robbievanleeuwen robbievanleeuwen commented Oct 9, 2023

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

Tasks:

  • Successfully run test_compound_rectangular_offset()

@robbievanleeuwen
Copy link
Owner Author

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

- Offset now works with three or more equal-sized geometries stacked together (not just two)
- Fixed error in test calculation: A = pi * r**2, not A = pi * r**2 / 4
- Tests pass but more work needs to be done on the algorithm to deal with re-entrant corners (later date).
@connorferster
Copy link
Collaborator

@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
Copy link
Collaborator

Also, for fun:

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

@robbievanleeuwen
Copy link
Owner Author

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 robbievanleeuwen merged commit 59ec96a into master Jul 19, 2024
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CompoundGeometry.offset_perimeter() with positive amount creates overlapping geometry
2 participants