[processing] support show/hide parameters in providers

[rkanavath]

This commits allows one to control visiblity of parameters in
AlgorithmDialog UI. mVisible flag in QgsProcessingParameterDefinition
defines whether the parameter widget and its associate QLabel is
visible in AlgorithmDialog.

A processing provider such as OTB requires one such feature for smooth
integration of its application through gui. Rules that decide if
parameter should be visible at any point is left out for providers to
handle. OTB has a fixed scheme for this behavior and it is explained
here.
https://lists.osgeo.org/pipermail/qgis-developer/2018-February/051797.html

This flag is true by default so that existing providers will not be affected.

Description

Include a few sentences describing the overall goals for this PR (pull request). If applicable also add screenshots.

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]

@rkanavath How does this differ from the current QgsProcessingAlgorithm.FlagHidden flag, which also hides a parameter from the gui?

[rkanavath]

@nyalldawson, FlagHidden is not adding a widget and its associate QLabel(if any). So I cannot add simply hide those parameter(s) later.
According to code here: https://github.com/CS-SI/QGIS/blob/80d6434a4cf30e8255fd97409d7572bf75096b8d/python/plugins/processing/gui/ParametersPanel.py#L105

FlagHidden == NotAdded.

Please see my other post on qgis-developer list about group parameters in OTB.

https://github.com/CS-SI/QGIS/blob/80d6434a4cf30e8255fd97409d7572bf75096b8d/python/plugins/processing/gui/ParametersPanel.py#L122

All widgets are created and whether to hide/show them is based on user input. so setting visibility with a flag is better than add and then remove each time user changes a particular combobox index.

Hope its clear

[alexbruy]

How this will work in case of modeler, batch, scripts? How not visible parameters should be handled in different cases (visible and no value, hidden and value etc)? IMHO we need a QEP first.

[rkanavath]

@alexbruy, it is only used in otb and is default is set to true. does modeler and batch scripts using such a feature now. ? What is the issue exactly here can you explain?

[alexbruy]

Modeler and batch modes applied to any algorithm, including OTB. How not visible parameters will work if I use OTB in batch mode? in models? in conjunction with another algorithms?

[rkanavath]

if it asks for list of parameters then give them what is only visible. Does that work?

[rkanavath]

@alexbruy, can you point the source where list of parameters from an algorithm is taken in those cases?. Is it parametersdefinitions( ) ?
https://github.com/CS-SI/QGIS/blob/80d6434a4cf30e8255fd97409d7572bf75096b8d/src/core/processing/qgsprocessingalgorithm.h#L246

[rkanavath]

Could we have this added only for AlgorithmDialog and not BATCH dialog?
Because batch process is like running some commands by adding one row for an execution.
the commit in this PR have no effect on existing code or batch algorithm. Just that it needs more work. For the time being, could we merge this one and deal with other later?

[nyalldawson]

Ok, so you're wanting the widgets created, but hidden at a later stage? When would they be hidden? Have you got a code example?

[rkanavath]

not a code example, but i think this graphic helps,
https://www.orfeo-toolbox.org/qgis/schema_parameter.png
https://www.orfeo-toolbox.org/qgis/Screenshot_smoothing_anisotropic.png
https://www.orfeo-toolbox.org/qgis/Screenshot_smoothing_gaussian.png
based on combox index change, user is given only those parameters that are valid.
gui does not show mean.radius and gaussian.radius when smoothing type is "anidif"

see the application doc: https://www.orfeo-toolbox.org/CookBook/Applications/app_Smoothing.html

[rkanavath]

a sample descriptor

customwidgetwrapper|-type|Smoothing Type|mean;gaussian;anidif|0|False
QgsProcessingParameterNumber|-type.mean.radius|mean Radius|QgsProcessingParameterNumber.Integer|2|False|None|None
QgsProcessingParameterNumber|-type.gaussian.radius|gaussian Radius|QgsProcessingParameterNumber.Integer|3|False
QgsProcessingParameterNumber|-type.anidif.timestep|Time Step|QgsProcessingParameterNumber.Double|0.125|False
QgsProcessingParameterNumber|-type.anidif.nbiter|Nb Iterations|QgsProcessingParameterNumber.Integer|10|False
QgsProcessingParameterNumber|-type.anidif.conductance|Conductance|QgsProcessingParameterNumber.Integer|1|False
QgsProcessingParameterRasterDestination|-out|Output smoothed image

