From 191e362ed441b3571e51a89444161bca99012e46 Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Mon, 1 Jun 2020 15:55:22 +1000 Subject: [PATCH] Move FieldMapper parameter type to c++ Fixes sip "forgeting" about Python overrides for the type definition Refs #36706 --- .../qgsprocessingparameterfieldmap.sip.in | 7 +- ...singparametervectortilewriterlayers.sip.in | 2 +- python/core/core_auto.sip | 1 + .../processing/algs/qgis/FieldsMapper.py | 82 ++----------------- .../algs/qgis/QgisAlgorithmProvider.py | 13 +-- .../algs/qgis/ui/FieldsMappingPanel.py | 2 +- .../qgsprocessingparameterfieldmap.cpp | 36 +++++--- .../qgsprocessingparameterfieldmap.h | 5 +- .../processing/qgsprocessingparameters.cpp | 1 + ...rocessingparametervectortilewriterlayers.h | 2 +- tests/src/analysis/testqgsprocessing.cpp | 69 ++++++++++++++++ 11 files changed, 111 insertions(+), 109 deletions(-) diff --git a/python/core/auto_generated/processing/qgsprocessingparameterfieldmap.sip.in b/python/core/auto_generated/processing/qgsprocessingparameterfieldmap.sip.in index 345ae7facdf..d2227e488ae 100644 --- a/python/core/auto_generated/processing/qgsprocessingparameterfieldmap.sip.in +++ b/python/core/auto_generated/processing/qgsprocessingparameterfieldmap.sip.in @@ -8,7 +8,6 @@ - class QgsProcessingParameterFieldMapping : QgsProcessingParameterDefinition { %Docstring @@ -24,7 +23,7 @@ class QgsProcessingParameterFieldMapping : QgsProcessingParameterDefinition #include "qgsprocessingparameterfieldmap.h" %End public: - QgsProcessingParameterFieldMapping( const QString &name, const QString &description = QString(), const QString &parentLayerParameterName = QString() ); + QgsProcessingParameterFieldMapping( const QString &name, const QString &description = QString(), const QString &parentLayerParameterName = QString(), bool optional = false ); %Docstring Constructor for QgsProcessingParameterFieldMapping. %End @@ -33,7 +32,7 @@ Constructor for QgsProcessingParameterFieldMapping. virtual QString type() const; - virtual bool checkValueIsAcceptable( const QVariant &input, QgsProcessingContext *context ) const; + virtual bool checkValueIsAcceptable( const QVariant &input, QgsProcessingContext *context = 0 ) const; virtual QString valueAsPythonString( const QVariant &value, QgsProcessingContext &context ) const; @@ -43,6 +42,8 @@ Constructor for QgsProcessingParameterFieldMapping. virtual bool fromVariantMap( const QVariantMap &map ); + virtual QStringList dependsOnOtherParameters() const; + static QString typeName(); %Docstring diff --git a/python/core/auto_generated/processing/qgsprocessingparametervectortilewriterlayers.sip.in b/python/core/auto_generated/processing/qgsprocessingparametervectortilewriterlayers.sip.in index a0d5609b11f..dff142a86e2 100644 --- a/python/core/auto_generated/processing/qgsprocessingparametervectortilewriterlayers.sip.in +++ b/python/core/auto_generated/processing/qgsprocessingparametervectortilewriterlayers.sip.in @@ -48,7 +48,7 @@ Constructor for QgsProcessingParameterVectorTileWriterLayers. virtual QString type() const; - virtual bool checkValueIsAcceptable( const QVariant &input, QgsProcessingContext *context ) const; + virtual bool checkValueIsAcceptable( const QVariant &input, QgsProcessingContext *context = 0 ) const; virtual QString valueAsPythonString( const QVariant &value, QgsProcessingContext &context ) const; diff --git a/python/core/core_auto.sip b/python/core/core_auto.sip index 10e0af62922..40b757b7b95 100644 --- a/python/core/core_auto.sip +++ b/python/core/core_auto.sip @@ -451,6 +451,7 @@ %Include auto_generated/processing/qgsprocessingcontext.sip %Include auto_generated/processing/qgsprocessingfeedback.sip %Include auto_generated/processing/qgsprocessingoutputs.sip +%Include auto_generated/processing/qgsprocessingparameterfieldmap.sip %Include auto_generated/processing/qgsprocessingparameters.sip %Include auto_generated/processing/qgsprocessingparametertype.sip %Include auto_generated/processing/qgsprocessingparametervectortilewriterlayers.sip diff --git a/python/plugins/processing/algs/qgis/FieldsMapper.py b/python/plugins/processing/algs/qgis/FieldsMapper.py index 564e1eb5af9..764ac66c6e7 100644 --- a/python/plugins/processing/algs/qgis/FieldsMapper.py +++ b/python/plugins/processing/algs/qgis/FieldsMapper.py @@ -28,12 +28,9 @@ from qgis.core import ( QgsFields, QgsProcessing, QgsProcessingException, - QgsProcessingParameterDefinition, - QgsProcessingParameterType, + QgsProcessingParameterFieldMapping, NULL) -from qgis.PyQt.QtCore import QCoreApplication - from processing.algs.qgis.QgisAlgorithm import QgisFeatureBasedAlgorithm @@ -52,8 +49,9 @@ class FieldsMapper(QgisFeatureBasedAlgorithm): return self.tr('attributes,table').split(',') def initParameters(self, config=None): - fields_mapping = FieldsMapper.ParameterFieldsMapping(self.FIELDS_MAPPING, - description=self.tr('Fields mapping')) + fields_mapping = QgsProcessingParameterFieldMapping(self.FIELDS_MAPPING, + description=self.tr('Fields mapping'), + parentLayerParameterName='INPUT') fields_mapping.setMetadata({ 'widget_wrapper': 'processing.algs.qgis.ui.FieldsMappingPanel.FieldsMappingWidgetWrapper' }) @@ -108,7 +106,7 @@ class FieldsMapper(QgisFeatureBasedAlgorithm): if expression.hasParserError(): feedback.reportError( self.tr('Parser error for field "{}" with expression "{}": {}') - .format( + .format( field_def['name'], expression.expression(), expression.parserErrorString()), @@ -147,73 +145,3 @@ class FieldsMapper(QgisFeatureBasedAlgorithm): feature.setAttributes(attributes) self._row_number += 1 return [feature] - - class ParameterFieldsMappingType(QgsProcessingParameterType): - - def __init__(self): - super().__init__() - - def create(self, name): - return FieldsMapper.ParameterFieldsMapping(name) - - def metadata(self): - return {'widget_wrapper': 'processing.algs.qgis.ui.FieldsMappingPanel.FieldsMappingWidgetWrapper'} - - def name(self): - return QCoreApplication.translate('Processing', 'Fields Mapper') - - def id(self): - return 'fields_mapping' - - def pythonImportString(self): - return 'from processing.algs.qgis.FieldsMapper import FieldsMapper' - - def className(self): - return 'FieldsMapper.ParameterFieldsMapping' - - def description(self): - return QCoreApplication.translate('Processing', 'A mapping of field names to field type definitions and expressions. Used for the refactor fields algorithm.') - - class ParameterFieldsMapping(QgsProcessingParameterDefinition): - - def __init__(self, name, description='', parentLayerParameterName='INPUT'): - super().__init__(name, description) - self._parentLayerParameter = parentLayerParameterName - - def clone(self): - copy = FieldsMapper.ParameterFieldsMapping(self.name(), self.description(), self._parentLayerParameter) - return copy - - def type(self): - return self.typeName() - - @staticmethod - def typeName(): - return 'fields_mapping' - - def checkValueIsAcceptable(self, value, context=None): - if not isinstance(value, list): - return False - for field_def in value: - if not isinstance(field_def, dict): - return False - if 'name' not in field_def.keys(): - return False - if 'type' not in field_def.keys(): - return False - if 'expression' not in field_def.keys(): - return False - return True - - def valueAsPythonString(self, value, context): - return str(value) - - def asScriptCode(self): - raise NotImplementedError() - - @classmethod - def fromScriptCode(cls, name, description, isOptional, definition): - raise NotImplementedError() - - def parentLayerParameter(self): - return self._parentLayerParameter diff --git a/python/plugins/processing/algs/qgis/QgisAlgorithmProvider.py b/python/plugins/processing/algs/qgis/QgisAlgorithmProvider.py index 5da51136bf1..4f0e01e8e6f 100644 --- a/python/plugins/processing/algs/qgis/QgisAlgorithmProvider.py +++ b/python/plugins/processing/algs/qgis/QgisAlgorithmProvider.py @@ -100,8 +100,6 @@ class QgisAlgorithmProvider(QgsProcessingProvider): def __init__(self): super().__init__() - self.algs = [] - self.externalAlgs = [] QgsApplication.processingRegistry().addAlgorithmAlias('qgis:rectanglesovalsdiamondsfixed', 'native:rectanglesovalsdiamonds') def getAlgs(self): @@ -190,26 +188,17 @@ class QgisAlgorithmProvider(QgsProcessingProvider): return QgsApplication.iconPath("providerQgis.svg") def loadAlgorithms(self): - self.algs = self.getAlgs() - for a in self.algs: - self.addAlgorithm(a) - for a in self.externalAlgs: + for a in self.getAlgs(): self.addAlgorithm(a) def load(self): with QgsRuntimeProfiler.profile('QGIS Python Provider'): success = super().load() - if success: - self.parameterTypeFieldsMapping = FieldsMapper.ParameterFieldsMappingType() - QgsApplication.instance().processingRegistry().addParameterType(self.parameterTypeFieldsMapping) - return success def unload(self): super().unload() - QgsApplication.instance().processingRegistry().removeParameterType(self.parameterTypeFieldsMapping) - def supportsNonFileBasedOutput(self): return True diff --git a/python/plugins/processing/algs/qgis/ui/FieldsMappingPanel.py b/python/plugins/processing/algs/qgis/ui/FieldsMappingPanel.py index e7a856fb75e..5da162a51ad 100644 --- a/python/plugins/processing/algs/qgis/ui/FieldsMappingPanel.py +++ b/python/plugins/processing/algs/qgis/ui/FieldsMappingPanel.py @@ -214,7 +214,7 @@ class FieldsMappingWidgetWrapper(WidgetWrapper): def postInitialize(self, wrappers): for wrapper in wrappers: - if wrapper.parameterDefinition().name() == self.parameterDefinition().parentLayerParameter(): + if wrapper.parameterDefinition().name() == self.parameterDefinition().parentLayerParameterName(): if wrapper.parameterValue(): self.setLayer(wrapper.parameterValue()) wrapper.widgetValueHasChanged.connect(self.parentLayerChanged) diff --git a/src/core/processing/qgsprocessingparameterfieldmap.cpp b/src/core/processing/qgsprocessingparameterfieldmap.cpp index 163de67fe88..19e550391a7 100644 --- a/src/core/processing/qgsprocessingparameterfieldmap.cpp +++ b/src/core/processing/qgsprocessingparameterfieldmap.cpp @@ -18,8 +18,8 @@ #include "qgsvectorlayer.h" -QgsProcessingParameterFieldMapping::QgsProcessingParameterFieldMapping( const QString &name, const QString &description, const QString &parentLayerParameterName ) - : QgsProcessingParameterDefinition( name, description, QVariant(), false ) +QgsProcessingParameterFieldMapping::QgsProcessingParameterFieldMapping( const QString &name, const QString &description, const QString &parentLayerParameterName, bool optional ) + : QgsProcessingParameterDefinition( name, description, QVariant(), optional ) , mParentLayerParameterName( parentLayerParameterName ) { } @@ -34,7 +34,7 @@ QString QgsProcessingParameterFieldMapping::type() const return typeName(); } -bool QgsProcessingParameterFieldMapping::checkValueIsAcceptable( const QVariant &input, QgsProcessingContext *context ) const +bool QgsProcessingParameterFieldMapping::checkValueIsAcceptable( const QVariant &input, QgsProcessingContext * ) const { if ( !input.isValid() ) return mFlags & FlagOptional; @@ -61,13 +61,9 @@ bool QgsProcessingParameterFieldMapping::checkValueIsAcceptable( const QVariant return true; } -QString QgsProcessingParameterFieldMapping::valueAsPythonString( const QVariant &value, QgsProcessingContext &context ) const +QString QgsProcessingParameterFieldMapping::valueAsPythonString( const QVariant &value, QgsProcessingContext & ) const { - QStringList parts; - - // TODO - - return parts.join( ',' ).prepend( '[' ).append( ']' ); + return QgsProcessingUtils::variantToPythonLiteral( value ); } QString QgsProcessingParameterFieldMapping::asPythonString( QgsProcessing::PythonOutputType outputType ) const @@ -76,7 +72,13 @@ QString QgsProcessingParameterFieldMapping::asPythonString( QgsProcessing::Pytho { case QgsProcessing::PythonQgsProcessingAlgorithmSubclass: { - QString code = QStringLiteral( "QgsProcessingParameterVectorTileWriterLayers('%1', '%2')" ).arg( name(), description() ); + QString code = QStringLiteral( "QgsProcessingParameterFieldMapping('%1', '%2'" ).arg( name(), description() ); + if ( !mParentLayerParameterName.isEmpty() ) + code += QStringLiteral( ", parentLayerParameterName=%1" ).arg( QgsProcessingUtils::stringToPythonLiteral( mParentLayerParameterName ) ); + + if ( mFlags & FlagOptional ) + code += QStringLiteral( ", optional=True" ); + code += ')'; return code; } } @@ -85,12 +87,24 @@ QString QgsProcessingParameterFieldMapping::asPythonString( QgsProcessing::Pytho QVariantMap QgsProcessingParameterFieldMapping::toVariantMap() const { - + QVariantMap map = QgsProcessingParameterDefinition::toVariantMap(); + map.insert( QStringLiteral( "parent_layer" ), mParentLayerParameterName ); + return map; } bool QgsProcessingParameterFieldMapping::fromVariantMap( const QVariantMap &map ) { + QgsProcessingParameterDefinition::fromVariantMap( map ); + mParentLayerParameterName = map.value( QStringLiteral( "parent_layer" ) ).toString(); + return true; +} +QStringList QgsProcessingParameterFieldMapping::dependsOnOtherParameters() const +{ + QStringList depends; + if ( !mParentLayerParameterName.isEmpty() ) + depends << mParentLayerParameterName; + return depends; } QString QgsProcessingParameterFieldMapping::parentLayerParameterName() const diff --git a/src/core/processing/qgsprocessingparameterfieldmap.h b/src/core/processing/qgsprocessingparameterfieldmap.h index b0607195f7b..7a8f48c49e4 100644 --- a/src/core/processing/qgsprocessingparameterfieldmap.h +++ b/src/core/processing/qgsprocessingparameterfieldmap.h @@ -18,8 +18,6 @@ #include "qgsprocessingparameters.h" #include "qgsprocessingparametertype.h" -#include "qgsvectortilewriter.h" - /** * \ingroup core @@ -30,7 +28,7 @@ class CORE_EXPORT QgsProcessingParameterFieldMapping : public QgsProcessingParam { public: //! Constructor for QgsProcessingParameterFieldMapping. - QgsProcessingParameterFieldMapping( const QString &name, const QString &description = QString(), const QString &parentLayerParameterName = QString() ); + QgsProcessingParameterFieldMapping( const QString &name, const QString &description = QString(), const QString &parentLayerParameterName = QString(), bool optional = false ); QgsProcessingParameterDefinition *clone() const override; QString type() const override; @@ -39,6 +37,7 @@ class CORE_EXPORT QgsProcessingParameterFieldMapping : public QgsProcessingParam QString asPythonString( QgsProcessing::PythonOutputType outputType = QgsProcessing::PythonQgsProcessingAlgorithmSubclass ) const override; QVariantMap toVariantMap() const override; bool fromVariantMap( const QVariantMap &map ) override; + QStringList dependsOnOtherParameters() const override; //! Returns the type name for the parameter class. static QString typeName() { return QStringLiteral( "fields_mapping" ); } diff --git a/src/core/processing/qgsprocessingparameters.cpp b/src/core/processing/qgsprocessingparameters.cpp index 53b17ec9f03..77917f2cc20 100644 --- a/src/core/processing/qgsprocessingparameters.cpp +++ b/src/core/processing/qgsprocessingparameters.cpp @@ -1959,6 +1959,7 @@ QgsProcessingParameterDefinition *QgsProcessingParameters::parameterFromVariantM def.reset( new QgsProcessingParameterColor( name ) ); else if ( type == QgsProcessingParameterCoordinateOperation::typeName() ) def.reset( new QgsProcessingParameterCoordinateOperation( name ) ); + else { QgsProcessingParameterType *paramType = QgsApplication::instance()->processingRegistry()->parameterType( type ); if ( paramType ) diff --git a/src/core/processing/qgsprocessingparametervectortilewriterlayers.h b/src/core/processing/qgsprocessingparametervectortilewriterlayers.h index 7976dc1d03c..7a7f7cfaa73 100644 --- a/src/core/processing/qgsprocessingparametervectortilewriterlayers.h +++ b/src/core/processing/qgsprocessingparametervectortilewriterlayers.h @@ -50,7 +50,7 @@ class CORE_EXPORT QgsProcessingParameterVectorTileWriterLayers : public QgsProce QgsProcessingParameterDefinition *clone() const override; QString type() const override; - bool checkValueIsAcceptable( const QVariant &input, QgsProcessingContext *context ) const override; + bool checkValueIsAcceptable( const QVariant &input, QgsProcessingContext *context = nullptr ) const override; QString valueAsPythonString( const QVariant &value, QgsProcessingContext &context ) const override; QString asPythonString( QgsProcessing::PythonOutputType outputType = QgsProcessing::PythonQgsProcessingAlgorithmSubclass ) const override; diff --git a/tests/src/analysis/testqgsprocessing.cpp b/tests/src/analysis/testqgsprocessing.cpp index 4095681ebe0..1dfd2b56f11 100644 --- a/tests/src/analysis/testqgsprocessing.cpp +++ b/tests/src/analysis/testqgsprocessing.cpp @@ -48,6 +48,7 @@ #include "qgslayoutitemlabel.h" #include "qgscoordinatetransformcontext.h" #include "qgsrasterfilewriter.h" +#include "qgsprocessingparameterfieldmap.h" class DummyAlgorithm : public QgsProcessingAlgorithm { @@ -596,6 +597,7 @@ class TestQgsProcessing: public QObject void parameterProviderConnection(); void parameterDatabaseSchema(); void parameterDatabaseTable(); + void parameterFieldMapping(); void checkParamValues(); void combineLayerExtent(); void processingFeatureSource(); @@ -7592,6 +7594,73 @@ void TestQgsProcessing::parameterDatabaseTable() QVERIFY( def->allowNewTableNames() ); } +void TestQgsProcessing::parameterFieldMapping() +{ + QgsProcessingContext context; + + // not optional! + std::unique_ptr< QgsProcessingParameterFieldMapping > def( new QgsProcessingParameterFieldMapping( "non_optional", QString(), QStringLiteral( "parent" ), false ) ); + QVERIFY( !def->checkValueIsAcceptable( 1 ) ); + QVERIFY( !def->checkValueIsAcceptable( "test" ) ); + QVERIFY( !def->checkValueIsAcceptable( "" ) ); + QVERIFY( def->checkValueIsAcceptable( QVariantList() ) ); + QVariantMap map; + QVERIFY( !def->checkValueIsAcceptable( QVariantList() << map ) ); + map.insert( QStringLiteral( "name" ), QStringLiteral( "n" ) ); + QVERIFY( !def->checkValueIsAcceptable( QVariantList() << map ) ); + map.insert( QStringLiteral( "type" ), QStringLiteral( "t" ) ); + QVERIFY( !def->checkValueIsAcceptable( QVariantList() << map ) ); + map.insert( QStringLiteral( "expression" ), QStringLiteral( "e" ) ); + QVERIFY( def->checkValueIsAcceptable( QVariantList() << map ) ); + QVariantMap map2; + map2.insert( QStringLiteral( "name" ), QStringLiteral( "n2" ) ); + map2.insert( QStringLiteral( "type" ), QStringLiteral( "t2" ) ); + map2.insert( QStringLiteral( "expression" ), QStringLiteral( "e2" ) ); + QVERIFY( def->checkValueIsAcceptable( QVariantList() << map << map2 ) ); + + QVariantMap params; + params.insert( "non_optional", QVariantList() << map << map2 ); + + QCOMPARE( def->valueAsPythonString( QVariant(), context ), QStringLiteral( "None" ) ); + QCOMPARE( def->valueAsPythonString( 5, context ), QStringLiteral( "5" ) ); + QCOMPARE( def->valueAsPythonString( QStringLiteral( "abc" ), context ), QStringLiteral( "'abc'" ) ); + QCOMPARE( def->valueAsPythonString( QStringLiteral( "abc\ndef" ), context ), QStringLiteral( "'abc\\ndef'" ) ); + QCOMPARE( def->valueAsPythonString( QVariant::fromValue( QgsProperty::fromExpression( "\"a\"=1" ) ), context ), QStringLiteral( "QgsProperty.fromExpression('\"a\"=1')" ) ); + QCOMPARE( def->valueAsPythonString( QVariant( QVariantList() << map << map2 ), context ), QStringLiteral( "[{'expression': 'e','name': 'n','type': 't'},{'expression': 'e2','name': 'n2','type': 't2'}]" ) ); + + QString pythonCode = def->asPythonString(); + QCOMPARE( pythonCode, QStringLiteral( "QgsProcessingParameterFieldMapping('non_optional', '', parentLayerParameterName='parent')" ) ); + + QVariantMap mapDef = def->toVariantMap(); + QgsProcessingParameterFieldMapping fromMap( "x" ); + QVERIFY( fromMap.fromVariantMap( mapDef ) ); + QCOMPARE( fromMap.name(), def->name() ); + QCOMPARE( fromMap.description(), def->description() ); + QCOMPARE( fromMap.flags(), def->flags() ); + QCOMPARE( fromMap.defaultValue(), def->defaultValue() ); + QCOMPARE( fromMap.parentLayerParameterName(), def->parentLayerParameterName() ); + def.reset( dynamic_cast< QgsProcessingParameterFieldMapping *>( QgsProcessingParameters::parameterFromVariantMap( mapDef ) ) ); + QVERIFY( dynamic_cast< QgsProcessingParameterFieldMapping *>( def.get() ) ); + + def->setParentLayerParameterName( QString() ); + QVERIFY( def->dependsOnOtherParameters().isEmpty() ); + def->setParentLayerParameterName( QStringLiteral( "test_layer" ) ); + QCOMPARE( def->dependsOnOtherParameters(), QStringList() << QStringLiteral( "test_layer" ) ); + + + // optional + def.reset( new QgsProcessingParameterFieldMapping( "non_optional", QString(), QStringLiteral( "parent" ), true ) ); + QVERIFY( !def->checkValueIsAcceptable( 1 ) ); + QVERIFY( !def->checkValueIsAcceptable( "test" ) ); + QVERIFY( !def->checkValueIsAcceptable( "" ) ); + QVERIFY( def->checkValueIsAcceptable( QVariantList() ) ); + QVERIFY( def->checkValueIsAcceptable( QVariantList() << map << map2 ) ); + QVERIFY( def->checkValueIsAcceptable( QVariant() ) ); + + pythonCode = def->asPythonString(); + QCOMPARE( pythonCode, QStringLiteral( "QgsProcessingParameterFieldMapping('non_optional', '', parentLayerParameterName='parent', optional=True)" ) ); +} + void TestQgsProcessing::parameterDateTime() { QgsProcessingContext context;