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

polygonize should return a MultiPolygon, not a Vector of Polygon #139

Closed
rafaqz opened this issue May 13, 2024 · 9 comments
Closed

polygonize should return a MultiPolygon, not a Vector of Polygon #139

rafaqz opened this issue May 13, 2024 · 9 comments

Comments

@rafaqz
Copy link
Member

rafaqz commented May 13, 2024

See: https://discourse.julialang.org/t/polygonize-a-raster/114189/7

@asinghvi17
Copy link
Member

Shouldn't it actually return a feature collection, with one feature for each unique value? Or do you mean in the special case of an AbstractMatrix{Bool}, in which case I agree.

@rafaqz
Copy link
Member Author

rafaqz commented May 13, 2024

We don't actually do that case at the moment. But I'll add it, and yes that would need a FeatureCollection.

Currently is just Bool and f(x) -> Bool. I guess we can allow Integer arrays to be converted to FeatureCollection? But floating point would need some more complicated contour function.

(I guess we can make it a feature collection of pooled MultiPolygons, one for each value?)

@asinghvi17
Copy link
Member

That sounds good to me.

Makie uses Contour.jl and Isoband.jl to generate isolines - perhaps we could create an extension on that and use it as an algorithm if needed, the necessary code can be copied wholesale from Makie.

@rafaqz
Copy link
Member Author

rafaqz commented May 13, 2024

This is actually a very different algorithm, its exact pixels like the raster. But we could have those too.

(practically ive found exact pixels better to work with, and most R packages do this too, or allow for 4/8/16 nodes rather than just 4, which I'm also considering doing)

@rafaqz
Copy link
Member Author

rafaqz commented May 13, 2024

I added handling for Integer, just returning a FeatureCollection of MultiPolygon. But maybe we can do this for other Number using either N isolines specified with a keyword nintervals (any better name ideas?) or with IntervalSets.jl intervals? Would that dependency be ok?

(and now I'm thinking this approach will be slow for large numbers of values, though its fast for small numbers... maybe we need to create all polygons at the same time...)

@rafaqz
Copy link
Member Author

rafaqz commented May 13, 2024

I have no guage of this being fast or not, its 10 million pixels and 400 categories...

julia> @time featurecollection = polygonize(rand(1:400, 10000, 1000));
 16.385245 seconds (29.87 M allocations: 7.798 GiB, 29.17% gc time)

The alternative is to do one pass and write an algorithm that considers all the neighboring categories and adds edges where needed. Probably 100x faster in this case, but slower for the base Bool case.

No scratch that the number of categories is not the main cost. Probably this can be faster with a Dict lookup

@asinghvi17
Copy link
Member

asinghvi17 commented May 13, 2024

Maybe test against Contours.jl? I can send a script later tonight once I get on the plane and have to stay awake.

I can see the appeal of exact pixel polygons though.

@asinghvi17
Copy link
Member

Just thought of this but as you mentioned it's not really a comparison - the algorithms are too different!

@asinghvi17
Copy link
Member

fixed by #141 I think

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

No branches or pull requests

2 participants