best clean way is to have some sub-parameter relation defined in descriptor files..

[alexbruy]

Why not make separate tools for different smoothing algorithms? What is the reason to keep them in the single algorithm (except fact that this is how it is implemented in OTB) if anyway user will use only single method at once and all methods have different set of parameters?

[rkanavath]

@alexbruy, not visible parameters are skipped in modeler. Can you check all commits?
Batch processing is still an issue because of way things are. So that is not added. But still OTB can work on STANDARD and MODELER without issues. On BATCH, it shows all parameters and otb can report in case of misuse

[rkanavath]

separate algo for each type of smoothing isn't going to happen. We had talked about this already. The entire issue of mess in maintaining is due this single point. OTB has to split applications for QGIS.

What is going to real issue here other than in batch processing?
Going back old way of maintenance is not for a long term. I can't ask someone to work on this each three months of otb release. Eventually, that will end up in no support of otb in processing.
In future, if we can do something about batch processing that is fine for me. But now, these are the options.

Can you explain why this change would be harmful?

[volaya]

Looks good to me. It's true that, if it make it unnecesary to split algs each time a new version is released, that is a good step ahead.

[nyalldawson]

Sorry - I'm feeling lost here. I still do not see how this differs from the existing flag which prevents creation of the widgets. I thought the intention here was to dynamically show or hide parameters based on other parameter values, isn't it?

If so, how does this pr work? I can't see where you link the parameters to show or hide based on, where that logic is, and where the parameters are dynamically hidden after the dialog is created.

[rkanavath]

If you look at sample descriptor in #6272 (comment) ,
customwidgetwrapper|-type|Smoothing Type|mean;gaussian;anidif|0|False
In provider/algs/otb I uses custom widget wrapper to parse this special line and use
OTBSelectionParameter(output_from_parsed_line).
customwidgetwrapper is a Qcomobox with values "mean, gaussian, anidif"
Here when currentIndexChanged signal, I can do logic specific for OTB.
such as
if "type" parameter has value "mean" (found from current index)
hide other parameters starting with key type.<NOT-mean>.<anything>

Since this particular widget is not part of other widgets in processing, I had to do this way.

But still i need to hide them at start. Because current index changed cannot be triggered automatically once entire widget ui is created. I guess that part is clear to you.

I agree that it could be simplified if there is QgsOTBParameter added among others in python/processing/gui/.
It will still keep otb stuff outside and control of visibility can be reused by other providers if needed.

[nyalldawson]

Do you have a link to the relevant code in your custom widget?

[rkanavath]

Nothing published yet. I am still working on it and testing locally. Can you see if any existing providers in core is affected?.

[rkanavath]

I can only put that code when plugin is ready to be published. And I can't resume work on it until this issue is solved. I am open for a different solution if you can propose something.

But, if you want to see how it works in OTB. Please try this standalone linux installer. It's fairly easy to install. https://www.orfeo-toolbox.org//packages/OTB-6.4.0-Linux64.run

wget https://www.orfeo-toolbox.org/packages/OTB-6.4.0-Linux64.run
./OTB-6.4.0-Linux64.run
./OTB-6.4.0-Linux64/bin/otbgui_Smoothing

There are much complex ones such as otbgui_TrainImagesClassifier

It won't take much time to download and see the example working in action. I already posted screenshot of otbgui applications.

[alexbruy]

If so, how does this pr work? I can't see where you link the parameters to show or hide based on, where that logic is, and where the parameters are dynamically hidden after the dialog is created.

I also want to see code or at least QEP explaining how this will work in models and batch mode. Right now I have an impression that using algorithms with such widgets in models will break them.

[rkanavath]

Okay. it is awful to put a simple PR for small stuff. I cannot go ask sponsors to give more money just because of you can't understand this simple stuff. Let's close this PR and end this discussion.
I am giving up on this part here.

From QEP page it says, "Generally smaller features do not require a QEP unless they can have large knock on effect."
you should atleast beware how custom widget wrapper works!. That's your problem. fix it on your side.

seriously, I did say, it didn't break other algorithms because nobody is using this flag.

[savmickael]

Hi
As technical manager of Rashad at CS SI, I will take the action to push the QEP and try to facilitate the discussion between us and the QGIS team.
Our primary goal is to made an interesting contribution to QGIS to help user to use OTB and its raster algorithms. Currently the status is not good for the OTB providers. I am aware of that as user and also as designer with other at CS SI of the first solution based on splited applications. The maintenance of this solution is not easy and generate some troubles to the OTB users which didn't find the same interface between QGIS applications in Processing and the OTB-Qt or OTB cli one.
Please don't take into account the last message of Rashad which try to do its best.
Hope we can solve smoothly this situation.

[rkanavath]

@savmickael I had tested this on my machine and grass, saga, gdal works well!. I also found that the code doesn't break modeler and scripts for all other algorithm providers.

so the change is okay to be merged from my point of view. Didn't do any testing above that.
@alexbruy if you can help me find where this PR breaks other parts, that would be great. I can then update this PR as per your suggestions.

[rkanavath]

@nyalldawson , @alexbruy
I am skipping the part of, how custom widget wrapper works. Because it is already used in existing processing algorithms. see for example https://github.com/qgis/QGIS/blob/master/python/plugins/processing/algs/gdal/ui/RasterOptionsWidget.py
below code snippet is custom widget wrapper maintained inside plugin tree.

class otbCustomWidgetWrapper(WidgetWrapper):
    def createWidget(self):
        if self.dialogType == DIALOG_BATCH:
            widget = QLineEdit()
            if self.param.defaultValue():
                widget.setText(self.param.defaultValue())
            return widget
        else:
            widget = QComboBox()
            widget.addItems(self.param.options)
            if self.dialogType == DIALOG_MODELER:
              widget.currentIndexChanged.connect(self.valchangedInModeler)
            else:
                widget.currentIndexChanged.connect(self.valchanged)
            return widget

    def valchangedInModeler(self, v):
        pass

    def valchanged(self, v):
        n = self.param.name() + "."
        sel = n + self.value()
        for pa in self.dialog.algorithm().parameterDefinitions():
            pname = pa.name()
            if n in pname:
                flag = False
                if sel in pname:
                    flag = True
                self.dialog.mainWidget().wrappers[pname].widget.setVisible(flag)
                self.dialog.algorithm().parameterDefinition(pname).setVisible(flag)
                if pname in self.dialog.mainWidget().labels:
                    self.dialog.mainWidget().labels[pname].setVisible(flag)

..And when reading descriptor file #6272 (comment) we use custom widget wrapper with a simple check

def defineCharacteristicsFromFile(self):
     .....
     if 'customwidgetwrapper' in line:
                        tokens = line.split("|")
                        params = [t if str(t) != str(None) else None for t in tokens[1:]]
                        options = params[2].split(';')
                        param = otbcustomwidget(params[0], params[1], options, params[3], params[4])

 class otbcustomwidget(QgsProcessingParameterDefinition):
    def __init__(self, name='', description='', options=[], default=None, isSource=False,
                 multiple=False, optional=False):

        QgsProcessingParameterDefinition.__init__(self, name, description, default, optional)

        self.setMetadata({
            'widget_wrapper': {
                'class': 'myselectionwidget.otbCustomWidgetWrapper'}})        
        self.multiple = multiple
        self.options = options

I hope this can clear your confusion and things can be merged.

[nyalldawson]

Thanks for the fruitful discussion last night, and being flexible with defering the technical discussion here.

self.dialog.algorithm().parameterDefinition(pname).setVisible(flag)

Why is this needed though? You've already hidden the widget via

self.dialog.mainWidget().wrappers[pname].widget.setVisible(flag)

so setting the parameter definition itself as invisible doesn't seem to be used anywhere anyway. Also keep in mind that these parameter definitions belong to the algorithm instance - and a new instance of an algorithm is used when it's executed. I.e. instead of changing the parameter definition here, what you actually need to do is change its value, since only the parameter's value (not the definition!) is passed to the algorithm when it's executed.

In other words, the whole value of the parameter must be encapsulated in a QVariant so it can be included in the parameters dictionary passed to the algorithm. Definitions are immutable outside of an algorithm's initAlgorithm method.

[rkanavath]

Thanks for review and comments on code. I agree it is more easy for you if the entire code is attached. I looked at line in question and have some notes. Here is a screenshot of qgis' log messages panel. It seems same by removing line you mentioned.

self.dialog.mainWidget().wrappers[pname].widget.setVisible(flag)

It all looks good upto here and there is no need to update parameterdefinition at this point.
But in the log messages panel, things are bit different.

Below is the output after removing line in question.

Processing algorithm...
Input parameters:
{ '-in' : '/home/rashad/data/qgis/QB_Toulouse_Ortho_XS.tif', '-ram' : 128, '-type' : 'gaussian', '-type.mean.radius' : 2, '-out' : '/tmp/processing_5502db0ba56546e69d91b44825d6d620/803acf76c688417cadff41ba837dd686/-out.tif' }

Execution completed in 0.04 seconds
Results:
{}

Loading resulting layers
Processing algorithm...
Input parameters:
{ '-in' : '/home/rashad/data/qgis/QB_Toulouse_Ortho_XS.tif', '-ram' : 128, '-type' : 'anidif', '-type.mean.radius' : 2, '-out' : '/tmp/processing_5502db0ba56546e69d91b44825d6d620/e1a01b89bb37429d9821332ea21b68a2/-out.tif' }

Execution completed in 0.02 seconds
Results:
{}

Loading resulting layers
Processing algorithm...
Input parameters:
{ '-in' : '/home/rashad/data/qgis/QB_Toulouse_Ortho_XS.tif', '-ram' : 128, '-type' : 'mean', '-type.mean.radius' : 2, '-out' : '/tmp/processing_5502db0ba56546e69d91b44825d6d620/4266e8d6171c417d98c453484d613faf/-out.tif' }

Execution completed in 0.02 seconds
Results:
{}

Loading resulting layers

I guess, the code here is handles which parameters are show in Processing windows's "Log" tab.
https://github.com/qgis/QGIS/pull/6272/files#diff-6cb7ac55ec779f415713126ff31484daL207

After setting,
self.dialog.mainWidget().wrappers[pname].widget.setVisible(flag)
I get the correct output in "Log" tab.

Processing algorithm...
Input parameters:
{ '-in' : '/home/rashad/data/qgis/QB_Toulouse_Ortho_XS.tif', '-ram' : 128, '-type' : 'mean', '-type.mean.radius' : 2, '-out' : '/tmp/processing_cd35522f902445c39f73475a185aa214/baf6c2bba04445e18b480611cf3b1d97/-out.tif' }

Execution completed in 0.03 seconds
Results:
{}

Loading resulting layers
Processing algorithm...
Input parameters:
{ '-in' : '/home/rashad/data/qgis/QB_Toulouse_Ortho_XS.tif', '-ram' : 128, '-type' : 'gaussian', '-type.gaussian.radius' : 3, '-out' : '/tmp/processing_cd35522f902445c39f73475a185aa214/e2ea667f06d748beaf27763c454bde50/-out.tif' }

Execution completed in 0.02 seconds
Results:
{}

Loading resulting layers
Processing algorithm...
Input parameters:
{ '-in' : '/home/rashad/data/qgis/QB_Toulouse_Ortho_XS.tif', '-ram' : 128, '-type' : 'anidif', '-type.anidif.timestep' : 0.125, '-type.anidif.nbiter' : 10, '-type.anidif.conductance' : 1, '-out' : '/tmp/processing_cd35522f902445c39f73475a185aa214/fb3d40a49eed4bc88749a4fe24f38361/-out.tif' }

Execution completed in 0.02 seconds
Results:
{}

Loading resulting layers

Any thoughts?

[nyalldawson]

Any thoughts?

Yes - what you need to do here is ensure that your custom widget returns an NULL value for the parameter if the widget is hidden. Then the existing processing api will handle everything else for you!

