Skip to content

Commit

Permalink
If a feature sink parameter is optional and not set, don't create the…
Browse files Browse the repository at this point in the history 
… sink

This adds a lot of flexibility to algorithms, as it makes output
sinks truely optional. For instance, the various "Extract by..."
algorithms could add a new optional sink for features which
'fail' the extraction criteria. This effectively allows these
algorithms to become feature 'routers', directing features onto
other parts of a model depending on whether they pass or fail
the test.

But in this situation we don't always care about these failing
features, and we don't want to force them to always be fetched
from the provider. By making the outputs truely optional,
the algorithm can tweak its logic to either fetch all features
and send them to the correct output, or only fetch
matching features from the provider in the first place (a big
speed boost).
  • Loading branch information
@nyalldawson
nyalldawson committed Jun 9, 2017
1 parent 2d2dff9 commit 6b55300
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 10 deletions.
5 changes: 3 additions & 2 deletions python/plugins/processing/gui/AlgorithmDialog.py
View file
Expand Up @@ -118,8 +118,9 @@ def getParamValues(self):
dest_project = QgsProject.instance()

value = self.mainWidget.outputWidgets[param.name()].getValue()
value.destinationProject = dest_project
parameters[param.name()] = value
if value:
value.destinationProject = dest_project
parameters[param.name()] = value

return parameters

Expand Down
43 changes: 38 additions & 5 deletions python/plugins/processing/gui/DestinationSelectionPanel.py
View file
Expand Up @@ -39,7 +39,8 @@
QgsExpression,
QgsSettings,
QgsProcessingParameterFeatureSink,
QgsProcessingOutputLayerDefinition)
QgsProcessingOutputLayerDefinition,
QgsProcessingParameterDefinition)
from processing.core.ProcessingConfig import ProcessingConfig
from processing.core.outputs import OutputVector
from processing.core.outputs import OutputDirectory
Expand All @@ -58,6 +59,8 @@ class DestinationSelectionPanel(BASE, WIDGET):
'DestinationSelectionPanel', '[Save to temporary file]')
SAVE_TO_TEMP_LAYER = QCoreApplication.translate(
'DestinationSelectionPanel', '[Create temporary layer]')
SKIP_OUTPUT = QCoreApplication.translate(
'DestinationSelectionPanel', '[Skip output]')

def __init__(self, parameter, alg):
super(DestinationSelectionPanel, self).__init__(None)
Expand All @@ -67,23 +70,42 @@ def __init__(self, parameter, alg):
self.alg = alg
settings = QgsSettings()
self.encoding = settings.value('/Processing/encoding', 'System')
self.use_temporary = True

if hasattr(self.leText, 'setPlaceholderText'):
if isinstance(self.parameter, QgsProcessingParameterFeatureSink) \
if parameter.flags() & QgsProcessingParameterDefinition.FlagOptional and parameter.defaultValue() is None:
self.leText.setPlaceholderText(self.SKIP_OUTPUT)
self.use_temporary = False
elif isinstance(self.parameter, QgsProcessingParameterFeatureSink) \
and alg.provider().supportsNonFileBasedOutput():
# use memory layers for temporary files if supported
self.leText.setPlaceholderText(self.SAVE_TO_TEMP_LAYER)
else:
self.leText.setPlaceholderText(self.SAVE_TO_TEMP_FILE)

self.btnSelect.clicked.connect(self.selectOutput)
self.leText.textEdited.connect(self.textChanged)

def textChanged(self):
self.use_temporary = False

def skipOutput(self):
self.leText.setPlaceholderText(self.SKIP_OUTPUT)
self.use_temporary = False

def selectOutput(self):
if isinstance(self.parameter, OutputDirectory):
self.selectDirectory()
else:
popupMenu = QMenu()

if isinstance(self.parameter, QgsProcessingParameterFeatureSink) \
and self.parameter.flags() & QgsProcessingParameterDefinition.FlagOptional:
actionSkipOutput = QAction(
self.tr('Skip output'), self.btnSelect)
actionSkipOutput.triggered.connect(self.skipOutput)
popupMenu.addAction(actionSkipOutput)

if isinstance(self.parameter, QgsProcessingParameterFeatureSink) \
and self.alg.provider().supportsNonFileBasedOutput():
# use memory layers for temporary layers if supported
Expand Down Expand Up @@ -133,12 +155,18 @@ def showExpressionsBuilder(self):
self.leText.setText(expression.evaluate(context))

