Skip to content

Commit

Permalink
[processing] Add a better API for specifying that outputs are temporary
Browse files Browse the repository at this point in the history
Instead of requiring clients to generate temporary file names themselves,
(or use the cryptic "memory:" string!), this PR adds a static constant

    QgsProcessing.TEMPORARY_OUTPUT

If a layer/sink output parameter is set to QgsProcessing.TEMPORARY_OUTPUT,
then the temporary output filename will be generated automatically at
algorithm run time. This means callers don't need to mess around with
finding appropriate temporary folders and paths.

Another benefit is that TEMPORARY_OUTPUT is stored in the processing
history, so if you re-run a previous algorithm which was set to
output to a temporary file, it no longer tries to use that same
previous temporary path and instead generates a new one.
  • Loading branch information
nyalldawson committed Jan 25, 2019
1 parent a90fb87 commit 431861a
Show file tree
Hide file tree
Showing 8 changed files with 102 additions and 8 deletions.
1 change: 1 addition & 0 deletions python/core/auto_generated/processing/qgsprocessing.sip.in
Expand Up @@ -39,6 +39,7 @@ and parameters.
TypeMesh
};

static const QString TEMPORARY_OUTPUT;
};

/************************************************************************
Expand Down
12 changes: 7 additions & 5 deletions python/plugins/processing/gui/DestinationSelectionPanel.py
Expand Up @@ -34,7 +34,8 @@
from qgis.PyQt.QtWidgets import QDialog, QMenu, QAction, QFileDialog, QInputDialog
from qgis.PyQt.QtGui import QCursor
from qgis.gui import QgsEncodingSelectionDialog
from qgis.core import (QgsDataSourceUri,
from qgis.core import (QgsProcessing,
QgsDataSourceUri,
QgsCredentials,
QgsExpression,
QgsSettings,
Expand Down Expand Up @@ -302,7 +303,7 @@ def setValue(self, value):
else:
self.saveToTemporary()
else:
if value == 'memory:':
if value in ('memory:', QgsProcessing.TEMPORARY_OUTPUT):
self.saveToTemporary()
elif isinstance(value, QgsProcessingOutputLayerDefinition):
if value.sink.staticValue() in ('memory:', ''):
Expand All @@ -322,16 +323,17 @@ def setValue(self, value):
def getValue(self):
key = None
if self.use_temporary and isinstance(self.parameter, QgsProcessingParameterFeatureSink):
key = 'memory:'
key = QgsProcessing.TEMPORARY_OUTPUT
elif self.use_temporary and not self.default_selection:
key = self.parameter.generateTemporaryDestination()
key = QgsProcessing.TEMPORARY_OUTPUT
else:
key = self.leText.text()

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

if key and not key.startswith('memory:') \
if key and not key == QgsProcessing.TEMPORARY_OUTPUT \
and not key.startswith('memory:') \
and not key.startswith('ogr:') \
and not key.startswith('postgres:') \
and not key.startswith('postgis:'):
Expand Down
18 changes: 17 additions & 1 deletion python/plugins/processing/tests/GuiTest.py
Expand Up @@ -355,11 +355,17 @@ def testFeatureSink(self):
alg = QgsApplication.processingRegistry().createAlgorithmById('native:centroids')
panel = DestinationSelectionPanel(param, alg)

panel.setValue(QgsProcessing.TEMPORARY_OUTPUT)
v = panel.getValue()
self.assertIsInstance(v, QgsProcessingOutputLayerDefinition)
self.assertEqual(v.createOptions, {'fileEncoding': 'System'})
self.assertEqual(v.sink.staticValue(), QgsProcessing.TEMPORARY_OUTPUT)

panel.setValue('memory:')
v = panel.getValue()
self.assertIsInstance(v, QgsProcessingOutputLayerDefinition)
self.assertEqual(v.createOptions, {'fileEncoding': 'System'})
self.assertEqual(v.sink.staticValue(), 'memory:')
self.assertEqual(v.sink.staticValue(), QgsProcessing.TEMPORARY_OUTPUT)

panel.setValue('''ogr:dbname='/me/a.gpkg' table="d" (geom) sql=''')
v = panel.getValue()
Expand Down Expand Up @@ -391,6 +397,11 @@ def testVectorDestination(self):
alg = QgsApplication.processingRegistry().createAlgorithmById('native:centroids')
panel = DestinationSelectionPanel(param, alg)

panel.setValue(QgsProcessing.TEMPORARY_OUTPUT)
v = panel.getValue()
self.assertIsInstance(v, QgsProcessingOutputLayerDefinition)
self.assertEqual(v.sink.staticValue(), QgsProcessing.TEMPORARY_OUTPUT)

panel.setValue('''ogr:dbname='/me/a.gpkg' table="d" (geom) sql=''')
v = panel.getValue()
self.assertIsInstance(v, QgsProcessingOutputLayerDefinition)
Expand Down Expand Up @@ -427,6 +438,11 @@ def testRasterDestination(self):
self.assertEqual(v.createOptions, {'fileEncoding': 'System'})
self.assertEqual(v.sink.staticValue(), '/home/me/test.tif')

panel.setValue(QgsProcessing.TEMPORARY_OUTPUT)
v = panel.getValue()
self.assertIsInstance(v, QgsProcessingOutputLayerDefinition)
self.assertEqual(v.sink.staticValue(), QgsProcessing.TEMPORARY_OUTPUT)

ProcessingConfig.setSettingValue(ProcessingConfig.OUTPUT_FOLDER, testDataPath)
panel.setValue('test.tif')
v = panel.getValue()
Expand Down
1 change: 1 addition & 0 deletions src/core/CMakeLists.txt
Expand Up @@ -111,6 +111,7 @@ SET(QGIS_CORE_SRCS
locator/qgslocatormodel.cpp
locator/qgslocatormodelbridge.cpp

processing/qgsprocessing.cpp
processing/qgsprocessingalgorithm.cpp
processing/qgsprocessingalgrunnertask.cpp
processing/qgsprocessingcontext.cpp
Expand Down
20 changes: 20 additions & 0 deletions src/core/processing/qgsprocessing.cpp
@@ -0,0 +1,20 @@
/***************************************************************************
qgsprocessing.h
---------------
begin : July 2017
copyright : (C) 2017 by Nyall Dawson
email : nyall dot dawson at gmail dot com
***************************************************************************/

