aggregate_spatial

[kempenep]

created new process aggregate_spatial as agreed during openEO process meeting on 20191212. The process aggregate_polygon will then be deprecated

Issue: #62

[m-mohr]

Thank you for the PR, @kempenep. I made some suggestions for the overall consistency (e.g. use geometries instead of vector).

As we also have filter_polygon, we should also apply the changes we make here to filter_polygon and rename it to filter_spatial, I think?

Edit: May also apply to mask_polygon in #103.

[kempenep]

Thanks for the suggestions, agreed. The filter_polygon was already discussed during the meeting, where we decided to proceed as you suggested. This was going to be taken care of by ... (don't remember who)...

[m-mohr]

@kempenep Do you want me to commit all the changes?

I think filter_spatial was assigned to @GreatEmerald.

I'm wondering whether filter/aggregate_geometries would be more intuitive to some users? Especially thinking about meteorology where I heard they would be likely confused about aggregate_spatial.

[kempenep]

You can commit all changes. Don't know which of spatial/geometries is to be preferred. I like "spatial" as it is in line with the "temporal" we already have.

[m-mohr]

@kempenep I don't have write permissions to your repository ;-) Can you batch commit them via the FIles tab here at GitHub, please?

[kempenep]

@m-mohr
https://github.com/kempenep/openeo-processes/invitations

[m-mohr]

We still need to deprecate aggregate_polygon, other than that the PR looks good now.

[jdries]

Is the behaviour for lines and points already defined?
Or is it this line:

The process considers all pixels for which the point at the pixel center intersects with the corresponding geometry (as defined in the Simple Features standard by the OGC).

[m-mohr]

Is the behaviour for lines and points already defined?
Or is it this line:

The process considers all pixels for which the point at the pixel center intersects with the corresponding geometry (as defined in the Simple Features standard by the OGC).

@jdries Yes, it's that line.

[jdries]

For points, and lines, I would consider an intersection of the pixel bounding box with the geometry, instead of considering only pixel center.
Alternative approach could be something like: in case of lines/points intersection geometry is buffered using half the size of a pixel (hoping that pixels are square), and than the aggregation reduces to aggregating over a polygon.
(That alternative could also be a solution for users in case the backends simply does not support lines and points.)

[jdries]

I agree on that, and here are two more reasons:

• aggregation over point or line is way more exotic, so backends may not support it. Having separate processes would allow those backends to make that clear
• avoiding unneeded backwards incompatible changes in the process definitions

[m-mohr]

Indeed, the wording regarding the pixel center works only well with polygons. And for points it should consider the pixel (the whole, not just the center) that intersersects.

But that's why I think separate processes are better suited for different geometry types and initially restricted it to polygons only... If you need to handle the geometry types differently, make different processes otherwise the process gets to difficult. We'll see how it works out also in filter_spatial.

Edit: Your response was too fast ;-) Consider the second paragraph to be posted before your/Jeroens previous post.

[lforesta]

So the decision now is between having this process or separate ones? Maybe we need to discuss this in the next telecon again..

[m-mohr]

Yes, I think so, too.

[GreatEmerald]

I don't see why we'd handle it any differently for different types of geometry. If the geomrety touches a pixel in any way, the pixel is included.

Alternatively, like in R, add an optional argument that says whether to include anything that touches, or anything that the geometry envelops, or any pixels whose centres the geometry envelops. That would make it more flexible. For line or point features, enveloping implies being at the exact same location (so if it's right on the pixel centre and the user asks for enveloping, it's OK, otherwise the result is empty)

[kempenep]

I agree with Dainius here. I am also in favor of the optional argument, where I would adopt the GDAL terminology (as in gdal_rasterize):
ALL_TOUCHED: all pixels touched by lines or polygons will be updated, not just those on the line render path, or whose center point is within the polygon. Defaults to disabled for normal rendering rules.

[m-mohr]

I don't see why we'd handle it any differently for different types of geometry. If the geomrety touches a pixel in any way, the pixel is included.

That sounds reasonable and would work consistently across all geometry types. See the suggestion below...

Alternatively, like in R, add an optional argument that says whether to include anything that touches, or anything that the geometry envelops, or any pixels whose centres the geometry envelops. That would make it more flexible.

But also more complex to implement. Maybe we can start with the simpler case above and then see at which point we need the additional parameter.

[m-mohr]

CHANGELOG entry is missing.

[GreatEmerald]

It's true that the "aggregate" name is a bit misleading in the point and line case, but from the usability point of view I think it's still better to have one process that gives summary statistics over geometry, than to have separate processes. You don't want to tell users "use this process to extract information over a polygon, and this process to extract information over lines and points", for the users it's all geometry anyway. You can make it handle different geometry in different ways if that makes the most sense (so it would be a process that behind the scenes calls one of three processes based on geometry type), as long as that makes it do the right thing for most users.

Using a buffer around points and lines is also fine as far as implementation goes. (Though we need to think about what happens if you have a pixel on the corner between 4; I'd say it should return all 4 and reduce it with the given reducer.)

[m-mohr]

Telco: We remove the "uniqueness" constraint for aggregate from the glossary.
We allow all geometries in aggregate_spatial. If I remember correctly, we'll still use the pixel centers, but for lines and points it will consider the closest pixel centers. This means pixels may be used for multiple geometries. Correct? @mkadunc @jdries @lforesta @aljacob
Any other things I should mention in the process description? Did I forget anything?

Edit: A to do before merging is to replace the new occurrences of aggregate_polygon in the UDF (coming from PR #111) processes with aggregate_spatial.

[m-mohr]

Closing for now, I'll work on a joint PR for the issues #37, #62, also re-based on #94 and #111.

[m-mohr]

There's a new proposal in PR #131