What I'm wondering is whether a "preprocessParameters" method in QgsProcessingAlgorithm would be useful to help here. This method could be overridden to automatically remove any parameter values which don't apply in certain circumstances. E.g. here you could have the preprocessor clear the interpolation parameters which don't apply for certain interpolation types.

To me that's a more elegant solution to this use case. What do you think @alexbruy ?

[rkanavath]

I tried to set value to None in indexChanged slot but isn't working as expected.
I like the idea of preprocessParameters which can exclude some parameters.
and the name of parameters can be passed to this method from customwidget wrapper.
This way providers can choose some kind of rule to decide which parameters can be actually used. Also, Can we have some way to force value to none in the widget.setVisible(False) method?

[nyalldawson]

Can we have some way to force value to none in the widget.setVisible(False) method?

I'd do this in the widget wrapper's value() method -- something like:

def value(self):
    if not self.widget().isVisible():
        return None

    ... # normal logic here

[rkanavath]

@nyalldawson , does this evaluates to true always?
because custom widget wrapper here is a QCombobox and its always visible. what it hides are other parameters. Am I missing something?

[rkanavath]

I tried another way to skip them but didn't work.
def getParameterValues(self): method, I tried to set value of parameters to None if widget.isVisible() return False. This given an error on execute algo, "Incorrect parameter value for ". The error comes from QgsProcessingParameterDefinition::checkValueIsAcceptable() method.

[rkanavath]

@nyalldawson , can you discuss more about "preprocessParameters"? I am interested to try this one.

[rkanavath]

@nyalldawson,
Here is the code for plugin in its current state.
https://gitlab.orfeo-toolbox.org/orfeotoolbox/qgis-otb-plugin
Please have a look so that all of us are on same page. Plugin code even though can be installed will not "execute" any otb algorithms. However you can install it without OTB and you can get two algorithms namely Smoothing and TrainImagesClassifier. Plugin can load ui interface, and on execute it simply print a command line that will be executed. There is "processParameters" which I added for now but that part of diff is not included in this PR. we can discuss this once we are through plugin code. Load time of plugin is significantly improved and is hopefully better than other providers.

Hope it helps

[nyalldawson]

thanks @rkanavath -- it's certainly much easier when we can see the code :)

Here's my suggestion:

OTBAlgorithm.py

• Change this:

    if '.' in name and len(name.split('.')) > 2:
                    p = name.split('.')[:-2]
                    group_key = '.'.join(p)
                    group_param = self.parameterDefinition(group_key)
                    default_value = group_param.options[ int(group_param.defaultValue()) ]
                    filtre = group_key + "." + default_value
                    if not filtre in name: param.setVisible(False)

to this:

    if '.' in name and len(name.split('.')) > 2:
                    p = name.split('.')[:-2]
                    group_key = '.'.join(p)
                    group_value = name.split('.')[-2]
                    metadata = param.metadata()
                    metadata['group_key'] = group_key
                    metadata['group_value']  = group_value
                    param.setMetadata(metadata)

Then preprocessParameters becomes:

def preprocessParameters(self, parameters):
        valid_params = {}
        for k,v in parameters.items():
            def = self.parameterDefinition(k)
            if not 'group_key' in def.metadata() or parameters[def.metadata()['group_key']] == def.metadata()['group_value']:
                # valid parameters are those with no group_key dependency, or where the group_key parameter value matches the specified group_value
                valid_params[k] = v
        return valid_params

and at the start of processAlgorithm:

parameters = self.preprocessParameters(parameters)

Lastly, you'll need to update OTBChoiceWidgetWrapper:

• drop valchangedInModeler -- in modeler you should show ALL the parameters for all options, because you won't know in advance what the parent choice is.
• valueChanged becomes something like:

    def valueChanged(self, value):
        for parameter in self.dialog.algorithm().parameterDefinitions():
            if not 'group_key' in parameter.metadata() or parameter.metadata()['group_key'] != self.param.name():
                 continue

            name = parameter.name()
            v = self.value() == parameter.metadata()['group_value']
            self.dialog.mainWidget().wrappers[name].widget.setVisible(v)
            if name in self.dialog.mainWidget().labels:
                self.dialog.mainWidget().labels[name].setVisible(v)

