Port gdalcalc with formula excaping fix #6984

luipir · 2018-05-14T10:06:46Z

Description

Ported missing gdal processing alg gdalcalc (wrapper to wdal_calc) to 3.X. Console tests included.

fix promoted inside GeoMove [1] project for Cartolab [2] (A Coruña university)

[1] http://cartolab.udc.es/geomove/
[2] http://cartolab.udc.es

Checklist

Reviewing is a process done by project maintainers, mostly on a volunteer basis. We try to keep the overhead as small as possible and appreciate if you help us to do so by completing the following items. Feel free to ask in a comment if you have troubles with any of them.

Commit messages are descriptive and explain the rationale for changes
Commits which fix bugs include fixes #11111 in the commit message next to the description
Commits which add new features are tagged with [FEATURE] in the commit message
Commits which change the UI or existing user workflows are tagged with [needs-docs] in the commit message and contain sufficient information in the commit message to be documented
I have read the QGIS Coding Standards and this PR complies with them
This PR passes all existing unit tests (test results will be reported by travis-ci after opening this PR)
New unit tests have been added for core changes
I have run the scripts/prepare-commit.sh script before each commit

nyalldawson

Great!! So nice to see this resurrected.

nyalldawson · 2018-05-14T10:15:30Z

python/plugins/processing/algs/gdal/GdalAlgorithmProvider.py

@@ -176,7 +176,7 @@ def loadAlgorithms(self):
            warp(),
            # rasterize(),
            # ExtractProjection(),
-            # gdalcalc(),
+            gdalcalc(),


#pedantic , but we've started sorting these imports and algorithms

nyalldawson · 2018-05-14T10:17:55Z

python/plugins/processing/algs/gdal/gdalcalc.py