def saveToTemporary(self):
if isinstance(self.parameter, QgsProcessingParameterFeatureSink) and self.alg.provider().supportsNonFileBasedOutput():
self.leText.setPlaceholderText(self.SAVE_TO_TEMP_LAYER)
else:
self.leText.setPlaceholderText(self.SAVE_TO_TEMP_FILE)
self.leText.setText('')
self.use_temporary = True

def saveToPostGIS(self):
dlg = PostgisTableSelector(self, self.parameter.name().lower())
dlg.exec_()
if dlg.connection:
self.use_temporary = False
settings = QgsSettings()
mySettings = '/PostgreSQL/connections/' + dlg.connection
dbname = settings.value(mySettings + '/database')
Expand Down Expand Up @@ -173,6 +201,7 @@ def saveToSpatialite(self):
fileDialog.setOption(QFileDialog.DontConfirmOverwrite, True)

if fileDialog.exec_() == QDialog.Accepted:
self.use_temporary = False
files = fileDialog.selectedFiles()
self.encoding = str(fileDialog.encoding())
fileName = str(files[0])
Expand Down Expand Up @@ -208,6 +237,7 @@ def selectFile(self):
fileDialog.setOption(QFileDialog.DontConfirmOverwrite, False)

if fileDialog.exec_() == QDialog.Accepted:
self.use_temporary = False
files = fileDialog.selectedFiles()
self.encoding = str(fileDialog.encoding())
fileName = str(files[0])
Expand All @@ -230,11 +260,14 @@ def selectDirectory(self):

def getValue(self):
key = None
if not self.leText.text():
if isinstance(self.parameter, QgsProcessingParameterFeatureSink):
key = 'memory:'
if self.use_temporary and isinstance(self.parameter, QgsProcessingParameterFeatureSink):
key = 'memory:'
else:
key = self.leText.text()

if not key and self.parameter.flags() & QgsProcessingParameterDefinition.FlagOptional:
return None

value = QgsProcessingOutputLayerDefinition(key)
value.createOptions = {'fileEncoding': self.encoding}
return value
5 changes: 5 additions & 0 deletions src/core/processing/qgsprocessingparameters.cpp
View file
Expand Up @@ -231,6 +231,11 @@ QgsFeatureSink *QgsProcessingParameters::parameterAsSink( const QgsProcessingPar
}
else if ( !val.isValid() || val.toString().isEmpty() )
{
if ( definition && definition->flags() & QgsProcessingParameterDefinition::FlagOptional && !definition->defaultValue().isValid() )
{
// unset, optional sink, no default => no sink
return nullptr;
}
// fall back to default
dest = definition->defaultValue().toString();
}
Expand Down
41 changes: 38 additions & 3 deletions tests/src/core/testqgsprocessing.cpp
View file
Expand Up @@ -2562,23 +2562,58 @@ void TestQgsProcessing::processingFeatureSink()
context.setProject( &p );

// first using static string definition
QgsProcessingParameterDefinition *def = new QgsProcessingParameterString( QStringLiteral( "layer" ) );
std::unique_ptr< QgsProcessingParameterDefinition > def( new QgsProcessingParameterString( QStringLiteral( "layer" ) ) );
QVariantMap params;
params.insert( QStringLiteral( "layer" ), QgsProcessingOutputLayerDefinition( "memory:test", nullptr ) );
QString dest;
std::unique_ptr< QgsFeatureSink > sink( QgsProcessingParameters::parameterAsSink( def, params, QgsFields(), QgsWkbTypes::Point, QgsCoordinateReferenceSystem( "EPSG:3111" ), context, dest ) );
std::unique_ptr< QgsFeatureSink > sink( QgsProcessingParameters::parameterAsSink( def.get(), params, QgsFields(), QgsWkbTypes::Point, QgsCoordinateReferenceSystem( "EPSG:3111" ), context, dest ) );
QVERIFY( sink.get() );
QgsVectorLayer *layer = qobject_cast< QgsVectorLayer *>( QgsProcessingUtils::mapLayerFromString( dest, context, false ) );
QVERIFY( layer );
QCOMPARE( layer->crs().authid(), QStringLiteral( "EPSG:3111" ) );

