[processing] allow passing additional command line parameters to GDAL algorithms

[alexbruy]

Description

Adds a new optional advanced parameter to majority of the Processing GDAL algorithms allowing users to pass additional command line parameters which are not exposed as algorithm parameters. Addresses number of existing (and possible future) feature request, e.g. #28813, #28210, #26990, #25784, #26280, #14378 and probably #19619.

Also GDAL algorithms test now reorganized into separate files similarly to the QGIS algorithms test. There are separate test for raster and vector algorithms as well as general test for common stuff. This makes easier to check logs to find failing test and also speedups tests execution a bit.

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

[alexbruy]

@nyalldawson as most of the GDAL algorithms are covered with robust tests which check generated command line, I'm wondering is it make sense to keep YAML-based tests. They are sometimes flaky and give different results on Tavis and local machine. If we decide to drop or reduce them, I will be happy to port uncovered algorithms to the command line based tests.

[m-kuhn]

They are sometimes flaky and give different results on Tavis and local machine

Do you have any idea why they behave flaky and give different results?

[alexbruy]

@m-kuhn sorry no ideas. Maybe different libraries versions or env settings.

[m-kuhn]

Are the other tests more robust against these parameters?

[alexbruy]

This is not related to the added parameters, I was talking in general. For example, raster hashes gerenerated in local machine and on Travis sometimes different, or feature geometry/count is different.

if I'm not wrong this is why a tests which compare generated GDAL command line with a valid command were developed.

[m-kuhn]

Ah, now I understand, they just compare the command line. Yes, then they should be robust against this and more portable.
On the other hand, actually running the algorithm in tests additionally covers a bigger range of potential issues.

PS: With "parameters" I was referring to "env and library versions", not processing parameters :)

[nyalldawson]

I'm in between views here -- along the lines of what @m-kuhn's said, I think we should keep ONE hash test per algorithm, just to ensure that the gdal command is actually run and that the algorithm returns with exceptions. But ultimately these tests can only verify that something is generated, and haven't been any use in actually verifying that the output is correct. So I think our main attention should be focused on the command line tests like @alexbruy has done here, and trust that gdal gets things right from there...

So that said, +1 to merge this, looks great to me!

[m-kuhn]

The hash based tests suffer from the ability to even manually identify "slightly varying" or "improved". Not just for GDAL. I understand very well if they are dropped.
What about the others?

[nyalldawson]

@m-kuhn again, I'd say we aim for 1 "compare the result" test per algorithm, and limit testing of handling of different parameter settings to a command line only test.

[alexbruy]

Don't get me wrong, I'm not against tests which actually run commands. They are important too and allow use to be sure that algorithms can be actually executed and nothing broken in the provider/Processing code.

Let's keep both types of tests, and maybe we will be able to find why some of them produce different results in different environments

[m-kuhn]

Feel free to remove individual tests. I'm just a bit afraid of the "let's drop it almost altogether" approach. I'd prefer to drop (or flag) tests which have been identified to suffer from such issues.
In such cases I'd invest a minimal amount of time to see if reducing coordinate precision can fix the issue and then easily flag them without spending too much energy.

[nyalldawson]

@m-kuhn I've never seen issues with the vector tests, only the raster hashes.

[m-kuhn]

Since we are sidetracking the discussion already... Maybe we should switch to something with data too. At least as an alternative to the hashes. I'm a bit afraid of inflating the repo but if we are careful, that could work and allow for adding some tolerance too.

[luipir]

sorry @alexbruy I'm late (sabbatic)... just a tech question, if EXTRA belongs to all command why not reorganize initAlgorithm adding base parameters in the GdalAlgorithm::initAlgorithm called as super before to add custom parameter for each reimplemented initAlgorithm?

[alexbruy]

There is no sense to add it to all commands, as some tools have only few parameters which are alredy exposed via GUI.

[luipir]

There is no sense to add it to all commands

IMHO can be disabled by default or set as advanced params... btw my 2c to avoid to duplicate a lot of code and maintain that!