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

simplify #3

Merged
merged 12 commits into from
May 28, 2023
Merged

simplify #3

merged 12 commits into from
May 28, 2023

Conversation

rafaqz
Copy link
Member

@rafaqz rafaqz commented May 5, 2023

These are from Turf.jl but very heavily refactored.

I realised I had already half converted turf to the latest geointerface 🤣

@DanielVandH
Copy link
Contributor

Will you consider at some point using some form of exact predicates (aka ExactPredicates.jl) for some of this? I'd be a bit unsure about using e.g. is_point_on_segment in general in it's current form.

@rafaqz rafaqz changed the title predicates and simplify grab bag simplify May 5, 2023
@rafaqz
Copy link
Member Author

rafaqz commented May 5, 2023

Sure! I haven't used the package myself but it looks good, although maybe not fast as it requires duplication of all objects as tuples. Many algorithms here will not allocate.

In this PR I am just hacking in someone elses code from Turf.jl (which copied turf.js) with a focus on getting the high level algorithms working on any GeoInterface.jl compatible objects in a clean, generic way.

The lower level predicates can really be swapped out at any point, feel free to jump in and hook them up if you want to.

I've actually move the predicataes out to another PR for now too.

@rafaqz
Copy link
Member Author

rafaqz commented May 5, 2023

@DanielVandH now that I look at it, I cant see what predicates from ExactPredicates.jl would replace is_point_on_segment. What would you suggest?

@DanielVandH
Copy link
Contributor

Maybe since this is something that'll be considered properly down the line, I'll put some of my thoughts about this in an issue and respond there.

@rafaqz
Copy link
Member Author

rafaqz commented May 26, 2023

@skygering @asinghvi17 as I mentioned in #8, I've written up simplify with variants for the major algorithms, selectable with constructors:

simplify(RadialDistance(; tol=0.1), obj)
simplify(DouglasPeucker(; tol=0.01), obj)
simplify(VisvalingamWhyatt(; number=7), obj)

And the default uses DouglasPeucker as its the most common:

simplify(obj; tol=0.1)

What do you think about this as a general pattern?

Also, simplify is built on apply so it will simplify over any object. There is way of selecting a Union of types with apply that seems pretty useful when you need to have different algorithms for different geometry traits:
https://github.com/asinghvi17/GeometryOps.jl/blob/5aad1e958434ed93e302ec6250cb26d922178cae/src/transformations/simplify.jl#L59

@rafaqz rafaqz merged commit 2e99d8b into main May 28, 2023
@rafaqz rafaqz deleted the simplify branch May 28, 2023 14:58
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.

2 participants