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

Sg/cut poly #56

Merged
merged 9 commits into from
Feb 12, 2024
Merged

Sg/cut poly #56

merged 9 commits into from
Feb 12, 2024

Conversation

skygering
Copy link
Collaborator

I needed this functionality in my code and figured that it might be useful to other people. While it isn't a feature in GEOS, it is a feature in MATLAB with the function cutpolygon. It also has quite a few similar posts on Stack Overflow. I was able to take advantage of the new clipping helper functions to write it. Hoping this isn't too specific.

I also realized I made a type mistake in the clipping with the Target field so I fixed that as well.

@skygering skygering requested a review from rafaqz February 8, 2024 20:46
@skygering
Copy link
Collaborator Author

skygering commented Feb 8, 2024

One thing I was wondering was if this should also have a Target field. To me, it seems intuitive that if you cut a polygon, you get a list of polygons. If you cut a multipolygon, you also get a list of polygons. The question is then how far would we want to extend something like this? If gets more confusing with cutting Linear Rings, for example as those would become LineStrings if cut, but stay as LinearRings if not cut. But I don't necessarily think we need to extend to curves...

skygering and others added 2 commits February 9, 2024 11:01
@skygering
Copy link
Collaborator Author

@asinghvi17 I updated the docs. Any other feedback?

[[[5.0, 0.0], [10.0, 0.0], [10.0, 10.0], [5.0, 10.0], [5.0, 0.0]]]
```
"""
cut(geom, line::GI.Line, ::Type{T} = Float64) where {T <: AbstractFloat} =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason to limit to GI.Line here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The algorithm only works with one line segment right now. I could also broaden to LineString and then throw an error if there is more than one non-collinear segment. That just seemed unnecessary right now.

Copy link
Member

@rafaqz rafaqz Feb 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean it could also accept other Lines, like GeometryBasics

Using GI traits

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah, you're right. Whoops. I will fix that.

src/methods/clipping/difference.jl Show resolved Hide resolved
src/methods/clipping/cut.jl Show resolved Hide resolved
src/methods/clipping/cut.jl Show resolved Hide resolved
@skygering
Copy link
Collaborator Author

skygering commented Feb 12, 2024

I am going to merge this then now that I addressed those problems. Any final comments?

@skygering skygering merged commit 854b036 into JuliaGeo:main Feb 12, 2024
3 checks passed
@skygering skygering deleted the sg/cut_poly branch February 12, 2024 20:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants