Port checkInputCRS to c++, and allow algorithms to flag when they

require all input layers to be in the same CRS

The default behaviour is to assume that algorithms are well behaved
and can handle multi-CRS inputs, but algs have the option to
flag that they do not allow this and require the input CRS check.

Those algs should document that they require all inputs to have
matching CRS - processing 3.0 behaviour is to assume that algs
can handle this.
This commit is contained in:
Nyall Dawson 2017-06-09 16:55:31 +10:00
parent 386c4246b2
commit 2d2c229332
8 changed files with 160 additions and 26 deletions

View File

@ -29,6 +29,7 @@ class QgsProcessingAlgorithm
FlagHideFromModeler,
FlagSupportsBatch,
FlagCanCancel,
FlagRequiresMatchingCrs,
FlagDeprecated,
};
typedef QFlags<QgsProcessingAlgorithm::Flag> Flags;
@ -244,6 +245,15 @@ class QgsProcessingAlgorithm
:rtype: QgsExpressionContext
%End
virtual bool validateInputCrs( const QVariantMap &parameters,
QgsProcessingContext &context ) const;
%Docstring
Checks whether the coordinate reference systems for the specified set of ``parameters``
are valid for the algorithm. For instance, the base implementation performs
checks to ensure that all input CRS are equal
Returns true if ``parameters`` have passed the CRS check.
:rtype: bool
%End
protected:

View File

@ -264,26 +264,6 @@ class GeoAlgorithm(QgsProcessingAlgorithm):
except:
pass
def checkInputCRS(self, context=None):
"""It checks that all input layers use the same CRS. If so,
returns True. False otherwise.
"""
if context is None:
context = dataobjects.createContext()
crsList = []
for param in self.parameterDefinitions():
if isinstance(param, (ParameterRaster, ParameterVector, ParameterMultipleInput)):
if param.value:
if isinstance(param, ParameterMultipleInput):
layers = param.value.split(';')
else:
layers = [param.value]
for item in layers:
crs = QgsProcessingUtils.mapLayerFromString(item, context).crs()
if crs not in crsList:
crsList.append(crs)
return len(crsList) < 2
def addOutput(self, output):
# TODO: check that name does not exist
if isinstance(output, Output):

View File