/***************************************************************************
* *
* This program is free software; you can redistribute it and/or modify *
* it under the terms of the GNU General Public License as published by *
* the Free Software Foundation; either version 2 of the License, or *
* (at your option) any later version. *
* *
***************************************************************************/

#include "qgsprocessing.h"

const QString QgsProcessing::TEMPORARY_OUTPUT = QStringLiteral( "TEMPORARY_OUTPUT" );
7 changes: 7 additions & 0 deletions src/core/processing/qgsprocessing.h
Expand Up @@ -19,6 +19,7 @@
#define QGSPROCESSING_H

#include "qgis_core.h"
#include <QString>

//
// Output definitions
Expand Down Expand Up @@ -53,6 +54,12 @@ class CORE_EXPORT QgsProcessing
TypeMesh = 6 //!< Mesh layers \since QGIS 3.6
};

/**
* Constant used to indicate that a Processing algorithm output should be a temporary layer/file.
*
* \since QGIS 3.6
*/
static const QString TEMPORARY_OUTPUT;
};

#endif // QGSPROCESSING_H
16 changes: 15 additions & 1 deletion src/core/processing/qgsprocessingparameters.cpp
Expand Up @@ -399,6 +399,11 @@ QgsFeatureSink *QgsProcessingParameters::parameterAsSink( const QgsProcessingPar
{
dest = val.toString();
}
if ( dest == QgsProcessing::TEMPORARY_OUTPUT )
{
if ( const QgsProcessingDestinationParameter *destParam = dynamic_cast< const QgsProcessingDestinationParameter * >( definition ) )
dest = destParam->generateTemporaryDestination();
}

if ( dest.isEmpty() )
return nullptr;
Expand Down Expand Up @@ -624,6 +629,11 @@ QString QgsProcessingParameters::parameterAsOutputLayer( const QgsProcessingPara
{
dest = val.toString();
}
if ( dest == QgsProcessing::TEMPORARY_OUTPUT )
{
if ( const QgsProcessingDestinationParameter *destParam = dynamic_cast< const QgsProcessingDestinationParameter * >( definition ) )
dest = destParam->generateTemporaryDestination();
}

if ( destinationProject )
{
Expand Down Expand Up @@ -682,7 +692,11 @@ QString QgsProcessingParameters::parameterAsFileOutput( const QgsProcessingParam
{
dest = val.toString();
}

if ( dest == QgsProcessing::TEMPORARY_OUTPUT )
{
if ( const QgsProcessingDestinationParameter *destParam = dynamic_cast< const QgsProcessingDestinationParameter * >( definition ) )
val = destParam->generateTemporaryDestination();
}
return dest;
}

Expand Down
35 changes: 34 additions & 1 deletion tests/src/analysis/testqgsprocessing.cpp
Expand Up @@ -5105,6 +5105,15 @@ void TestQgsProcessing::parameterVectorOut()
QCOMPARE( context2.layersToLoadOnCompletion().values().at( 0 ).outputName, QStringLiteral( "x" ) );
QCOMPARE( context2.layersToLoadOnCompletion().values().at( 0 ).layerTypeHint, QgsProcessingUtils::Vector );

QgsProcessingContext context3;
params.insert( QStringLiteral( "x" ), QgsProcessing::TEMPORARY_OUTPUT );
QCOMPARE( QgsProcessingParameters::parameterAsOutputLayer( def.get(), params, context3 ).right( 6 ), QStringLiteral( "/x.shp" ) );

QgsProcessingContext context4;
fs.sink = QgsProperty::fromValue( QgsProcessing::TEMPORARY_OUTPUT );
params.insert( QStringLiteral( "x" ), QVariant::fromValue( fs ) );
QCOMPARE( QgsProcessingParameters::parameterAsOutputLayer( def.get(), params, context4 ).right( 6 ), QStringLiteral( "/x.shp" ) );

// test supported output vector layer extensions

def.reset( new QgsProcessingParameterVectorDestination( "with_geom", QString(), QgsProcessing::TypeVectorAnyGeometry, QString(), true ) );
Expand Down Expand Up @@ -5291,6 +5300,14 @@ void TestQgsProcessing::parameterFileOut()
params.insert( "non_optional", QgsProcessingOutputLayerDefinition( "test.txt" ) );
QCOMPARE( QgsProcessingParameters::parameterAsFileOutput( def.get(), params, context ), QStringLiteral( "test.txt" ) );

params.insert( QStringLiteral( "non_optional" ), QgsProcessing::TEMPORARY_OUTPUT );
QCOMPARE( QgsProcessingParameters::parameterAsFileOutput( def.get(), params, context ).right( 7 ), QStringLiteral( "_OUTPUT" ) );

QgsProcessingOutputLayerDefinition fs;
fs.sink = QgsProperty::fromValue( QgsProcessing::TEMPORARY_OUTPUT );
params.insert( QStringLiteral( "non_optional" ), QVariant::fromValue( fs ) );
QCOMPARE( QgsProcessingParameters::parameterAsFileOutput( def.get(), params, context ).right( 7 ), QStringLiteral( "_OUTPUT" ) );

QCOMPARE( def->valueAsPythonString( QVariant(), context ), QStringLiteral( "None" ) );
QCOMPARE( def->valueAsPythonString( QStringLiteral( "abc" ), context ), QStringLiteral( "'abc'" ) );
QCOMPARE( def->valueAsPythonString( QVariant::fromValue( QgsProcessingOutputLayerDefinition( "abc" ) ), context ), QStringLiteral( "'abc'" ) );
Expand Down Expand Up @@ -5673,7 +5690,7 @@ void TestQgsProcessing::processingFeatureSink()
context.setProject( &p );

// first using static string definition
std::unique_ptr< QgsProcessingParameterDefinition > def( new QgsProcessingParameterString( QStringLiteral( "layer" ) ) );
std::unique_ptr< QgsProcessingParameterDefinition > def( new QgsProcessingParameterFeatureSink( QStringLiteral( "layer" ) ) );
QVariantMap params;
params.insert( QStringLiteral( "layer" ), QgsProcessingOutputLayerDefinition( "memory:test", nullptr ) );
QString dest;
Expand All @@ -5691,6 +5708,22 @@ void TestQgsProcessing::processingFeatureSink()
QVERIFY( layer2 );
QCOMPARE( layer2->crs().authid(), QStringLiteral( "EPSG:3113" ) );

// temporary sink
params.insert( QStringLiteral( "layer" ), QgsProcessing::TEMPORARY_OUTPUT );
sink.reset( QgsProcessingParameters::parameterAsSink( def.get(), params, QgsFields(), QgsWkbTypes::Point, QgsCoordinateReferenceSystem( "EPSG:28356" ), context, dest ) );
QVERIFY( sink.get() );
QgsVectorLayer *layer3 = qobject_cast< QgsVectorLayer *>( QgsProcessingUtils::mapLayerFromString( dest, context, false ) );
QVERIFY( layer3 );
QCOMPARE( layer3->crs().authid(), QStringLiteral( "EPSG:28356" ) );
QCOMPARE( layer3->dataProvider()->name(), QStringLiteral( "memory" ) );

params.insert( QStringLiteral( "layer" ), QgsProcessingOutputLayerDefinition( QgsProperty::fromValue( QgsProcessing::TEMPORARY_OUTPUT ), nullptr ) );
sink.reset( QgsProcessingParameters::parameterAsSink( def.get(), params, QgsFields(), QgsWkbTypes::Point, QgsCoordinateReferenceSystem( "EPSG:28354" ), context, dest ) );
QVERIFY( sink.get() );
QgsVectorLayer *layer4 = qobject_cast< QgsVectorLayer *>( QgsProcessingUtils::mapLayerFromString( dest, context, false ) );
QVERIFY( layer4 );
QCOMPARE( layer4->crs().authid(), QStringLiteral( "EPSG:28354" ) );
QCOMPARE( layer4->dataProvider()->name(), QStringLiteral( "memory" ) );

// non optional sink
def.reset( new QgsProcessingParameterFeatureSink( QStringLiteral( "layer" ), QString(), QgsProcessing::TypeMapLayer, QVariant(), false ) );
Expand Down

0 comments on commit 431861a

Please sign in to comment.