add applyreduce method #40
Conversation
src/methods/area.jl
Outdated
| @@ -77,8 +83,11 @@ computed slighly differently for different geometries: | |||
| Result will be of type T, where T is an optional argument with a default value | |||
| of Float64. | |||
| """ | |||
| signed_area(geom, ::Type{T} = Float64) where T <: AbstractFloat = | |||
| _signed_area(T, GI.trait(geom), geom) | |||
| function signed_area(geom, ::Type{T} = Float64; threaded=false) where T <: AbstractFloat | |||
There was a problem hiding this comment.
One thing is that I didn't define the signed area for multi-geometries. What does it really mean since different polygons could have different winding orders. If I understand the code correctly, this does define signed area for multi-geometries
There was a problem hiding this comment.
If we are going to define it for multi-geometries, we should probably put a warning.
There was a problem hiding this comment.
Ok I did wonder about that. So it only works if the winding is consistent.
I guess we can not do this for signed area? Just area? Honestly I implemented it to be complete along with distance and signed distance, and it does make sense in that case.
There was a problem hiding this comment.
What does it mean to aggregate signed areas? is that a useful thing to have?
We could make sure that at the collection and multi-geometry level and children have the same sign in the area?
Like all polygons must have positive signed areas, or all netagive, or it doesn't make sense. We could check that and throw a warning if it doesn't hold?
There was a problem hiding this comment.
I guess I don't see the use case. We still check if the polygon or shape is CW or CCW under the hood, so we aren't saving the user any computation by giving them a signed area if that is what they want to know (in addition to area obviously). We are also adding in a check and potentially throwing a warning, which will slow things down a bit. It seems like if a user wants the area and wants to know if they all have the same winding order, they should just get the signed area of each polygon, or the area and then check the winding order themselves. However, if we do have it, I think a warning is good to avoid a misunderstanding. Because the issue is if we have two polygons, one CW with an area of 50 and one CCW with an area of 50, the total signed area of the multipolygon will be 0. Which to me seems pretty confusing and now the user has none of the information that we calculated under the hood. So they lose the area and winding order information entirely.
There was a problem hiding this comment.
Ok, lets just keep area using applyreduce then and make it clear in the docs that signed_area has a more limited application.
There was a problem hiding this comment.
One of my main intentions with these methods is to standardize everything and make all of our functions work on the same set of possible inputs, and to do that in a centralized, structured way rather than actually writing all that code.
But the generality will break in some places.
There was a problem hiding this comment.
Yeah, I agree with that. Just not in this case.
This PR adds an
applyreducemethod, like apply, but reducing rather than rebuilding.applyreduceis a variation onmapreduceas its kind of similar except for the nesting ofapply. It has a functionfthat is applied to each nested geom, and a reducingopas the second argument.I'm implementing it for
area,centroidand maybedistance. The idea is you could take theareaof a whole dataframe column ofFeatureCollections,Feature, geometries, whatever in one go (threaded if you like).centroidwill also take the centroid across the whole column/whatever by combining centroids and areas fractionally in the reducingop.distancecould take the minimum distance from any collection of objects to a point? I'm not sure if thats as useful as the othere but it may as well?It could also be used for e.g. polygon
unionover any object.