+                self.tr('Calculation in gdalnumeric syntax using +-/* or any numpy array functions (i.e. logical_and())'), 
+                'A*2', 
+                optional=False))
+        self.addParameter(


Should be a number input - copy https://github.com/qgis/QGIS/blob/master/python/plugins/processing/algs/gdal/merge.py#L85

nyalldawson · 2018-05-14T10:18:58Z

python/plugins/processing/algs/gdal/gdalcalc.py

-        self.addParameter(ParameterString(self.EXTRA,
-                                          self.tr('Additional creation parameters'), '', optional=True))
-        self.addOutput(OutputRaster(self.OUTPUT, self.tr('Calculated')))
+        self.addParameter(


Set the metadata for this param to get the nice widget - e.g. copy https://github.com/qgis/QGIS/blob/master/python/plugins/processing/algs/gdal/merge.py#L93

nyalldawson · 2018-05-14T10:19:40Z

python/plugins/processing/algs/gdal/gdalcalc.py

@@ -105,24 +172,32 @@ def group(self):
    def groupId(self):
        return 'rastermiscellaneous'

+    def commandName(self):
+        if isWindows():


commandName doesn't include the .py extension - this should always return 'gdal_calc'

nyalldawson · 2018-05-14T10:19:57Z

python/plugins/processing/algs/gdal/gdalcalc.py

-        noData = self.getParameterValue(self.NO_DATA)
-        if noData is not None:
-            noData = str(noData)
+        if not parameters:


Shouldn't be needed

otherwise would fail test https://github.com/qgis/QGIS/blob/master/python/plugins/processing/tests/GdalAlgorithmsTest.py#L112
or the test should be adapted to consider other type of processing exceptions

If you implement the other changes I've listed here then that test should pass ok

ok I see, tnx

nyalldawson · 2018-05-14T10:21:36Z

python/plugins/processing/algs/gdal/gdalcalc.py

+        #     extra = str(extra)
+        #debug = self.getParameterValue(parameters, self.DEBUG)
+        formula = self.parameterAsString(parameters, self.FORMULA, context)
+        noData = self.parameterAsDouble(parameters, self.NO_DATA, context)


Here you need to handle noData = 0, so copy this logic:
https://github.com/qgis/QGIS/blob/master/python/plugins/processing/algs/gdal/proximity.py#L145

nyalldawson · 2018-05-14T10:22:23Z

python/plugins/processing/algs/gdal/gdalcalc.py


        arguments = []
-        arguments.append('--calc')
-        arguments.append('"' + formula + '"')
+        arguments.append('--calc "{}"'.format(formula) )
        arguments.append('--format')
        arguments.append(GdalUtils.getFormatShortNameFromFilename(out))


Here use QgsRasterFileWriter.driverForExtension(os.path.splitext(out)[1])

nyalldawson · 2018-05-14T10:24:08Z

python/plugins/processing/algs/gdal/gdalcalc.py

-        if self.getParameterValue(self.BAND_A):
-            arguments.append('--A_band ' + self.getParameterValue(self.BAND_A))
-        if self.getParameterValue(self.INPUT_B):
+        arguments.append(self.parameterAsLayer(parameters, self.INPUT_A, context).source())


This needs a bit more safety:

layer_a = self.parameterAsRasterLayer(parameters, self.INPUT_A, context) if layer_a is None: raise QgsProcessingException(self.invalidRasterError(parameters, self.INPUT_A)) arguments.append(layer_a.source())

nyalldawson · 2018-05-14T10:25:38Z

python/plugins/processing/algs/gdal/gdalcalc.py

+        arguments.append(self.parameterAsLayer(parameters, self.INPUT_A, context).source())
+        if self.parameterAsString(parameters, self.BAND_A, context):
+            arguments.append('--A_band ' + self.parameterAsString(parameters, self.BAND_A, context))
+        if self.parameterAsLayer(parameters, self.INPUT_B, context):


For the optional layers you would go:

if self.INPUT_B in parameters and parameters[self.INPUT_B] is not None: layer_b = self.parameterAsRasterLayer(parameters, self.INPUT_B, context) if layer_b is None: raise QgsProcessingException(self.invalidRasterError(parameters, self.INPUT_B)) arguments.append(layer_b.source())

…rrect exception

… in linux

nyalldawson · 2018-05-14T20:21:22Z

python/plugins/processing/algs/gdal/gdalcalc.py

+                'A*2',
+                optional=False))
+        self.addParameter(
+            QgsProcessingParameterString(


This should be a numeric parameter, not string

nyalldawson · 2018-05-14T20:22:19Z

python/plugins/processing/algs/gdal/gdalcalc.py

-            return ['gdal_calc', GdalUtils.escapeAndJoin(arguments)]
-        else:
-            return ['gdal_calc.py', GdalUtils.escapeAndJoin(arguments)]
+        return ['gdal_calc.py', GdalUtils.escapeAndJoin(arguments)]


Do we still need to use gdal_calc (no .py) outside of windows? Other gdal algs still use the non .py command names on Linux.

no idea if it's a problem with my compied installation... but I receive a "gdal_calc" not found if I use self.commandName(), that's running test locally. No idea if it change on travis, it should be strange. btw I try to set again self.commandName and see what append to travis.

sorry I was referring running the alg. Tests just check commandConsole return

GDAL command:
gdal_calc --calc "A*2" --format GTiff --type Float32 -A /home/ginetto/PROGRAMMING/GIS/GISDATA/qgis_sample_data/raster/landcover.img --A_band 1 --outfile /tmp/processing_b75de98ae82149cea15c40d81f2d6b96/c8207372386d409d87ef0351a3718727/OUTPUT.tif
GDAL command output:
/bin/sh: 1: gdal_calc: not found

Ok, confirmed on Fedora - gdal_calc.py works, gdal_calc doesn't

nyalldawson · 2018-05-14T21:58:44Z

Perfect - much appreciated!!

luipir · 2018-05-14T22:14:07Z

thx @nyalldawson for the review

luipir · 2018-05-15T08:19:35Z

@nyalldawson should I have to produce a fix for the commandName() result depending on win or not? (adding .py)? or it's a packaging problem? No idea why travis is able to run the command and a compiled version do not!

nyalldawson · 2018-05-15T08:30:07Z

I'd wait and see. It looks ok to me as is, so let's just see if anyone reports issues.

luipir · 2018-05-15T08:37:59Z

I'll ask to @gioman if he can verify any issue on his different platforms

gioman · 2018-05-15T17:35:49Z

I'll ask to @gioman if he can verify any issue on his different platforms

@luipir on Linux the command works only with the .py extension, on Windows is the opposite.

luipir · 2018-05-15T19:50:59Z

so I can't explain because it pass tests on docker! any idea @3nids ?

nyalldawson · 2018-05-15T20:07:07Z

on Linux the command works only with the .py extension, on Windows is the opposite.

I don't see that - for me I have to use gdal_calc.py:

luipir added 3 commits May 14, 2018 12:05

ported gdalcalc with fix related to escaped formula

9035cb1

rise QgsProcessingException in case no parameters

95b591e

added test for gdal_calc console command

f34af51

luipir requested a review from nyalldawson May 14, 2018 10:06

nyalldawson reviewed May 14, 2018

View reviewed changes

luipir added 15 commits May 14, 2018 13:17

fix indentation

e4bcb9d

moved alg to follow alphabetic order

9ef633e

added raster advanced options

1628473

adapted test to AdvancedRasterOption default to GTiff

d6ba22b

no .py extension to commandName return

159ece1

better management of noData options

fadd343

safer management of A layer

0b18bdc

emit QgsProcessingException if A is not available

33fdc5a

safer management of optional bands

39154f4

removed control of empty parameters delegating to band control the co…

5e2aa0d

…rrect exception

correct nodata management

c9db84a

use gdal_calc.py directly and not commandName, otherwise cannot found…

1ed9004

… in linux

moved test to the correct alphabetic order

1c1c099

fixed test with JPEg out default in test environment

6e91315

fixed way to manage NoData to avoid iniect value when None or ''

82ad2a0

nyalldawson reviewed May 14, 2018

View reviewed changes

luipir added 3 commits May 14, 2018 22:52

correct use of Numeric for NoData + import cleaning

449d841

revert using commandName() instead of gdal_calc.py directly

a4c9e1b

fixed tests to be adaptet to new numeric + fix for commandName

06e5ca4

nyalldawson merged commit 9b28e3d into qgis:master May 14, 2018

luipir deleted the port_gdalcalc_with_formula_exaping_fix branch May 15, 2018 08:43

rudivs mentioned this pull request Jul 5, 2018

Fix gdal_calc for use in modeler #7360

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Port gdalcalc with formula excaping fix #6984

Port gdalcalc with formula excaping fix #6984

luipir commented May 14, 2018 •

edited

Loading

nyalldawson left a comment

nyalldawson May 14, 2018

nyalldawson May 14, 2018

nyalldawson May 14, 2018

nyalldawson May 14, 2018

nyalldawson May 14, 2018

luipir May 14, 2018

nyalldawson May 14, 2018

luipir May 14, 2018

nyalldawson May 14, 2018

nyalldawson May 14, 2018

nyalldawson May 14, 2018

nyalldawson May 14, 2018

nyalldawson May 14, 2018

nyalldawson May 14, 2018

luipir May 14, 2018

luipir May 14, 2018

luipir May 14, 2018

nyalldawson May 14, 2018

nyalldawson commented May 14, 2018

luipir commented May 14, 2018

luipir commented May 15, 2018

nyalldawson commented May 15, 2018

luipir commented May 15, 2018

gioman commented May 15, 2018

luipir commented May 15, 2018

nyalldawson commented May 15, 2018

Port gdalcalc with formula excaping fix #6984

Port gdalcalc with formula excaping fix #6984

Conversation

luipir commented May 14, 2018 • edited Loading

Description

Checklist

nyalldawson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nyalldawson commented May 14, 2018

luipir commented May 14, 2018

luipir commented May 15, 2018

nyalldawson commented May 15, 2018

luipir commented May 15, 2018

gioman commented May 15, 2018

luipir commented May 15, 2018

nyalldawson commented May 15, 2018

luipir commented May 14, 2018 •

edited

Loading