Skip to content

Commit 870d207

Browse files
nyalldawsonnyalldawson
committed
[processing] Tweaks and checks for checkParameterValues
1 parent 373b6bb commit 870d207

File tree

5 files changed

+95
-17
lines changed

5 files changed

+95
-17
lines changed

‎python/plugins/processing/gui/AlgorithmDialog.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ def accept(self):
138138
if reply == QMessageBox.No:
139139
return
140140
ok, msg = self.algorithm().checkParameterValues(parameters, context)
141-
if msg:
141+
if not ok:
142142
QMessageBox.warning(
143143
self, self.tr('Unable to execute algorithm'), msg)
144144
return

‎python/plugins/processing/tests/AlgorithmsTestBase.py

+4
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,10 @@ def check_algorithm(self, name, defs):
117117

118118
feedback = QgsProcessingFeedback()
119119

120+
# first check that algorithm accepts the parameters we pass...
121+
ok, msg = alg.checkParameterValues(parameters, context)
122+
self.assertTrue(ok, 'Algorithm failed checkParameterValues with result {}'.format(msg))
123+
120124
if expectFailure:
121125
try:
122126
results, ok = alg.run(parameters, context, feedback)

‎src/analysis/processing/qgsalgorithmbuffer.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,8 @@ void QgsBufferAlgorithm::initAlgorithm( const QVariantMap & )
5555
addParameter( bufferParam.release() );
5656
addParameter( new QgsProcessingParameterNumber( QStringLiteral( "SEGMENTS" ), QObject::tr( "Segments" ), QgsProcessingParameterNumber::Integer, 5, false, 1 ) );
5757

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

6262
addParameter( new QgsProcessingParameterBoolean( QStringLiteral( "DISSOLVE" ), QObject::tr( "Dissolve result" ), false ) );

‎src/core/processing/qgsprocessingparameters.cpp