@ -212,7 +212,7 @@ class Processing(object):
Processing.tr("Processing"))
return
if not alg.checkInputCRS(context):
if not alg.validateInputCrs(parameters, context):
print('Warning: Not all input layers use the same CRS.\n' +
'This can cause unexpected results.')
QgsMessageLog.logMessage(

View File

@ -168,8 +168,7 @@ class AlgorithmDialog(AlgorithmDialogBase):
QgsMessageLog.logMessage(str(parameters), 'Processing', QgsMessageLog.CRITICAL)
# TODO
if False and checkCRS and not self.alg.checkInputCRS():
if checkCRS and not self.alg.validateInputCrs(parameters, context):
reply = QMessageBox.question(self, self.tr("Unmatching CRS's"),
self.tr('Layers do not all use the same CRS. This can '
'cause unexpected results.\nDo you want to '

View File

@ -125,11 +125,11 @@ class ScriptAlgorithm(GeoAlgorithm):
def canExecute(self):
return not self.error, self.error
def checkInputCRS(self):
def validateInputCrs(self, parameters, context):
if self.noCRSWarning:
return True
else:
return GeoAlgorithm.checkInputCRS(self)
return QgsProcessingAlgorithm.validateInputCrs(self, parameters, context)
def createDescriptiveName(self, s):
return s.replace('_', ' ')

View File

@ -22,6 +22,7 @@
#include "qgsprocessingoutputs.h"
#include "qgsrectangle.h"
#include "qgsprocessingcontext.h"
#include "qgsprocessingutils.h"
QgsProcessingAlgorithm::~QgsProcessingAlgorithm()
{
@ -117,6 +118,73 @@ QgsExpressionContext QgsProcessingAlgorithm::createExpressionContext( const QVar
return c;
}
bool QgsProcessingAlgorithm::validateInputCrs( const QVariantMap &parameters, QgsProcessingContext &context ) const
{
if ( !( flags() & FlagRequiresMatchingCrs ) )
{
// I'm a well behaved algorithm - I take work AWAY from users!
return true;
}
bool foundCrs = false;
QgsCoordinateReferenceSystem crs;
Q_FOREACH ( const QgsProcessingParameterDefinition *def, mParameters )
{
if ( def->type() == QStringLiteral( "layer" ) || def->type() == QStringLiteral( "raster" ) )
{
QgsMapLayer *layer = QgsProcessingParameters::parameterAsLayer( def, parameters, context );
if ( layer )
{
if ( foundCrs && layer->crs().isValid() && crs != layer->crs() )
{
return false;
}
else if ( !foundCrs && layer->crs().isValid() )
{
foundCrs = true;
crs = layer->crs();
}
}
}
else if ( def->type() == QStringLiteral( "source" ) )
{
QgsFeatureSource *source = QgsProcessingParameters::parameterAsSource( def, parameters, context );
if ( source )
{
if ( foundCrs && source->sourceCrs().isValid() && crs != source->sourceCrs() )
{
return false;
}
else if ( !foundCrs && source->sourceCrs().isValid() )
{
foundCrs = true;
crs = source->sourceCrs();
}
}
}
else if ( def->type() == QStringLiteral( "multilayer" ) )
{
QList< QgsMapLayer *> layers = QgsProcessingParameters::parameterAsLayerList( def, parameters, context );
Q_FOREACH ( QgsMapLayer *layer, layers )
{
if ( !layer )
continue;
if ( foundCrs && layer->crs().isValid() && crs != layer->crs() )
{
return false;
}
else if ( !foundCrs && layer->crs().isValid() )
{
foundCrs = true;
crs = layer->crs();
}
}
}
}
return true;
}
bool QgsProcessingAlgorithm::addParameter( QgsProcessingParameterDefinition *definition )
{
if ( !definition )

View File

@ -49,6 +49,7 @@ class CORE_EXPORT QgsProcessingAlgorithm
FlagHideFromModeler = 1 << 2, //!< Algorithm should be hidden from the modeler
FlagSupportsBatch = 1 << 3, //!< Algorithm supports batch mode
FlagCanCancel = 1 << 4, //!< Algorithm can be canceled
FlagRequiresMatchingCrs = 1 << 5, //!< Algorithm requires that all input layers have matching coordinate reference systems
FlagDeprecated = FlagHideFromToolbox | FlagHideFromModeler, //!< Algorithm is deprecated
};
Q_DECLARE_FLAGS( Flags, Flag )
@ -243,6 +244,14 @@ class CORE_EXPORT QgsProcessingAlgorithm
QgsExpressionContext createExpressionContext( const QVariantMap &parameters,
QgsProcessingContext &context ) const;
/**
* Checks whether the coordinate reference systems for the specified set of \a parameters
* are valid for the algorithm. For instance, the base implementation performs
* checks to ensure that all input CRS are equal
* Returns true if \a parameters have passed the CRS check.
*/
virtual bool validateInputCrs( const QVariantMap &parameters,
QgsProcessingContext &context ) const;
protected:

View File

@ -36,14 +36,19 @@ class DummyAlgorithm : public QgsProcessingAlgorithm
{
public:
DummyAlgorithm( const QString &name ) : mName( name ) {}
DummyAlgorithm( const QString &name ) : mName( name ) { mFlags = QgsProcessingAlgorithm::flags(); }
QString name() const override { return mName; }
QString displayName() const override { return mName; }
virtual QVariantMap processAlgorithm( const QVariantMap &,
QgsProcessingContext &, QgsProcessingFeedback * ) const override { return QVariantMap(); }
virtual Flags flags() const override { return mFlags; }
QString mName;
Flags mFlags;
void checkParameterVals()
{
addParameter( new QgsProcessingParameterString( "p1" ) );
@ -126,6 +131,62 @@ class DummyAlgorithm : public QgsProcessingAlgorithm
QVERIFY( hasHtmlOutputs() );
}
void runValidateInputCrsChecks()
{
addParameter( new QgsProcessingParameterMapLayer( "p1" ) );
addParameter( new QgsProcessingParameterMapLayer( "p2" ) );
QVariantMap parameters;
QgsVectorLayer *layer3111 = new QgsVectorLayer( "Point?crs=epsg:3111", "v1", "memory" );
QgsProject p;
p.addMapLayer( layer3111 );
QString testDataDir = QStringLiteral( TEST_DATA_DIR ) + '/'; //defined in CmakeLists.txt
QString raster1 = testDataDir + "tenbytenraster.asc";
QFileInfo fi1( raster1 );
QgsRasterLayer *r1 = new QgsRasterLayer( fi1.filePath(), "R1" );
QVERIFY( r1->isValid() );
p.addMapLayer( r1 );
QgsVectorLayer *layer4326 = new QgsVectorLayer( "Point?crs=epsg:4326", "v1", "memory" );
p.addMapLayer( layer4326 );
QgsProcessingContext context;
context.setProject( &p );
// flag not set
mFlags = 0;
parameters.insert( "p1", QVariant::fromValue( layer3111 ) );
QVERIFY( validateInputCrs( parameters, context ) );
mFlags = FlagRequiresMatchingCrs;
QVERIFY( validateInputCrs( parameters, context ) );
// two layers, different crs
parameters.insert( "p2", QVariant::fromValue( layer4326 ) );
// flag not set
mFlags = 0;
QVERIFY( validateInputCrs( parameters, context ) );
mFlags = FlagRequiresMatchingCrs;
QVERIFY( !validateInputCrs( parameters, context ) );
// raster layer
parameters.remove( "p2" );
addParameter( new QgsProcessingParameterRasterLayer( "p3" ) );
parameters.insert( "p3", QVariant::fromValue( r1 ) );
QVERIFY( !validateInputCrs( parameters, context ) );
// feature source
parameters.remove( "p3" );
addParameter( new QgsProcessingParameterFeatureSource( "p4" ) );
parameters.insert( "p4", layer4326->id() );
QVERIFY( !validateInputCrs( parameters, context ) );
parameters.remove( "p4" );
addParameter( new QgsProcessingParameterMultipleLayers( "p5" ) );
parameters.insert( "p5", QVariantList() << layer4326->id() << r1->id() );
QVERIFY( !validateInputCrs( parameters, context ) );
}
};
//dummy provider for testing
@ -228,6 +289,7 @@ class TestQgsProcessing: public QObject
void processingFeatureSource();
void processingFeatureSink();
void algorithmScope();
void validateInputCrs();
private:
@ -2640,5 +2702,11 @@ void TestQgsProcessing::algorithmScope()
QCOMPARE( exp2.evaluate( &context ).toInt(), 5 );
}
void TestQgsProcessing::validateInputCrs()
{
DummyAlgorithm alg( "test" );
alg.runValidateInputCrsChecks();
}
QGSTEST_MAIN( TestQgsProcessing )
#include "testqgsprocessing.moc"