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

Add vector_buffer #314

Merged
merged 7 commits into from
Mar 15, 2022
Merged

Add vector_buffer #314

merged 7 commits into from
Mar 15, 2022

Conversation

m-mohr
Copy link
Member

@m-mohr m-mohr commented Dec 17, 2021

Adds a new process "vector_buffer", see #264

If I understand https://openeo.vito.be/openeo/1.0/processes/backend/vector_buffer correctly (some ambiguities are present), it should be possible to merge both in a way that there's no conflict.

Implementations looked at:

Question:

  • How to handle the number of segments per quadrant? -> leave up to implementation, be tolerant in test suite
  • Should we allow negative distances? -> seems useful

@m-mohr m-mohr added the vector label Dec 17, 2021
@m-mohr m-mohr added this to the 1.3.0 milestone Dec 17, 2021
@m-mohr m-mohr linked an issue Dec 17, 2021 that may be closed by this pull request
@m-mohr m-mohr mentioned this pull request Dec 20, 2021
Copy link

@mattia6690 mattia6690 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that a negative buffer is very useful for many researchers to either exclude or specifically research areas at the boundaries of Polygons or in the vicinity of points and lines.
Otherwise the current structure is fine

@m-mohr m-mohr requested a review from jdries December 21, 2021 12:05
@jdries
Copy link
Contributor

jdries commented Dec 21, 2021

We certainly use negative buffer distances.
Not sure about segments per quadrant, seems like a parameter that you can leave to the implementation?
Seems fine otherwise, hope we don't need to mathematically specify what a rounded buffer is.

@m-mohr
Copy link
Member Author

m-mohr commented Dec 21, 2021

Not sure about segments per quadrant, seems like a parameter that you can leave to the implementation?

@jdries I assume that would be an issue in the process validation suite if the implementations differ in the number of segments.

@jdries
Copy link
Contributor

jdries commented Dec 21, 2021

I assume that would be an issue in the process validation suite if the implementations differ in the number of segments.

Yes, when doing vector operations on the sphere, quite a few things can go wrong and need to be specified if you want absolute accuracy. So you would need to start by defining your error tolerances. Or make it dynamic, because if vector data is combined with raster, then there's no need to go far beyond the resolution of your raster cubes.
Another example is vector-pixel intersection tests, which you use in aggregate_spatial.

So my experience is that the validation suite will need to have some built-in leniency as to what the error can be for a specific process.

@m-mohr
Copy link
Member Author

m-mohr commented Dec 21, 2021

Okay, happy to keep it simple (except in the test suite). Then we can (until requested by users) define the process without such a parameter. Negative distances seem useful so then this is ready for review.

proposals/vector_buffer.json Outdated Show resolved Hide resolved
proposals/vector_buffer.json Outdated Show resolved Hide resolved
proposals/vector_buffer.json Outdated Show resolved Hide resolved
proposals/vector_buffer.json Outdated Show resolved Hide resolved
@m-mohr m-mohr added the help wanted Extra attention is needed label Feb 24, 2022
@m-mohr m-mohr mentioned this pull request Feb 25, 2022
@m-mohr
Copy link
Member Author

m-mohr commented Mar 15, 2022

Merging as this seems to be finished, except for the general unit discussion in #330.

@m-mohr m-mohr merged commit f4d5d03 into draft Mar 15, 2022
@m-mohr m-mohr deleted the issue-264 branch March 15, 2022 14:09
@m-mohr m-mohr removed this from the 1.3.0 milestone Feb 1, 2023
@m-mohr m-mohr added this to the 2.0.0 milestone Feb 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed new process vector
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Buffer process
5 participants