+19-6
Original file line numberDiff line numberDiff line change
@@ -1230,10 +1230,11 @@ QgsProcessingParameterDefinition::QgsProcessingParameterDefinition( const QStrin
12301230

12311231
bool QgsProcessingParameterDefinition::checkValueIsAcceptable( const QVariant &input, QgsProcessingContext * ) const
12321232
{
1233-
if ( !input.isValid() )
1233+
if ( !input.isValid() && !mDefault.isValid() )
12341234
return mFlags & FlagOptional;
12351235

1236-
if ( input.type() == QVariant::String && input.toString().isEmpty() )
1236+
if ( ( input.type() == QVariant::String && input.toString().isEmpty() )
1237+
|| ( !input.isValid() && mDefault.type() == QVariant::String && mDefault.toString().isEmpty() ) )
12371238
return mFlags & FlagOptional;
12381239

12391240
return true;
@@ -2045,10 +2046,16 @@ QgsProcessingParameterDefinition *QgsProcessingParameterNumber::clone() const
20452046
return new QgsProcessingParameterNumber( *this );
20462047
}
20472048

2048-
bool QgsProcessingParameterNumber::checkValueIsAcceptable( const QVariant &input, QgsProcessingContext * ) const
2049+
bool QgsProcessingParameterNumber::checkValueIsAcceptable( const QVariant &value, QgsProcessingContext * ) const
20492050
{
2051+
QVariant input = value;
20502052
if ( !input.isValid() )
2051-
return mFlags & FlagOptional;
2053+
{
2054+
if ( !defaultValue().isValid() )
2055+
return mFlags & FlagOptional;
2056+
2057+
input = defaultValue();
2058+
}
20522059

20532060
if ( input.canConvert<QgsProperty>() )
20542061
{
@@ -2317,10 +2324,16 @@ QgsProcessingParameterDefinition *QgsProcessingParameterEnum::clone() const
23172324
return new QgsProcessingParameterEnum( *this );
23182325
}
23192326

2320-
bool QgsProcessingParameterEnum::checkValueIsAcceptable( const QVariant &input, QgsProcessingContext * ) const
2327+
bool QgsProcessingParameterEnum::checkValueIsAcceptable( const QVariant &value, QgsProcessingContext * ) const
23212328
{
2329+
QVariant input = value;
23222330
if ( !input.isValid() )
2323-
return mFlags & FlagOptional;
2331+
{
2332+
if ( !defaultValue().isValid() )
2333+
return mFlags & FlagOptional;
2334+
2335+
input = defaultValue();
2336+
}
23242337

23252338
if ( input.canConvert<QgsProperty>() )
23262339
{

‎tests/src/analysis/testqgsprocessing.cpp

100755100644
+69-8
Original file line numberDiff line numberDiff line change
@@ -1983,7 +1983,7 @@ void TestQgsProcessing::parameterBoolean()
19831983
QVERIFY( def->checkValueIsAcceptable( true ) );
19841984
QVERIFY( def->checkValueIsAcceptable( "false" ) );
19851985
QVERIFY( def->checkValueIsAcceptable( "true" ) );
1986-
QVERIFY( !def->checkValueIsAcceptable( QVariant() ) );
1986+
QVERIFY( def->checkValueIsAcceptable( QVariant() ) ); // should be acceptable, because it falls back to default value
19871987

19881988
params.insert( "non_optional_default_true", false );
19891989
QCOMPARE( QgsProcessingParameters::parameterAsBool( def.get(), params, context ), false );
@@ -2006,6 +2006,14 @@ void TestQgsProcessing::parameterBoolean()
20062006
QCOMPARE( fromCode->description(), QStringLiteral( "non optional default true" ) );
20072007
QCOMPARE( fromCode->flags(), def->flags() );
20082008
QCOMPARE( fromCode->defaultValue().toBool(), true );
2009+
2010+
def.reset( new QgsProcessingParameterBoolean( "non_optional_no_default", QString(), QVariant(), false ) );
2011+
2012+
QVERIFY( def->checkValueIsAcceptable( false ) );
2013+
QVERIFY( def->checkValueIsAcceptable( true ) );
2014+
QVERIFY( def->checkValueIsAcceptable( "false" ) );
2015+
QVERIFY( def->checkValueIsAcceptable( "true" ) );
2016+
QVERIFY( !def->checkValueIsAcceptable( QVariant() ) ); // should NOT be acceptable, because it falls back to invalid default value
20092017
}
20102018

20112019
void TestQgsProcessing::parameterCrs()
@@ -3034,7 +3042,7 @@ void TestQgsProcessing::parameterDistance()
30343042
QVERIFY( !def->checkValueIsAcceptable( "1.1,2" ) );
30353043
QVERIFY( !def->checkValueIsAcceptable( "layer12312312" ) );
30363044
QVERIFY( !def->checkValueIsAcceptable( "" ) );
3037-
QVERIFY( !def->checkValueIsAcceptable( QVariant() ) );
3045+
QVERIFY( def->checkValueIsAcceptable( QVariant() ) ); // should be acceptable, falls back to default value
30383046

30393047
// string representing a number
30403048
QVariantMap params;
@@ -3102,6 +3110,18 @@ void TestQgsProcessing::parameterDistance()
31023110
params.insert( "optional", QVariant( "aaaa" ) );
31033111
number = QgsProcessingParameters::parameterAsDouble( def.get(), params, context );
31043112
QGSCOMPARENEAR( number, 5.4, 0.001 );
3113+
3114+
// non-optional, invalid default
3115+
def.reset( new QgsProcessingParameterDistance( "non_optional", QString(), QVariant(), QStringLiteral( "parent" ), false ) );
3116+
QCOMPARE( def->parentParameterName(), QStringLiteral( "parent" ) );
3117+
def->setParentParameterName( QStringLiteral( "parent2" ) );
3118+
QCOMPARE( def->parentParameterName(), QStringLiteral( "parent2" ) );
3119+
QVERIFY( def->checkValueIsAcceptable( 5 ) );
3120+
QVERIFY( def->checkValueIsAcceptable( "1.1" ) );
3121+
QVERIFY( !def->checkValueIsAcceptable( "1.1,2" ) );
3122+
QVERIFY( !def->checkValueIsAcceptable( "layer12312312" ) );
3123+
QVERIFY( !def->checkValueIsAcceptable( "" ) );
3124+
QVERIFY( !def->checkValueIsAcceptable( QVariant() ) ); // should NOT be acceptable, falls back to invalid default value
31053125
}
31063126

31073127
void TestQgsProcessing::parameterNumber()
@@ -3115,7 +3135,7 @@ void TestQgsProcessing::parameterNumber()
31153135
QVERIFY( !def->checkValueIsAcceptable( "1.1,2" ) );
31163136
QVERIFY( !def->checkValueIsAcceptable( "layer12312312" ) );
31173137
QVERIFY( !def->checkValueIsAcceptable( "" ) );
3118-
QVERIFY( !def->checkValueIsAcceptable( QVariant() ) );
3138+
QVERIFY( def->checkValueIsAcceptable( QVariant() ) ); // should be acceptable, falls back to default value
31193139

31203140
// string representing a number
31213141
QVariantMap params;
@@ -3221,6 +3241,15 @@ void TestQgsProcessing::parameterNumber()
32213241
QCOMPARE( fromCode->description(), QStringLiteral( "optional" ) );
32223242
QCOMPARE( fromCode->flags(), def->flags() );
32233243
QVERIFY( !fromCode->defaultValue().isValid() );
3244+
3245+
// non-optional, invalid default
3246+
def.reset( new QgsProcessingParameterNumber( "non_optional", QString(), QgsProcessingParameterNumber::Double, QVariant(), false ) );
3247+
QVERIFY( def->checkValueIsAcceptable( 5 ) );
3248+
QVERIFY( def->checkValueIsAcceptable( "1.1" ) );
3249+
QVERIFY( !def->checkValueIsAcceptable( "1.1,2" ) );
3250+
QVERIFY( !def->checkValueIsAcceptable( "layer12312312" ) );
3251+
QVERIFY( !def->checkValueIsAcceptable( "" ) );
3252+
QVERIFY( !def->checkValueIsAcceptable( QVariant() ) ); // should NOT be acceptable, falls back to invalid default value
32243253
}
32253254

32263255
void TestQgsProcessing::parameterRange()
@@ -3458,7 +3487,7 @@ void TestQgsProcessing::parameterEnum()
34583487
QVERIFY( !def->checkValueIsAcceptable( -1 ) );
34593488
QVERIFY( !def->checkValueIsAcceptable( "layer12312312" ) );
34603489
QVERIFY( !def->checkValueIsAcceptable( "" ) );
3461-
QVERIFY( !def->checkValueIsAcceptable( QVariant() ) );
3490+
QVERIFY( def->checkValueIsAcceptable( QVariant() ) ); // should be acceptable, because falls back to default value
34623491

34633492
// string representing a number
34643493
QVariantMap params;
@@ -3571,7 +3600,7 @@ void TestQgsProcessing::parameterEnum()
35713600
QVERIFY( !def->checkValueIsAcceptable( -1 ) );
35723601
QVERIFY( !def->checkValueIsAcceptable( "layer12312312" ) );
35733602
QVERIFY( !def->checkValueIsAcceptable( "" ) );
3574-
QVERIFY( def->checkValueIsAcceptable( QVariant() ) );
3603+
QVERIFY( !def->checkValueIsAcceptable( QVariant() ) );
35753604

35763605
code = def->asScriptCode();
35773606
QCOMPARE( code, QStringLiteral( "##optional=optional enum a;b 5" ) );
@@ -3628,6 +3657,23 @@ void TestQgsProcessing::parameterEnum()
36283657
QCOMPARE( fromCode->defaultValue(), def->defaultValue() );
36293658
QCOMPARE( fromCode->options(), def->options() );
36303659
QCOMPARE( fromCode->allowMultiple(), def->allowMultiple() );
3660+
3661+
// non optional, no default
3662+
def.reset( new QgsProcessingParameterEnum( "non_optional", QString(), QStringList() << "A" << "B" << "C", false, QVariant(), false ) );
3663+
QVERIFY( !def->checkValueIsAcceptable( false ) );
3664+
QVERIFY( !def->checkValueIsAcceptable( true ) );
3665+
QVERIFY( def->checkValueIsAcceptable( 1 ) );
3666+
QVERIFY( def->checkValueIsAcceptable( "1" ) );
3667+
QVERIFY( !def->checkValueIsAcceptable( "1,2" ) );
3668+
QVERIFY( def->checkValueIsAcceptable( 0 ) );
3669+
QVERIFY( !def->checkValueIsAcceptable( QVariantList() ) );
3670+
QVERIFY( !def->checkValueIsAcceptable( QVariantList() << 1 ) );
3671+
QVERIFY( !def->checkValueIsAcceptable( QVariantList() << "a" ) );
3672+
QVERIFY( !def->checkValueIsAcceptable( 15 ) );
3673+
QVERIFY( !def->checkValueIsAcceptable( -1 ) );
3674+
QVERIFY( !def->checkValueIsAcceptable( "layer12312312" ) );
3675+
QVERIFY( !def->checkValueIsAcceptable( "" ) );
3676+
QVERIFY( !def->checkValueIsAcceptable( QVariant() ) ); // should NOT be acceptable, because falls back to invalid default value
36313677
}
36323678