That approach should work with no changes in core/processing - so will work fine in 3.0/3.2 and will be fully compatible with models.

@alexbruy do you agree?

[rkanavath]

thanks for review and also for fix!. it will hide parameters when i change choice and is working upto there. When gui is loaded the it shows all parameters because widgets are set visible in ParametersPanel.py.

[nyalldawson]

When gui is loaded the it shows all parameters because widgets are set visible in ParametersPanel.py.

Ah - you should be able to handle that by implementing postInitialize in OTBChoiceWidgetWrapper, with similar logic:

    def postInitialize(self, wrappers):
        if self.dialogType in (DIALOG_STANDARD, DIALOG_BATCH):
            for parameter in self.dialog.algorithm().parameterDefinitions():
                if not 'group_key' in parameter.metadata() or parameter.metadata()['group_key'] != self.param.name():
                     continue
 
                name = parameter.name()
                v = self.value() == parameter.metadata()['group_value']
                for wrapper in wrappers:
                     if wrapper.param.name() == name:
                          wrapper.widget.setVisible(v)
                if name in self.dialog.mainWidget().labels:
                    self.dialog.mainWidget().labels[name].setVisible(v)

There's a lot of duplicate code here -- it's for demo purposes only and you can probably refactor a lot to clean this up.

[rkanavath]

looking better now. widgets are well hidden at startup. but labels are still showing up :(
I checked self.dialog.mainWidget() and is returning None. Maybe postinitialize needs to be called some time later. Or could we have a postinitialize for algorithm?

it would be nice if wrapper has label, then it be accessible with it.

[nyalldawson]

Maybe check self.widget.parent().labels? (Or self.widget.parent().parent().labels)

[rkanavath]

AttributeError: 'QWidget' object has no attribute 'labels'

[nyalldawson]

Ok, I think you're right - you'll need to add a label member to WidgetWrapper and set it in the panel construction.

[rkanavath]

below diff works for me. There is also a createWidget method in widgetWrapper class.
This can be used to create Qlabel if there is label argument to ctor is not null.
It is a minor detail but allows to manage creation of labels in a more clean way and self.lables maybe removed later.

index 6ec9e9bcd2..e5a27f447b 100644
--- a/python/plugins/processing/gui/ParametersPanel.py
+++ b/python/plugins/processing/gui/ParametersPanel.py
@@ -148,6 +148,7 @@ class ParametersPanel(BASE, WIDGET):
                     else:
                         label = QLabel(desc)
                         # label.setToolTip(tooltip)
+                        wrapper.label = label
                         self.labels[param.name()] = label

                         if param.flags() & QgsProcessingParameterDefinition.FlagAdvanced:
diff --git a/python/plugins/processing/gui/wrappers.py b/python/plugins/processing/gui/wrappers.py
index 476d94c94b..6ba27fe14f 100755
--- a/python/plugins/processing/gui/wrappers.py
+++ b/python/plugins/processing/gui/wrappers.py
@@ -139,10 +139,11 @@ def getExtendedLayerName(layer):
 class WidgetWrapper(QObject):
     widgetValueHasChanged = pyqtSignal(object)

-    def __init__(self, param, dialog, row=0, col=0, **kwargs):
+    def __init__(self, param, dialog, label=None, row=0, col=0, **kwargs):
         QObject.__init__(self)
         self.param = param
         self.dialog = dialog
+        self.label = label
         self.row = row
         self.col = col
         self.dialogType = dialogTypes.get(dialog.__class__.__name__, DIALOG_STANDARD)

[rkanavath]

I have another one for you.

index f1f2ac69bc..1cbf8546bf 100644
--- a/src/core/processing/qgsprocessingalgorithm.cpp
+++ b/src/core/processing/qgsprocessingalgorithm.cpp
@@ -86,15 +86,16 @@ bool QgsProcessingAlgorithm::canExecute( QString * ) const

 bool QgsProcessingAlgorithm::checkParameterValues( const QVariantMap &parameters, QgsProcessingContext &context, QString *message ) const
 {
-  Q_FOREACH ( const QgsProcessingParameterDefinition *def, mParameters )
-  {
-    if ( !def->checkValueIsAcceptable( parameters.value( def->name() ), &context ) )
+  for(auto key : parameters.keys())
     {
-      if ( message )
-        *message = QObject::tr( "Incorrect parameter value for %1" ).arg( def->name() );
-      return false;
+      const QgsProcessingParameterDefinition *def = parameterDefinition(key);
+      if ( !def->checkValueIsAcceptable( parameters.value( key ), &context ) )
+       {
+         if ( message )
+           *message = QObject::tr( "Incorrect parameter value for %1" ).arg( def->name() );
+         return false;
+       }
     }
-  }
   return true;
 }

[nyalldawson]

@rkanavath

checkParameterValues is virtual - so you should overwrite that with your method in OTBAlgorithm

re the label change - can you send that in as a separate PR?

[rkanavath]

yes override checkParameterValues is working.
what about preprocessParameters() ?. Right now it's called only in OTBAlgorithm's processAlgorithm. So execute is working as expected but on Log it shows all parameters. issue wasn't showing up until i cleaned changes in local tree. Maybe adding processparameters() somewhere internally in processing core is needed.

i will update plugin code and new pr soon. But you can see how is looks here:

QgsMessageLog windows "Processing" tab.

cmd=/tmp/OTB-install/otbcli_Smoothing -in "/home/rashad/data/qgis/QB_Toulouse_Ortho_XS.tif" -out "/tmp/processing_396ece5b1a3046309502c6812ec8eb80/f7bb5b697bdf4c188b8d0efa4914ae77/out.tif" -type "mean" -type.mean.radius "2.0"

AlgorithmDialog's "Log" tab:

Processing algorithm...
Algorithm 'Smoothing' starting...
Input parameters:
{ 'in' : '/home/rashad/data/qgis/QB_Toulouse_Ortho_XS.tif', 'type' : 'mean', 'type.mean.radius' : 2, 'type.gaussian.radius' : 3, 'type.anidif.timestep' : 0.125, 'type.anidif.nbiter' : 10, 'type.anidif.conductance' : 1, 'out' : '/tmp/processing_396ece5b1a3046309502c6812ec8eb80/f7bb5b697bdf4c188b8d0efa4914ae77/out.tif' }

Execution completed in 0.11 seconds
Results:
{}

Loading resulting layers
Algorithm 'Smoothing' finished

As you can see actual algorithm execution and log message are inconsistent and hence create confusion or "not-a-bug" reports from users ;)

