Sg/calc angles

[skygering]

Calculate angles of given geometries.

The only thing I am concerned about here again is type stability. Right now, points, lines, line strings, and linear rings all return Vectors of Floats, while Polygons return Vectors of Vectors of Floats.

And then a multi-geometry returns a vector of its element's angle return types. I don't really see a way to get around it.

[rafaqz]

LGTM

Instead of manually looping you could use applyreduce to get down to LineString/LinearRing/MultiPoint/Point targets and get vectors back. That would make it more generic. (Multipoint would also be an empty vector?)

I think applyreduce with identity or collect as op will return the unwrapped vectors?
But we may want to use filter to remove empty child vectors.

[skygering]

What do you think about polygons just returning a vector, rather than a nested vector? And then the angles after the first n are angles of holes... or do you think it needs to be nested?

[rafaqz]

Maybe inconsistent? But I have no idea what the use case is 😂

(But generally I think we should make nestedness patterns identical for everthing)

[skygering]

Yeah, it matches up more with the conventions. It is just irritating for me because none of the polygons in my simulations have holes and I don't need the extra allocations 😆

I have a decent amount of geometry code that I have written in the last year and a half floating around my model and I am going to try to move as much of it as I can into GeometryOps, both for the cleanliness of my model and also in case other people find it useful. So if there is slightly "off-topic" code coming in, that is why.

[skygering]

Given that we want Polygons to return Vector{Vector{T}}, I am not sure that applyreduce is the correct strategy. So then a "multi-geom" will be type Vector{Union{Vector, Vector{Vector{T}}}}.

If we want that to happen, I think we need to use push! as the applyreduce operation given that something like vcat would get rid of the nesting. Honestly, though, I kind of want to get rid of the nesting. I feel like it could be equally valid to just return a Vector of angles, there they are in the order provided by GI.getgeom, and then GI.getring. If someone wants the angles of a specific sub-geometry or sub-ring, they can calculate that... or figure it out by the number of points in each sub-geom/ring.

This also gets rid of the issue of empty Vectors (no angles, no elements). Versus if we filter out empty ones and we are nesting, I feel like that is strange as it leaves a missing slot for an existing geometry.

What do you think? Because I think it is going to be type-unstable if we keep the nesting for multi-geoms due to the depth of nesting depending on the type of sub-geometry.

[rafaqz]

Flattenimg is fine too! Just somethimg consistent thst will also work on features without manually writing it all.

You can easily flattten with applyreduce, or use GO.flatten ;)

[rafaqz]

Also not sure what you mean by multi-geom - where does the Union come from?

You should only be able to get a Union if you start with a Union.

[skygering]

By multi-geom I am referring to multipolygons, multipoints, multicurves, and geometry collections.

I think flattening is fine. We are just giving back a list of the angles of the shape.

[rafaqz]

What do you use these angles for in the end? Just so I get the purpose a little more.

[skygering]

So for me, the polygons are all pieces of ice on the ocean. Really sharp angles are un-physical and in the real world would probably fracture off. So I check the angle, and if it is smaller than a threshold, break off the corner, adding a new edge to the polygon and making the corner its own piece.

I also have code that breaks polygons through a given line segment to implement this (and more general fracturing), but that feels maybe a bit too niche to put in this library. Thoughts?

[rafaqz]

Ahh cool.

We do need polygon clipping!

But maybe people can hook up the angles/clipping part themselves?

[skygering]

Polygon-polygon clipping is entering a PR this week (I hope)! My intern is just cleaning up a few things, although there will probably be some more things to clean up afterward.

Ah, you're right! That is just clipping a polygon by a line. I hadn't thought of it like that. Perfect! I will move that code into the library sometime soon-ish then.

[skygering]

For now, I will try to finish this up today and get it merged.

[rafaqz]

Amazing. What clipping algorithm is it?

[skygering]

Greiner and Hormann (https://doi.org/10.1145/274363.274364)

[skygering]

Okay, I think this is good to go! I switched the polygon hole angles to the interior, which means they do need their own function but I use apply reduce for everything else.

[rafaqz]

Maybe MultiPointTrait is missing here?

Suggested change

	_angles(::Type{T}, ::Union{GI.PointTrait, GI.LineTrait}, geom) where T = T[]
	_angles(::Type{T}, ::Union{GI.PointTrait, GI.LineTrait, GI.MultiPointTrait}, geom) where T = T[]

[rafaqz]

Looks like this isn't used now

[rafaqz]

LGTM, few minor things

[skygering]

I can't figure out why the documentation is failing. I removed the angle documentation just to see if that was causing it. But I don't think I changed anything that would cause the docs to fail... The error messages also aren't that helpful. Have you every seen anything like this?

[rafaqz]

Looks like a Makie plot is broken somewhere, maybe in the barycentric docs?

[skygering]

Is there a way to run the documentation locally? This takes way too long to test...

[skygering]

Okay, so what I think happened is that with the updates to Documentor, it started running examples within comments, rather than just in DocStrings. I assumed it was the whole time, but I guess not given the number of issues. I fixed the errors, and many of the warnings, but there are still warnings left. Fixing those should be a separate PR.