// next using property based definition
params.insert( QStringLiteral( "layer" ), QgsProcessingOutputLayerDefinition( QgsProperty::fromExpression( QStringLiteral( "trim('memory' + ':test2')" ) ), nullptr ) );
sink.reset( QgsProcessingParameters::parameterAsSink( def, params, QgsFields(), QgsWkbTypes::Point, QgsCoordinateReferenceSystem( "EPSG:3113" ), context, dest ) );
sink.reset( QgsProcessingParameters::parameterAsSink( def.get(), params, QgsFields(), QgsWkbTypes::Point, QgsCoordinateReferenceSystem( "EPSG:3113" ), context, dest ) );
QVERIFY( sink.get() );
QgsVectorLayer *layer2 = qobject_cast< QgsVectorLayer *>( QgsProcessingUtils::mapLayerFromString( dest, context, false ) );
QVERIFY( layer2 );
QCOMPARE( layer2->crs().authid(), QStringLiteral( "EPSG:3113" ) );


// non optional sink
def.reset( new QgsProcessingParameterFeatureSink( QStringLiteral( "layer" ), QString(), QgsProcessingParameterDefinition::TypeAny, QVariant(), false ) );
QVERIFY( def->checkValueIsAcceptable( QStringLiteral( "memory:test" ) ) );
QVERIFY( def->checkValueIsAcceptable( QgsProcessingOutputLayerDefinition( "memory:test" ) ) );
QVERIFY( def->checkValueIsAcceptable( QgsProperty::fromValue( "memory:test" ) ) );
QVERIFY( !def->checkValueIsAcceptable( QString() ) );
QVERIFY( !def->checkValueIsAcceptable( QVariant() ) );
QVERIFY( !def->checkValueIsAcceptable( 5 ) );
params.insert( QStringLiteral( "layer" ), QStringLiteral( "memory:test" ) );
sink.reset( QgsProcessingParameters::parameterAsSink( def.get(), params, QgsFields(), QgsWkbTypes::Point, QgsCoordinateReferenceSystem( "EPSG:3113" ), context, dest ) );
QVERIFY( sink.get() );

// optional sink
def.reset( new QgsProcessingParameterFeatureSink( QStringLiteral( "layer" ), QString(), QgsProcessingParameterDefinition::TypeAny, QVariant(), true ) );
QVERIFY( def->checkValueIsAcceptable( QStringLiteral( "memory:test" ) ) );
QVERIFY( def->checkValueIsAcceptable( QgsProcessingOutputLayerDefinition( "memory:test" ) ) );
QVERIFY( def->checkValueIsAcceptable( QgsProperty::fromValue( "memory:test" ) ) );
QVERIFY( def->checkValueIsAcceptable( QString() ) );
QVERIFY( def->checkValueIsAcceptable( QVariant() ) );
QVERIFY( !def->checkValueIsAcceptable( 5 ) );
params.insert( QStringLiteral( "layer" ), QStringLiteral( "memory:test" ) );
sink.reset( QgsProcessingParameters::parameterAsSink( def.get(), params, QgsFields(), QgsWkbTypes::Point, QgsCoordinateReferenceSystem( "EPSG:3113" ), context, dest ) );
QVERIFY( sink.get() );
// optional sink, not set - should be no sink
params.insert( QStringLiteral( "layer" ), QVariant() );
sink.reset( QgsProcessingParameters::parameterAsSink( def.get(), params, QgsFields(), QgsWkbTypes::Point, QgsCoordinateReferenceSystem( "EPSG:3113" ), context, dest ) );
QVERIFY( !sink.get() );

//.... unless there's a default set
def.reset( new QgsProcessingParameterFeatureSink( QStringLiteral( "layer" ), QString(), QgsProcessingParameterDefinition::TypeAny, QStringLiteral( "memory:defaultlayer" ), true ) );
params.insert( QStringLiteral( "layer" ), QVariant() );
sink.reset( QgsProcessingParameters::parameterAsSink( def.get(), params, QgsFields(), QgsWkbTypes::Point, QgsCoordinateReferenceSystem( "EPSG:3113" ), context, dest ) );
QVERIFY( sink.get() );
}

void TestQgsProcessing::algorithmScope()
Expand Down

0 comments on commit 6b55300

Please sign in to comment.