Skip to content

Commit

Permalink
[processing] Tweaks and checks for checkParameterValues
Browse files Browse the repository at this point in the history
  • Loading branch information
@nyalldawson
nyalldawson committed May 28, 2018
1 parent 373b6bb commit 870d207
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 17 deletions.
2 changes: 1 addition & 1 deletion python/plugins/processing/gui/AlgorithmDialog.py
View file
Expand Up @@ -138,7 +138,7 @@ def accept(self):
if reply == QMessageBox.No:
return
ok, msg = self.algorithm().checkParameterValues(parameters, context)
if msg:
if not ok:
QMessageBox.warning(
self, self.tr('Unable to execute algorithm'), msg)
return
Expand Down
4 changes: 4 additions & 0 deletions python/plugins/processing/tests/AlgorithmsTestBase.py
View file
Expand Up @@ -117,6 +117,10 @@ def check_algorithm(self, name, defs):

feedback = QgsProcessingFeedback()

# first check that algorithm accepts the parameters we pass...
ok, msg = alg.checkParameterValues(parameters, context)
self.assertTrue(ok, 'Algorithm failed checkParameterValues with result {}'.format(msg))

if expectFailure:
try:
results, ok = alg.run(parameters, context, feedback)
Expand Down
4 changes: 2 additions & 2 deletions src/analysis/processing/qgsalgorithmbuffer.cpp
View file
Expand Up @@ -55,8 +55,8 @@ void QgsBufferAlgorithm::initAlgorithm( const QVariantMap & )
addParameter( bufferParam.release() );
addParameter( new QgsProcessingParameterNumber( QStringLiteral( "SEGMENTS" ), QObject::tr( "Segments" ), QgsProcessingParameterNumber::Integer, 5, false, 1 ) );

addParameter( new QgsProcessingParameterEnum( QStringLiteral( "END_CAP_STYLE" ), QObject::tr( "End cap style" ), QStringList() << QObject::tr( "Round" ) << QObject::tr( "Flat" ) << QObject::tr( "Square" ), false ) );
addParameter( new QgsProcessingParameterEnum( QStringLiteral( "JOIN_STYLE" ), QObject::tr( "Join style" ), QStringList() << QObject::tr( "Round" ) << QObject::tr( "Miter" ) << QObject::tr( "Bevel" ), false ) );
addParameter( new QgsProcessingParameterEnum( QStringLiteral( "END_CAP_STYLE" ), QObject::tr( "End cap style" ), QStringList() << QObject::tr( "Round" ) << QObject::tr( "Flat" ) << QObject::tr( "Square" ), false, 0 ) );
addParameter( new QgsProcessingParameterEnum( QStringLiteral( "JOIN_STYLE" ), QObject::tr( "Join style" ), QStringList() << QObject::tr( "Round" ) << QObject::tr( "Miter" ) << QObject::tr( "Bevel" ), false, 0 ) );
addParameter( new QgsProcessingParameterNumber( QStringLiteral( "MITER_LIMIT" ), QObject::tr( "Miter limit" ), QgsProcessingParameterNumber::Double, 2, false, 1 ) );

