qgis / QGIS-Enhancement-Proposals

QEP's (QGIS Enhancement Proposals) are used in the process of creating and discussing new enhancements for QGIS
118 stars 37 forks source link

QEP 84: Processing ModelerParametersDialog widget wrapper #84

Open arnaud-morvan opened 7 years ago

arnaud-morvan commented 7 years ago

QGIS Enhancement 84: Processing ModelerParametersDialog widget wrapper

Date 2016/12/13

Author Arnaud Morvan (@arnaud-morvan)

Contact arnaud dot morvan at camptocamp dot com

maintainer @arnaud-morvan

Version QGIS 3.0

Summary

For now, we have created widget wrappers to extract parameter type specific code from ParametersPanel, BatchDialog and ModelerParameterDialog. But it remains a lot of specific code for modeler in these widget wrappers.

The idea here is to factorize the part specific to modeler (model inputs selection in modeler algorithm parameter dialog).

Proposed Solution

My proposition is to create a modeler specific widget wrapper for modeler algorithm parameters dialog (the dialog that show up when you want to configure an algorithm inside a model). This new wrapper will be responsible to wrap the standard parameter widget with a model input selector.

Note that this ModelerWidgetWrapper class will have globally the same interface as standard widgets (createWidget, setValue and value methods), plus an option, in createWidget method, to give the compatible model inputs we want to propose to the user.

This will allow us to remove near to all the modeler specific part of code from standard widget wrappers, and will allow the user to use all the power of standard widgets from the algorithm dialog in the modeler algorithm dialog.

Example(s)

Here is an example code for the modeler widget wrapper that will be used by the ModelerParameterDialog exactly same way as standard wrapper by the ParametersPanel.

class ModelerWidgetWrapper(QWidget):

    def __init__(self, wrapper, model_values):
        super(ModelerWidgetWrapper, self).__init__()

        self.wrapper = wrapper
        self.widget = wrapper.widget
        self.implemented = wrapper.implemented
        self.model_values = model_values

        menu = QMenu()
        fixed_value_action = QAction(self.tr('Fixed value'), menu)
        fixed_value_action.triggered.connect(self.on_fixedValue)
        menu.addAction(fixed_value_action)
        menu.addSeparator()
        for text, value in model_values:
            model_value_action = QAction(text, menu)
            model_value_action.setData(value)
            model_value_action.triggered.connect(self.on_modelValue)
            menu.addAction(model_value_action)

        self.mIconDataDefine = QgsApplication.getThemeIcon("/mIconDataDefine.svg")
        self.mIconDataDefineOn = QgsApplication.getThemeIcon("/mIconDataDefineOn.svg")

        button = QToolButton()
        button.setIcon(self.mIconDataDefine)
        button.setPopupMode(QToolButton.InstantPopup)
        button.setMenu(menu)
        self.button = button

        label = QLabel()
        label.hide()
        self.label = label

        layout = QHBoxLayout()
        layout.addWidget(button, 0)
        layout.addWidget(label, 1)
        layout.addWidget(wrapper.widget, 1)
        self.setLayout(layout)

    def on_fixedValue(self):
        self.button.setIcon(self.mIconDataDefine)
        self.label.hide()
        self.wrapper.widget.show()

    def on_modelValue(self):
        action = self.sender()
        self.setValue(action.data())

    def setValue(self, value):
        for text, val in self.model_values:
            if val == value:
                self.model_value = value
                self.button.setIcon(self.mIconDataDefineOn)
                self.label.setText(text)
                self.label.show()
                self.wrapper.widget.hide()
                return
        self.wrapper.setValue(value)
        self.on_fixedValue()

    def value(self):
        if self.label.isVisible():
            return self.model_value
        else:
            return self.wrapper.value()

Here is the result on screen, with a drop down menu to select compatible model inputs:

a2cfaf64-7157-11e6-976c-d6b518cbd94a

Note that the UI appearance can be changed, for example we could use radio buttons (fixed values / model inputs) with a combo box for inputs, and this could be done easily transversally for all the parameter types.

IMHO it would be clearer for the user if the model values (model inputs and other algorithm outputs) are listed away from other values.

Affected Files

processing.gui.wrappers.py processing.modeler.ModelerParametersDialog.py

Performance Implications

Should have no effect on performance.

Further Considerations/Improvements

Backwards Compatibility

Full compatible with existing models, just an UI change.

Issue Tracking ID(s)

(optional)

Votes

(required)

arnaud-morvan commented 7 years ago

@volaya @alexbruy @nyalldawson @rldhont : How do you fell this ?

volaya commented 7 years ago

The idea looks good to me. We will have to see how that integrates with algorithms that have specific parameters and behaviour in the modeler, like the raster calculator.

I guess there will be no issues, so this sounds like a nice enhancement

arnaud-morvan commented 7 years ago

@nirvn : Any opinion on this ?

nyalldawson commented 7 years ago

@arnaud-morvan

https://github.com/qgis/QGIS/pull/4729 relates to this. In that PR the way model child algorithm parameter values are handled in the base model algorithm classes are changed. Instead of separate classes used to handle ValueFromInput/ValueFromOutput/etc, there's a single class QgsProcessingModelAlgorithm::ChildParameterSource which handles all possible source types for input parameters to child algorithms. The actual behaviour is specified via an enum (StaticValue /ModelParameter /ChildOutput ).

This would tie in very well with the proposal outlined here. We'd just make sure the the model widget wrapper checks the ChildParameterSource source type and shows the appropriate configuration widget, and when it returns the value it returns a properly created ChildParameterSource object.

The changes in that PR + the changes outlined here will make it easy to extend in future with other possible child algorithm parameter input types, such as a "calculate expression result before running child alg" type.

My 2c is that I'm very much in favour of this proposal, and think that separating model inputs from the static value choices from other child algorithm outputs will make processing's UI easier to understand. My only concern is the reuse of the data defined icon in a different context - that should be avoided and either a combobox used OR a new icon for this designed. It'll get very confusing when I add the data defined button to dynamic parameter widgets (it's now supported in the code, just not exposed in the UI yet!).

nyalldawson commented 7 years ago

The changes in that PR + the changes outlined here will make it easy to extend in future with other possible child algorithm parameter input types, such as a "calculate expression result before running child alg" type.

This is implemented in https://github.com/qgis/QGIS/pull/4812