QGIS » QGIS Application

We can change Processing behaviour to throw error and stop algorithm if there are invalid geometries in input layers. Is it OK?

Alexander Bruy wrote:

We can change Processing behaviour to throw error and stop algorithm if there are invalid geometries in input layers. Is it OK?

from my point of view is a big +1.

Is there any alternative? Flag and report at the end? IMO it's annoying when a tool stops on something that it can just ignore and move on. Invalid geometries are a fact of life and sometimes out of the control of the person running the algorithm.

Nathan Woodrow wrote:

IMO it's annoying when a tool stops on something that it can just ignore and move on.

but it is not really that easy: in this case (but also others, I'm putting together a report on the "union" tool) ignoring the geometry error produces a wrong result! If the input layers do not contain many features maybe the error is easy to spot, but usually is not the case, and the user is left with a very bad issue without knowing (right now there is just a log message, no message bar message).

Invalid geometries are a fact of life and sometimes out of the control of the person running the algorithm.

yes, but as I wrote above this is not the main issue. The main issue is to warn the user in a explicit, clear way (that there are geometry issues). And personally I prefer to have also the operation stopped rather than having a wrong output that I know that I have to redo again anyway (after cleaning/fixing).

Right ok. That makes sense. Would this be up front before running the tool itself,e.g run valid check, continue if all good. If that was the default action I think that would solve a "I ran this for over and hour and it stopped, now I have to start again" situations.

Nathan Woodrow wrote:

Right ok. That makes sense. Would this be up front before running the tool itself,e.g run valid check, continue if all good. If that was the default action I think that would solve a "I ran this for over and hour and it stopped, now I have to start again" situations.

Actually I think that we have already the solution: make liblwgeom a dependency (is available on osgeo4w and all major linux distros) and add to qgis geoprocessing tools an option to clean input layers beforehand. There should be also an option to snap geometries (to close small gaps/overlaps) exactly as we can do in GRASS when importing with v.in.ogr.

Nathan Woodrow wrote:

Is there any alternative? Flag and report at the end?

This is how it works now: invalid geometries skipped and information about them added to Processing log.

Giovanni Manghi wrote:

but it is not really that easy: in this case (but also others, I'm putting together a report on the "union" tool) ignoring the geometry error produces a wrong result!

In some cases, e.g. buffering skipping invalid geometries is ok.

Giovanni Manghi wrote:

The main issue is to warn the user in a explicit, clear way (that there are geometry issues).

Another possible approach is to show error message in the algorithm dialog and skip them. In this case user will be warned in "explicit, clear way" and algorithm will continue to work.

Giovanni Manghi wrote:

Actually I think that we have already the solution: make liblwgeom a dependency (is available on osgeo4w and all major linux distros) and add to qgis geoprocessing tools an option to clean input layers beforehand.

-1 from me. First of all this will increase processing time, moreover, moreover many algs does not really need geometry fixing, e.g. merging vector layers, selecting features etc. Also sometimes automatic cleaning may produce wrong results. IMHO better to leave this to users: there are already algs in Processing toolbox to clean geometry (for example, v.clean, (p)prepair), so users can easily run them before main operation, and even create pre-processing script or models if they need/want to run such tools automatically.

Alexander Bruy wrote:

Giovanni Manghi wrote:

Actually I think that we have already the solution: make liblwgeom a dependency (is available on osgeo4w and all major linux distros) and add to qgis geoprocessing tools an option to clean input layers beforehand.

-1 from me. First of all this will increase processing time, moreover, moreover many algs does not really need geometry fixing, e.g. merging vector layers, selecting features etc. Also sometimes automatic cleaning may produce wrong results. IMHO better to leave this to users

Imho, if it's an option, which is off by default, a "try to automatically clean my input layers" option could be helpful.

Imho, if it's an option, which is off by default, a "try to automatically clean my input layers" option could be helpful.

yes.

Anyway we can discuss this later.

Now it is paramount to (finally) have qgis geoprocessing tools (aka ftools) stop returning wrong results in an almost completely unnoticeable way.

So... warn the user clearly that there are invalid geometries and the result likely to be wrong and/or stop processing at all.

The fix adds a new flag "ignore invalid geometries" which if off by default.

The algorithm will abort and inform the user about the invalid geometries and ask him to either

• fix the geometries
• specify the "ignore invalid geometries" flag

Matthias Kuhn wrote:

The fix adds a new flag "ignore invalid geometries" which if off by default.

The algorithm will abort and inform the user about the invalid geometries and ask him to either

• fix the geometries
• specify the "ignore invalid geometries" flag

Thanks Matthias for tackling this painfully long standing issues!

I believe that the same approach should be applied also to the other geoprocessing operations (union, sym difference, intersection, possibily also clip). Make sense?