36333679
void TestQgsProcessing::parameterString()
@@ -3760,18 +3806,25 @@ void TestQgsProcessing::parameterString()
37603806
QCOMPARE( fromCode->flags(), def->flags() );
37613807
QCOMPARE( fromCode->defaultValue(), def->defaultValue() );
37623808
QCOMPARE( fromCode->multiLine(), def->multiLine() );
3809+
3810+
// not optional, valid default!
3811+
def.reset( new QgsProcessingParameterString( "non_optional", QString(), QString( "def" ), false, false ) );
3812+
QVERIFY( def->checkValueIsAcceptable( 1 ) );
3813+
QVERIFY( def->checkValueIsAcceptable( "test" ) );
3814+
QVERIFY( !def->checkValueIsAcceptable( "" ) );
3815+
QVERIFY( def->checkValueIsAcceptable( QVariant() ) ); // should be valid, falls back to valid default
37633816
}
37643817

37653818
void TestQgsProcessing::parameterExpression()
37663819
{
37673820
QgsProcessingContext context;
37683821

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

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

37873840
QString code = def->asScriptCode();
3788-
QCOMPARE( code, QStringLiteral( "##non_optional=expression" ) );
3841+
QCOMPARE( code, QStringLiteral( "##non_optional=expression 1+1" ) );
37893842
std::unique_ptr< QgsProcessingParameterExpression > fromCode( dynamic_cast< QgsProcessingParameterExpression * >( QgsProcessingParameters::parameterFromScriptCode( code ) ) );
37903843
QVERIFY( fromCode.get() );
37913844
QCOMPARE( fromCode->name(), def->name() );
@@ -3832,6 +3885,14 @@ void TestQgsProcessing::parameterExpression()
38323885
QCOMPARE( fromCode->description(), QStringLiteral( "optional" ) );
38333886
QCOMPARE( fromCode->flags(), def->flags() );
38343887
QCOMPARE( fromCode->defaultValue(), def->defaultValue() );
3888+
3889+
// non optional, no default
3890+
def.reset( new QgsProcessingParameterExpression( "non_optional", QString(), QString(), QString(), false ) );
3891+
QVERIFY( def->checkValueIsAcceptable( 1 ) );
3892+
QVERIFY( def->checkValueIsAcceptable( "test" ) );
3893+
QVERIFY( !def->checkValueIsAcceptable( "" ) );
3894+
QVERIFY( !def->checkValueIsAcceptable( QVariant() ) ); // should NOT be acceptable, because it will fallback to invalid default value
3895+
38353896
}
38363897

38373898
void TestQgsProcessing::parameterField()

0 commit comments

Comments
 (0)