Here is a diff from AlgoritmDialog.py (missing required parts in core).

--- a/python/plugins/processing/gui/AlgorithmDialog.py
+++ b/python/plugins/processing/gui/AlgorithmDialog.py
@@ -115,7 +115,7 @@ class AlgorithmDialog(QgsProcessingAlgorithmDialogBase):
                 if value:
                     parameters[param.name()] = value

-        return parameters
+        return self.algorithm().preprocessParameters(parameters)

     def checkExtentCRS(self):
         unmatchingCRS = False

Any thoughts?

[rkanavath]

here is new PR for wrapper label #6614

[rkanavath]

@nyalldawson , any feedback on preprocessParameters method?

[nyalldawson]

@rkanavath

any feedback on preprocessParameters method?

I just started to do this - but looking more into it it's not straightforward.

The issue is in where we call this -- if it's called as part of an algorithm execution (e.g. inside processAlgorithm() ), or in code paths prior to executing an algorithm (e.g. inside AlgorithmDialog.getParameterValues ). If we call it from both locations then there's potentially an issue if an algorithms preprocessing involves altering values ... say.... some type of string escaping, because we'll be preprocessing the same set of parameters twice before executing the alg (once by the dialog, once by the algorithm itself).

So I think we need to choose just one of these options. My vote would go to preprocessing outside of the algorithm execution only - so in AlgorithmDialog.

Does this work for you?

[rkanavath]

Yes. change in AlgorithmDialog works for me. with that, I don't need to do call preprocessParameters inside processAlgorithm(). you can go on with option you vote ;)

[nyalldawson]

@rkanavath ok, done in #6658

[rkanavath]

#6658 and #6614 + fixes from @nyalldawson on plugin code closes this PR. Thanks to all