Skip to content
Permalink
Browse files

[processing] fix buffer tool

  • Loading branch information
alexbruy committed Jul 22, 2016
1 parent e8bac30 commit 9976c30c9aa7b0134b7e72e64157ac9fdb0b042f
Showing with 5 additions and 2 deletions.
  1. +5 −2 python/plugins/processing/algs/qgis/Buffer.py
@@ -57,8 +57,11 @@ def buffering(progress, writer, distance, field, useField, layer, dissolve,
value = distance

inGeom = QgsGeometry(inFeat.geometry())
if inGeom.isGeosEmpty() or not inGeom.isGeosValid():
ProcessingLog.addToLog(ProcessingLog.LOG_WARNING, 'Feature {} has empty or invalid geometry. Skipping...'.format(inFeat.id()))
if inGeom.isGeosEmpty():
ProcessingLog.addToLog(ProcessingLog.LOG_WARNING, 'Feature {} has empty geometry. Skipping...'.format(inFeat.id()))
continue
if not inGeom.isGeosValid():
ProcessingLog.addToLog(ProcessingLog.LOG_WARNING, 'Feature {} has invalid geometry. Skipping...'.format(inFeat.id()))
continue
outGeom = inGeom.buffer(float(value), segments)
if first:

11 comments on commit 9976c30

@m-kuhn

This comment has been minimized.

Copy link
Member

@m-kuhn m-kuhn replied Jul 22, 2016

What do you think about a parameter IGNORE_INVALID, off by default like here
143cfab?

This way, a user has to consciously accept that he wants invalid geometries to be ignored or preprocess his data.

@alexbruy

This comment has been minimized.

Copy link
Contributor Author

@alexbruy alexbruy replied Jul 23, 2016

Personally I think that we should maintain same behavior for all Processing algorithms, including 3rd party backends. If one want to check/fix/reproject/etc layer before actual analysis, Processing provides all necessary tools for this: scripts, models and hooks.

@m-kuhn

This comment has been minimized.

Copy link
Member

@m-kuhn m-kuhn replied Jul 23, 2016

@gioman

This comment has been minimized.

Copy link
Contributor

@gioman gioman replied Jul 23, 2016

@alexbruy @alexbruy Hi Alex and Matthias. I also think that fixing tools must be left separate from actual geoprocessing tools. The real issue here are tools that return wrong almost silently (logs are not very useful for the vast majority of users) because of geometry errors. Unfortunately we still have even worst cases: qgis tools returning silently wrong results even when input layers are ok.

@m-kuhn

This comment has been minimized.

Copy link
Member

@m-kuhn m-kuhn replied Aug 3, 2016

@alexbruy what do you think about the approach in 143cfab?

@alexbruy

This comment has been minimized.

Copy link
Contributor Author

@alexbruy alexbruy replied Aug 3, 2016

@m-kuhn I think it is ok. But on other hand we can use this approach only with native algs. GDAL, SAGA and other algs will lack this feature and confuse users.

@volaya what is your opinion?

@m-kuhn

This comment has been minimized.

Copy link
Member

@m-kuhn m-kuhn replied Aug 3, 2016

@alexbruy is there a proper solution that works with 3rd party algs as well?

@alexbruy

This comment has been minimized.

Copy link
Contributor Author

@alexbruy alexbruy replied Aug 3, 2016

@m-kuhn AFAIK there is no proper solution for this. Only workaround: export layer in compatible format excluding invalid geometries if checkbox is checked. But this will increase analysis time considerably and this again can be done with models/scripts/pre-execution hook without altering algorihtms code

@m-kuhn

This comment has been minimized.

Copy link
Member

@m-kuhn m-kuhn replied Aug 3, 2016

@volaya

This comment has been minimized.

Copy link
Contributor

@volaya volaya replied Aug 9, 2016

I think that the more atomized the alg collection is, the better. If one needs to have several operations performed (clean + process), then the modeler can be used to wrap them. Easier algorithms are more useful and better to maintain.

@m-kuhn

This comment has been minimized.

Copy link
Member

@m-kuhn m-kuhn replied Aug 9, 2016

@volaya I think we all agree on that part.

The question is what to do by default with input data that produce erroneous output.

  • Write something to the log and continue (producing erroneous output).
  • Abort algorithm with an error (unless the user consciously opted in to ignore such errors).
Please sign in to comment.
You can’t perform that action at this time.