addParameter( new QgsProcessingParameterBoolean( QStringLiteral( "DISSOLVE" ), QObject::tr( "Dissolve result" ), false ) );
Expand Down
25 changes: 19 additions & 6 deletions src/core/processing/qgsprocessingparameters.cpp
View file
Expand Up @@ -1230,10 +1230,11 @@ QgsProcessingParameterDefinition::QgsProcessingParameterDefinition( const QStrin

bool QgsProcessingParameterDefinition::checkValueIsAcceptable( const QVariant &input, QgsProcessingContext * ) const
{
if ( !input.isValid() )
if ( !input.isValid() && !mDefault.isValid() )
return mFlags & FlagOptional;

if ( input.type() == QVariant::String && input.toString().isEmpty() )
if ( ( input.type() == QVariant::String && input.toString().isEmpty() )
|| ( !input.isValid() && mDefault.type() == QVariant::String && mDefault.toString().isEmpty() ) )
return mFlags & FlagOptional;

return true;
Expand Down Expand Up @@ -2045,10 +2046,16 @@ QgsProcessingParameterDefinition *QgsProcessingParameterNumber::clone() const
return new QgsProcessingParameterNumber( *this );
}

bool QgsProcessingParameterNumber::checkValueIsAcceptable( const QVariant &input, QgsProcessingContext * ) const
bool QgsProcessingParameterNumber::checkValueIsAcceptable( const QVariant &value, QgsProcessingContext * ) const
{
QVariant input = value;
if ( !input.isValid() )
return mFlags & FlagOptional;
{
if ( !defaultValue().isValid() )
return mFlags & FlagOptional;

input = defaultValue();
}

if ( input.canConvert<QgsProperty>() )
{
Expand Down Expand Up @@ -2317,10 +2324,16 @@ QgsProcessingParameterDefinition *QgsProcessingParameterEnum::clone() const
return new QgsProcessingParameterEnum( *this );
}

bool QgsProcessingParameterEnum::checkValueIsAcceptable( const QVariant &input, QgsProcessingContext * ) const
bool QgsProcessingParameterEnum::checkValueIsAcceptable( const QVariant &value, QgsProcessingContext * ) const
{
QVariant input = value;
if ( !input.isValid() )
return mFlags & FlagOptional;
{
if ( !defaultValue().isValid() )
return mFlags & FlagOptional;

input = defaultValue();
}

if ( input.canConvert<QgsProperty>() )
{
Expand Down
77 changes: 69 additions & 8 deletions tests/src/analysis/testqgsprocessing.cpp 100755 → 100644
View file
Expand Up @@ -1983,7 +1983,7 @@ void TestQgsProcessing::parameterBoolean()
QVERIFY( def->checkValueIsAcceptable( true ) );
QVERIFY( def->checkValueIsAcceptable( "false" ) );
QVERIFY( def->checkValueIsAcceptable( "true" ) );
QVERIFY( !def->checkValueIsAcceptable( QVariant() ) );
QVERIFY( def->checkValueIsAcceptable( QVariant() ) ); // should be acceptable, because it falls back to default value

params.insert( "non_optional_default_true", false );
QCOMPARE( QgsProcessingParameters::parameterAsBool( def.get(), params, context ), false );
Expand All @@ -2006,6 +2006,14 @@ void TestQgsProcessing::parameterBoolean()
QCOMPARE( fromCode->description(), QStringLiteral( "non optional default true" ) );
QCOMPARE( fromCode->flags(), def->flags() );
QCOMPARE( fromCode->defaultValue().toBool(), true );

def.reset( new QgsProcessingParameterBoolean( "non_optional_no_default", QString(), QVariant(), false ) );

QVERIFY( def->checkValueIsAcceptable( false ) );
QVERIFY( def->checkValueIsAcceptable( true ) );
QVERIFY( def->checkValueIsAcceptable( "false" ) );
QVERIFY( def->checkValueIsAcceptable( "true" ) );
QVERIFY( !def->checkValueIsAcceptable( QVariant() ) ); // should NOT be acceptable, because it falls back to invalid default value
}

void TestQgsProcessing::parameterCrs()
Expand Down Expand Up @@ -3034,7 +3042,7 @@ void TestQgsProcessing::parameterDistance()
QVERIFY( !def->checkValueIsAcceptable( "1.1,2" ) );
QVERIFY( !def->checkValueIsAcceptable( "layer12312312" ) );
QVERIFY( !def->checkValueIsAcceptable( "" ) );
QVERIFY( !def->checkValueIsAcceptable( QVariant() ) );
QVERIFY( def->checkValueIsAcceptable( QVariant() ) ); // should be acceptable, falls back to default value

// string representing a number
QVariantMap params;
Expand Down Expand Up @@ -3102,6 +3110,18 @@ void TestQgsProcessing::parameterDistance()
params.insert( "optional", QVariant( "aaaa" ) );
number = QgsProcessingParameters::parameterAsDouble( def.get(), params, context );
QGSCOMPARENEAR( number, 5.4, 0.001 );

// non-optional, invalid default
def.reset( new QgsProcessingParameterDistance( "non_optional", QString(), QVariant(), QStringLiteral( "parent" ), false ) );
QCOMPARE( def->parentParameterName(), QStringLiteral( "parent" ) );
def->setParentParameterName( QStringLiteral( "parent2" ) );
QCOMPARE( def->parentParameterName(), QStringLiteral( "parent2" ) );
QVERIFY( def->checkValueIsAcceptable( 5 ) );
QVERIFY( def->checkValueIsAcceptable( "1.1" ) );
QVERIFY( !def->checkValueIsAcceptable( "1.1,2" ) );
QVERIFY( !def->checkValueIsAcceptable( "layer12312312" ) );
QVERIFY( !def->checkValueIsAcceptable( "" ) );
QVERIFY( !def->checkValueIsAcceptable( QVariant() ) ); // should NOT be acceptable, falls back to invalid default value
}

void TestQgsProcessing::parameterNumber()
Expand All @@ -3115,7 +3135,7 @@ void TestQgsProcessing::parameterNumber()
QVERIFY( !def->checkValueIsAcceptable( "1.1,2" ) );
QVERIFY( !def->checkValueIsAcceptable( "layer12312312" ) );
QVERIFY( !def->checkValueIsAcceptable( "" ) );
QVERIFY( !def->checkValueIsAcceptable( QVariant() ) );
QVERIFY( def->checkValueIsAcceptable( QVariant() ) ); // should be acceptable, falls back to default value

// string representing a number
QVariantMap params;
Expand Down Expand Up @@ -3221,6 +3241,15 @@ void TestQgsProcessing::parameterNumber()
QCOMPARE( fromCode->description(), QStringLiteral( "optional" ) );
QCOMPARE( fromCode->flags(), def->flags() );
QVERIFY( !fromCode->defaultValue().isValid() );

// non-optional, invalid default
def.reset( new QgsProcessingParameterNumber( "non_optional", QString(), QgsProcessingParameterNumber::Double, QVariant(), false ) );
QVERIFY( def->checkValueIsAcceptable( 5 ) );
QVERIFY( def->checkValueIsAcceptable( "1.1" ) );
QVERIFY( !def->checkValueIsAcceptable( "1.1,2" ) );
QVERIFY( !def->checkValueIsAcceptable( "layer12312312" ) );
QVERIFY( !def->checkValueIsAcceptable( "" ) );
QVERIFY( !def->checkValueIsAcceptable( QVariant() ) ); // should NOT be acceptable, falls back to invalid default value
}

void TestQgsProcessing::parameterRange()
Expand Down Expand Up @@ -3458,7 +3487,7 @@ void TestQgsProcessing::parameterEnum()
QVERIFY( !def->checkValueIsAcceptable( -1 ) );
QVERIFY( !def->checkValueIsAcceptable( "layer12312312" ) );
QVERIFY( !def->checkValueIsAcceptable( "" ) );
QVERIFY( !def->checkValueIsAcceptable( QVariant() ) );
QVERIFY( def->checkValueIsAcceptable( QVariant() ) ); // should be acceptable, because falls back to default value

// string representing a number
QVariantMap params;
Expand Down Expand Up @@ -3571,7 +3600,7 @@ void TestQgsProcessing::parameterEnum()
QVERIFY( !def->checkValueIsAcceptable( -1 ) );
QVERIFY( !def->checkValueIsAcceptable( "layer12312312" ) );
QVERIFY( !def->checkValueIsAcceptable( "" ) );
QVERIFY( def->checkValueIsAcceptable( QVariant() ) );
QVERIFY( !def->checkValueIsAcceptable( QVariant() ) );

code = def->asScriptCode();
QCOMPARE( code, QStringLiteral( "##optional=optional enum a;b 5" ) );
Expand Down Expand Up @@ -3628,6 +3657,23 @@ void TestQgsProcessing::parameterEnum()
QCOMPARE( fromCode->defaultValue(), def->defaultValue() );
QCOMPARE( fromCode->options(), def->options() );
QCOMPARE( fromCode->allowMultiple(), def->allowMultiple() );

// non optional, no default
def.reset( new QgsProcessingParameterEnum( "non_optional", QString(), QStringList() << "A" << "B" << "C", false, QVariant(), false ) );
QVERIFY( !def->checkValueIsAcceptable( false ) );
QVERIFY( !def->checkValueIsAcceptable( true ) );
QVERIFY( def->checkValueIsAcceptable( 1 ) );
QVERIFY( def->checkValueIsAcceptable( "1" ) );
QVERIFY( !def->checkValueIsAcceptable( "1,2" ) );
QVERIFY( def->checkValueIsAcceptable( 0 ) );
QVERIFY( !def->checkValueIsAcceptable( QVariantList() ) );
QVERIFY( !def->checkValueIsAcceptable( QVariantList() << 1 ) );
QVERIFY( !def->checkValueIsAcceptable( QVariantList() << "a" ) );
QVERIFY( !def->checkValueIsAcceptable( 15 ) );
QVERIFY( !def->checkValueIsAcceptable( -1 ) );
QVERIFY( !def->checkValueIsAcceptable( "layer12312312" ) );
QVERIFY( !def->checkValueIsAcceptable( "" ) );
QVERIFY( !def->checkValueIsAcceptable( QVariant() ) ); // should NOT be acceptable, because falls back to invalid default value
}

