[processing] Fix and improve eliminate #4011
Conversation
|
@bstroebl I'm just trying to understand the intended workflow with this algorithm. I gather you must first select the polygons you want to merge (eg by selecting those with an area < certain threshold), and then you run the algorithm? |
|
|
||
| def __init__(self): | ||
| GeoAlgorithm.__init__(self) | ||
| self.showInToolbox = True |
There was a problem hiding this comment.
These aren't needed - showing in toolbox/modeller is the default behaviour
There was a problem hiding this comment.
ah, ok, copied that over from the deprecated algo
| self.tr('%s: (No selection in input layer "%s")' % (self.commandLineName(), self.getParameterValue(self.INPUT)))) | ||
|
|
||
| # Keep references to the features to eliminate | ||
| featToEliminate = [] |
There was a problem hiding this comment.
could simplify this block down to:
featToEliminate = inLayer.selectedFeatures()
There was a problem hiding this comment.
@nyalldawson Correct, although in my experience situations are normally not that simple; usually there are more criteria involved than just the area (GRASS has rmarea in v.clean for that already). I normally use the area/perimeter ratio to identify "real" sliver polygons but sometimes in addition certain attribute combinations are of interest, too. To use an existing selection enables the user to use any tool in QGIS to create this selection (expression builder, manual selection....). On the other hand it does not make sense to apply the algorithm on all polygons.
There was a problem hiding this comment.
@nyalldawson the above is the reply to your first question on the intended workflow
| # select and delete the features in ProcessLayer that are selected in inLayer | ||
| idsToEliminate = [] | ||
|
|
||
| for aFeat in featToEliminate: |
There was a problem hiding this comment.
Am I right in that this whole block is designed to match the selection from the source layer to the copy?
There was a problem hiding this comment.
yes, I did not find another way to do this
There was a problem hiding this comment.
@m-kuhn @wonder-sk Any ideas here? I can't think of anything currently in the api which would simplify this, but having to resort to this kind of process to match features is horrible....
There was a problem hiding this comment.
@nyalldawson I am not happy with that piece of code either. I was thinking about inverting the inLayer's selection and save the then selected features as processLayer. This could be either a normal outputLayer (i.e. shape file) or a memory layer (by copying the code from vector.duplicateInMemory or better, enhancing vector.duplicateInMemory to optionally duplicate only selected features). What do you think about that?
There was a problem hiding this comment.
Does the memory layer not preserve feature ids?
There was a problem hiding this comment.
It would only use memory for the edited features and not the whole layer.
There was a problem hiding this comment.
The buffer would be created with startEditing/stopEditing, right? Sounds like a good compromise. How do I get the layer object if i use output.getVectorWriter to create the layer in the first place?
There was a problem hiding this comment.
The buffer would be created with startEditing/stopEditing, right?
Yes, exactly
How do I get the layer object if i use output.getVectorWriter to create the layer in the first place?
I think you can do
wr = output.getVectorWriter()
# write...
output_layer = output.layer
There was a problem hiding this comment.
@m-kuhn thanks, maybe I try it this weekend
There was a problem hiding this comment.
added a new commit to address this
[FEATURE] allow installing plugins from local ZIP packages
[processing] rename execute sql output
I guess that the intention is parlty solved by the custom widgets
Avoid startup error when project template folder does not exist
Use native QGIS raster API instead of GDAL API in zonal statistics
[FEATURE][processing] removed otb and lidartools providers Tagged as feature to not forget mention this in changelog and docs
In postgres provider, a query layer is detected as a table starting with `(` and ending with `)`.
…ry_layer [BUGFIX][DB Manager] Detect query layer like providers do
Use weak layer pointers in labeling engine
(definitions are often accessed before a QgsSymboLayer/etc is constructed)
has a tag named "properties"
…ls#77 Changed isDeleteStyleFromDBSupported to isDeleteStyleFromDbSupported
…yers A new class QgsPathResolver is introduced for conversion between absolute and relative paths when reading/writing XML. Cleaned up code in layer definition file support (.qlr) to better handle relative/absolute path conversion.
|
Thanks, any objections to merging this from you @nyalldawson ? It looks like all previous comments have been addressed. @bstroebl can you
Thanks a lot and sorry for the delay |
…appers [processing] Add PostGIS widget wrappers
[processing] small fix for R provider
Rework map touch tool
[processing] improve graphs
[processing] gdal translate fixes
…n_wrapper [processing] fix boolean widget wrapper in batch dialog
Tests and fix to read sld:Rotation when does not have ogc sub tags
|
urrks rebasing added all the commits since when I created this branch :-( anyhow, I followed the instructions given in git push --help using
I could close this PR and make a new one based on current master if you like |
Fix: Eliminate claimed to work with an existing selection, this was not the case any more
Improve: Any in-algorithm way to create a selection has been removed as there now is a algorithm dedicated for this. Thus Eliminate can now be used within a model.