void TestQgsProcessing::parameterString()
Expand Down Expand Up @@ -3760,18 +3806,25 @@ void TestQgsProcessing::parameterString()
QCOMPARE( fromCode->flags(), def->flags() );
QCOMPARE( fromCode->defaultValue(), def->defaultValue() );
QCOMPARE( fromCode->multiLine(), def->multiLine() );

// not optional, valid default!
def.reset( new QgsProcessingParameterString( "non_optional", QString(), QString( "def" ), false, false ) );
QVERIFY( def->checkValueIsAcceptable( 1 ) );
QVERIFY( def->checkValueIsAcceptable( "test" ) );
QVERIFY( !def->checkValueIsAcceptable( "" ) );
QVERIFY( def->checkValueIsAcceptable( QVariant() ) ); // should be valid, falls back to valid default
}

void TestQgsProcessing::parameterExpression()
{
QgsProcessingContext context;

// not optional!
std::unique_ptr< QgsProcessingParameterExpression > def( new QgsProcessingParameterExpression( "non_optional", QString(), QString(), QString(), false ) );
std::unique_ptr< QgsProcessingParameterExpression > def( new QgsProcessingParameterExpression( "non_optional", QString(), QString( "1+1" ), QString(), false ) );
QVERIFY( def->checkValueIsAcceptable( 1 ) );
QVERIFY( def->checkValueIsAcceptable( "test" ) );
QVERIFY( !def->checkValueIsAcceptable( "" ) );
QVERIFY( !def->checkValueIsAcceptable( QVariant() ) );
QVERIFY( def->checkValueIsAcceptable( QVariant() ) ); // should be acceptable, because it will fallback to default value

// string
QVariantMap params;
Expand All @@ -3785,7 +3838,7 @@ void TestQgsProcessing::parameterExpression()
QCOMPARE( def->valueAsPythonString( QVariant::fromValue( QgsProperty::fromExpression( "\"a\"=1" ) ), context ), QStringLiteral( "QgsProperty.fromExpression('\"a\"=1')" ) );

QString code = def->asScriptCode();
QCOMPARE( code, QStringLiteral( "##non_optional=expression" ) );
QCOMPARE( code, QStringLiteral( "##non_optional=expression 1+1" ) );
std::unique_ptr< QgsProcessingParameterExpression > fromCode( dynamic_cast< QgsProcessingParameterExpression * >( QgsProcessingParameters::parameterFromScriptCode( code ) ) );
QVERIFY( fromCode.get() );
QCOMPARE( fromCode->name(), def->name() );
Expand Down Expand Up @@ -3832,6 +3885,14 @@ void TestQgsProcessing::parameterExpression()
QCOMPARE( fromCode->description(), QStringLiteral( "optional" ) );
QCOMPARE( fromCode->flags(), def->flags() );
QCOMPARE( fromCode->defaultValue(), def->defaultValue() );

// non optional, no default
def.reset( new QgsProcessingParameterExpression( "non_optional", QString(), QString(), QString(), false ) );
QVERIFY( def->checkValueIsAcceptable( 1 ) );
QVERIFY( def->checkValueIsAcceptable( "test" ) );
QVERIFY( !def->checkValueIsAcceptable( "" ) );
QVERIFY( !def->checkValueIsAcceptable( QVariant() ) ); // should NOT be acceptable, because it will fallback to invalid default value

}

void TestQgsProcessing::parameterField()
Expand Down

0 comments on commit 870d207

Please sign in to comment.