Browse Source

Merge pull request #214 from datalad/bf-212

Loads of changes re parameter widgets
Michael Hanke 2 years ago
parent
commit
5f6ebbbf3b

+ 3 - 0
datalad_gooey/datalad_ui.py

@@ -105,6 +105,9 @@ class DataladQtUIBridge(QObject):
                 # for those
                 if t
             ]
+        if not progress:
+            # we have ignore a progress tracker and now we have nothing
+            return
         progress = sum(progress) / len(progress)
         # default range setup is 0..100
         self._progress_bar.setValue(progress * 100)

+ 7 - 7
datalad_gooey/dataladcmd_ui.py

@@ -79,13 +79,13 @@ class GooeyDataladCmdUI(QObject):
                 api = sender.data().get('__api__')
             if cmdname is None:
                 cmdname = sender.data().get('__cmd_name__')
-                # pull in any signal-provided kwargs for the command
-                # unless they have been also specified directly to the method
-                cmdkwargs = {
-                    k: v for k, v in sender.data().items()
-                    if k not in ('__cmd_name__', '__api')
-                    and k not in cmdkwargs
-                }
+            # pull in any signal-provided kwargs for the command
+            # unless they have been also specified directly to the method
+            cmdkwargs = {
+                k: v for k, v in sender.data().items()
+                if k not in ('__cmd_name__', '__api__')
+                and k not in cmdkwargs
+            }
 
         assert cmdname is not None, \
             "GooeyDataladCmdUI.configure() called without command name"

+ 14 - 6
datalad_gooey/param_form_utils.py

@@ -93,6 +93,7 @@ def populate_form_w_params(
                 param.cmd_kwargs.get('default', _NoValue), \
                 param,
             )
+    cmdkwargs_defaults = dict()
     for pname, pdefault, param_spec in sorted(
             # across cmd params, and generic params
             chain(_specific_params(), _generic_params()),
@@ -110,6 +111,7 @@ def populate_form_w_params(
             # command knows
             param_spec.constraints = \
                 cmd_api_spec['parameter_constraints'][pname]
+        cmdkwargs_defaults[pname] = pdefault
         # populate the layout with widgets for each of them
         # we do not pass Parameter instances further down, but disassemble
         # and homogenize here
@@ -153,11 +155,14 @@ def populate_form_w_params(
             if pname1 == pname2:
                 continue
             pwidget1.value_changed.connect(
-                pwidget2.init_gooey_from_other_param)
+                pwidget2.init_gooey_from_params)
     # when all is wired up, set the values that need setting
+    # we set the respective default value to all widgets, and
+    # update it with the given value, if there was any
+    # (the true command parameter default was already set above)
+    cmdkwargs_defaults.update(cmdkwargs)
     for pname, pwidget in form_widgets.items():
-        if pname in cmdkwargs:
-            pwidget.set_gooey_param_value(cmdkwargs[pname])
+        pwidget.init_gooey_from_params(cmdkwargs_defaults)
 
 
 #
@@ -212,6 +217,8 @@ def _get_parameter_widget_factory(
     if argparse_spec is None:
         argparse_spec = {}
     argparse_action = argparse_spec.get('action')
+    disable_manual_path_input = active_suite.get('options', {}).get(
+        'disable_manual_path_input', False)
     # if we have no idea, use a simple line edit
     type_widget = pw.StrParamWidget
     # now some parameters where we can derive semantics from their name
@@ -219,12 +226,13 @@ def _get_parameter_widget_factory(
         type_widget = functools.partial(
             pw.PathParamWidget,
             pathtype=QFileDialog.Directory,
-            disable_manual_edit=active_suite.get('options', {}).get(
-                'disable_manual_path_input', False),
+            disable_manual_edit=disable_manual_path_input,
             basedir=basedir)
     elif name == 'path':
         type_widget = functools.partial(
-            pw.PathParamWidget, basedir=basedir)
+            pw.PathParamWidget,
+            disable_manual_edit=disable_manual_path_input,
+            basedir=basedir)
     elif name == 'cfg_proc':
         type_widget = pw.CfgProcParamWidget
     elif name == 'recursion_limit':

+ 131 - 141
datalad_gooey/param_multival_widget.py

@@ -1,193 +1,183 @@
-from PySide6.QtCore import (
-    QAbstractItemModel,
-    QModelIndex,
-    Qt,
-)
+from PySide6.QtCore import Qt
 from PySide6.QtWidgets import (
     QWidget,
     QListWidget,
     QListWidgetItem,
     QVBoxLayout,
     QHBoxLayout,
-    QStyledItemDelegate,
-    QStyleOptionViewItem,
     QPushButton,
 )
 
 from datalad.utils import ensure_list
 from .param_widgets import (
     GooeyParamWidgetMixin,
-    load_parameter_widget,
 )
 from .utils import _NoValue
 
 
-class MyItemDelegate(QStyledItemDelegate):
-    def __init__(self, mviw, *args, **kwargs):
-        super().__init__(*args, **kwargs)
-        self._mviw = mviw
-
-    # called on edit request
-    def createEditor(self,
-                     parent: QWidget,
-                     option: QStyleOptionViewItem,
-                     index: QModelIndex) -> QWidget:
-        mviw = self._mviw
-        wid = load_parameter_widget(
-            parent,
-            mviw._editor_factory,
-            name=mviw._gooey_param_name,
-            docs=mviw._editor_param_docs,
-            default=getattr(mviw, 'editor_param_default', _NoValue),
-            #validator=mviw._editor_validator,
-        )
-        value = getattr(mviw, 'editor_param_value', _NoValue)
-        if value != _NoValue:
-            wid.set_gooey_param_value(value)
-        wid.init_gooey_from_other_param(mviw._editor_init)
-        # we draw on top of the selected item, and having the highlighting
-        # "shine" through is not nice
-        wid.setAutoFillBackground(True)
-        return wid
-
-    # called after createEditor
-    def updateEditorGeometry(self,
-                             editor: QWidget,
-                             option: QStyleOptionViewItem,
-                             index: QModelIndex) -> None:
-        # force the editor widget into the item rectangle
-        editor.setGeometry(option.rect)
-
-    # called after updateEditorGeometry
-    def setEditorData(self, editor: QWidget, index: QModelIndex):
-        # this requires the inverse of the already existing
-        # _get_datalad_param_spec() "retriever" methods
-        data = index.data()
-        if data is not _NoValue:
-            editor.set_gooey_param_value(data)
-
-    # called after editing is done
-    def setModelData(self,
-                     editor:
-                     QWidget,
-                     model: QAbstractItemModel,
-                     index: QModelIndex):
-        # this could call the already existing _get_datalad_param_spec()
-        # "retriever" methods
-        got_value = False
-        try:
-            val = editor.get_gooey_param_value()
-            got_value = True
-        except ValueError:
-            # input widget saw no actual input
-            pass
-        if got_value:
-            #  TODO other role than Qt.EditRole?
-            model.setData(index, val)
-
-
 class MultiValueInputWidget(QWidget, GooeyParamWidgetMixin):
+    NativeObjectRole = Qt.UserRole + 233
+
     def __init__(self, editor_factory, *args, **kwargs):
         super().__init__(*args, **kwargs)
-        self._editor_factory = editor_factory
-        # maintained via init_gooey_from_other_param()
-        self._editor_init = dict()
 
+        # main layout
         layout = QVBoxLayout()
         # tight layout
         layout.setContentsMargins(0, 0, 0, 0)
         self.setLayout(layout)
-        # the main list for inputting multiple values
-        self._lw = QListWidget()
-        self._lw.setAlternatingRowColors(True)
-        # we assing the editor factory
-        self._lw.setItemDelegate(MyItemDelegate(self))
-        self._lw.setToolTip('Double-click to edit items')
-        layout.addWidget(self._lw)
-
-        # now the buttons
-        additem_button = QPushButton('+')
-        additem_button.clicked.connect(self._add_item)
-        removeitem_button = QPushButton('-')
-        removeitem_button.clicked.connect(self._remove_item)
-        button_layout = QHBoxLayout()
-        button_layout.addWidget(additem_button)
-        button_layout.addWidget(removeitem_button)
-        layout.addLayout(button_layout, 1)
-
-        self._additem_button = additem_button
-        self._removeitem_button = removeitem_button
 
         # define initial widget state
-        # empty by default, nothing to remove
-        removeitem_button.setDisabled(True)
         # with no item present, we can hide everything other than
         # the add button to save on space
-        removeitem_button.hide()
-        self._lw.hide()
 
-    def _add_item(self) -> QListWidgetItem:
+        # key component is a persistent editor
+        editor = editor_factory(parent=self)
+        self._editor = editor
+        layout.addWidget(editor)
+
+        # underneath the buttons
+        pb_layout = QHBoxLayout()
+        layout.addLayout(pb_layout, 0)
+        for name, label, callback in (
+                ('_add_pb', 'Add', self._add_item),
+                ('_update_pb', 'Update', self._update_item),
+                ('_del_pb', 'Remove', self._del_item),
+        ):
+            pb = QPushButton(label)
+            pb.clicked.connect(callback)
+            pb_layout.addWidget(pb)
+            setattr(self, name, pb)
+            if name != '_add_pb':
+                pb.setDisabled(True)
+
+        # the main list for inputting multiple values
+        lw = QListWidget()
+        lw.setAlternatingRowColors(True)
+        lw.itemChanged.connect(self._load_item_in_editor)
+        lw.itemSelectionChanged.connect(self._reconfigure_for_selection)
+        layout.addWidget(lw)
+        lw.hide()
+        self._lw = lw
+
+    def _reconfigure_for_selection(self):
+        items = self._lw.selectedItems()
+        n_items = len(items)
+        assert n_items < 2
+        if not n_items:
+            # nothing selected
+            self._update_pb.setDisabled(True)
+            self._del_pb.setDisabled(True)
+        else:
+            # we verify that there is only one item
+            self._load_item_in_editor(items[0])
+            self._update_pb.setEnabled(True)
+
+    def _load_item_in_editor(self, item):
+        self._editor.init_gooey_from_params({
+            self._editor._gooey_param_name:
+                item.data(
+                    MultiValueInputWidget.NativeObjectRole)
+        })
+
+    # * to avoid Qt passing unexpected stuff from signals
+    def _add_item(self, *, data=_NoValue) -> QListWidgetItem:
         newitem = QListWidgetItem(
             # must give custom type
             type=QListWidgetItem.UserType + 234,
         )
-        # TODO if a value is given, we do not want it to be editable
-        newitem.setFlags(newitem.flags() | Qt.ItemIsEditable)
-        # give it a special value if nothing is set
-        # this helps to populate the edit widget with existing
-        # values, or not
-        newitem.setData(Qt.EditRole, _NoValue)
-
+        self._update_item(item=newitem, data=data)
         # put in list
         self._lw.addItem(newitem)
         self._lw.setCurrentItem(newitem)
-        # edit mode, right away TODO unless value specified
-        self._lw.editItem(newitem)
-        self._removeitem_button.setDisabled(False)
-        self._removeitem_button.show()
+        self._del_pb.setDisabled(False)
+        self._del_pb.show()
         self._lw.show()
-
         return newitem
 
-    def _remove_item(self):
+    def _update_item(self, *, item=None, data=_NoValue):
+        if item is None:
+            item = self._lw.selectedItems()
+            assert len(item)
+            item = item[0]
+        if data is _NoValue:
+            print(self._editor.get_gooey_param_spec())
+            # TODO avoid the need to use the name
+            data = self._editor.get_gooey_param_spec().get(
+                self._gooey_param_name, _NoValue)
+        # give it a special value if nothing is set
+        # this helps to populate the edit widget with existing
+        # values, or not
+        item.setData(
+            MultiValueInputWidget.NativeObjectRole,
+            data)
+        item.setData(Qt.DisplayRole, _get_item_display(data))
+
+    def _del_item(self):
         for i in self._lw.selectedItems():
             self._lw.takeItem(self._lw.row(i))
         if not self._lw.count():
-            self._removeitem_button.setDisabled(True)
-            self._removeitem_button.hide()
+            self._del_pb.setDisabled(True)
             self._lw.hide()
+            self._set_gooey_param_value(_NoValue)
 
-    def _set_gooey_param_value(self, value):
+    def _set_gooey_param_value_in_widget(self, value):
+        # tabula rasa first, otherwise this would all be
+        # incremental
+        self._lw.clear()
         # we want to support multi-value setting
         for val in ensure_list(value):
-            item = self._add_item()
-            # TODO another place where we need to think about the underlying
-            # role specification
-            item.setData(Qt.EditRole, val)
-
-    def set_gooey_param_default(self, value):
-        self._editor_param_default = value
-
-    def get_gooey_param_value(self):
-        if not self._lw.count():
-            # do not report an empty list, when no items have been added.
+            self._add_item(data=val)
+
+    def _handle_input(self):
+        val = []
+        if self._lw.count():
+            for row in range(self._lw.count()):
+                item = self._lw.item(row)
+                val.append(item.data(
+                    MultiValueInputWidget.NativeObjectRole))
+            val = [v for v in val if v is not _NoValue]
+        if not val:
+            # do not report an empty list, when no valid items exist.
             # setting a value, even by API would have added one
-            raise ValueError("No items added")
+            val = _NoValue
+        self._set_gooey_param_value(val)
 
-        return [
-            # TODO consider using a different role, here and in setModelData()
-            self._lw.item(row).data(Qt.EditRole)
-            for row in range(self._lw.count())
-        ]
+    def set_gooey_param_spec(self, name: str, default=_NoValue):
+        super().set_gooey_param_spec(name, default)
+        self._editor.set_gooey_param_spec(name, default)
 
     def set_gooey_param_docs(self, docs: str) -> None:
         self._editor_param_docs = docs
         # the "+" button is always visible. Use it to make the docs accessible
-        self._additem_button.setToolTip(docs)
-
-    def init_gooey_from_other_param(self, spec):
-        # we just keep the union of all reported changes, i.e.
-        # the latest info for all parameters that ever changed.
-        # this is then passed to the editor widget, after its
-        # creation
-        self._editor_init.update(spec)
+        self._add_pb.setToolTip(docs)
+
+    def init_gooey_from_params(self, spec):
+        # first the normal handling
+        super().init_gooey_from_params(spec)
+        # for the editor widget, we just keep the union of all reported
+        # changes, i.e.  the latest info for all parameters that ever changed.
+        # this is then passed to the editor widget, after its creation
+        self._editor.init_gooey_from_params(spec)
+
+    def get_gooey_param_spec(self):
+        self._handle_input()
+        # we must override, because we need to handle the cases of list vs
+        # plain item in default settings.
+        # TODO This likely needs more work and awareness of `nargs`, see
+        # https://github.com/datalad/datalad-gooey/issues/212#issuecomment-1256950251
+        # https://github.com/datalad/datalad-gooey/issues/212#issuecomment-1257170208
+        val = self._gooey_param_value
+        default = self._gooey_param_default
+        if val == default:
+            val = _NoValue
+        elif val == [default]:
+            val = _NoValue
+        return {self._gooey_param_name: val}
+
+
+def _get_item_display(value) -> str:
+    if value is _NoValue:
+        return '--not set--'
+    else:
+        return str(value)

+ 182 - 178
datalad_gooey/param_widgets.py

@@ -19,6 +19,7 @@ from PySide6.QtWidgets import (
     QSpinBox,
     QToolButton,
     QWidget,
+    QMessageBox,
 )
 
 from .resource_provider import gooey_resources
@@ -28,63 +29,21 @@ from .utils import _NoValue
 class GooeyParamWidgetMixin:
     """API mixin for QWidget to get/set parameter specifications
 
-    Any parameter widget implementation should also derive from the class,
-    and implement, at minimum, `_set_gooey_param_value()` and
-    `get_gooey_param_value()` for compatibility with the command parameter
-    UI generator.
+    Any parameter widget implementation should also derive from this class,
+    and implement, at minimum, `_set_gooey_param_value_in_widget()` and
+    wire the input widget up in a way that `_set_gooey_param_value()`
+    receives any value a user entered in the widget.
 
-    The main API used by the GUI generator are `set_gooey_param_spec()`
-    and `get_gooey_param_spec()`. They take care of providing a standard
-    widget behavior across all widget types, such as, not returning values if
-    they do not deviate from the default.
+    The main API used by the GUI generator are `set_gooey_param_spec()`,
+    `get_gooey_param_spec()`, and `init_gooey_from_params()`.  They take care
+    of providing a standard widget behavior across all widget types, such as,
+    not returning values if they do not deviate from the default.
     """
 
     value_changed = Signal(MappingProxyType)
     """Signal to be emitted whenever a parameter value is changed. The signal
     payload type matches the return value of `get_gooey_param_spec()`"""
 
-    def set_gooey_param_value(self, value):
-        """Implement to set a particular value in the target widget.
-
-        The `value_changed` signal is emitted a the given value differs
-        from the current value.
-        """
-        # what we had
-        try:
-            prev = self.get_gooey_param_value()
-        except ValueError:
-            prev = _NoValue
-        # let widget implementation actually set the value
-        self._set_gooey_param_value(value)
-        if prev != value:
-            # an actual change, emit corresponding signal
-            self.emit_gooey_value_changed()
-
-    def _set_gooey_param_value(self, value):
-        """Implement to set a particular value in the target widget.
-
-        By default, this method is also used to set a default value.
-        If that is not desirable for a particular widget type,
-        override `set_gooey_param_default()`.
-        """
-        raise NotImplementedError
-
-    def get_gooey_param_value(self):
-        """Implement to get the parameter value from the widget.
-
-        Raises
-        ------
-        ValueError
-          The implementation must raise this exception, when no value
-          has been entered/is available.
-        """
-        raise NotImplementedError
-
-    def set_gooey_param_default(self, value):
-        """Implement to set a parameter default value in the widget
-        """
-        pass
-
     def set_gooey_param_spec(
             self, name: str, default=_NoValue):
         """Called by the command UI generator to set parameter
@@ -93,25 +52,86 @@ class GooeyParamWidgetMixin:
         self._gooey_param_name = name
         # always store here for later inspection by get_gooey_param_spec()
         self._gooey_param_default = default
-        self.set_gooey_param_default(default)
+        self._gooey_param_value = _NoValue
+        self._gooey_param_prev_value = _NoValue
+
+    def init_gooey_from_params(self, spec: Dict) -> None:
+        """Slot to receive changes of parameter values (self or other)
+
+        There can be parameter value reports for multiple parameters
+        at once.
+
+        The default implementation calls a widgets implementation of
+        self._init_gooey_from_other_params(), followed by
+        self.set_gooey_param_value() with any value of a key that matches the
+        parameter name, and afterwards call
+        self._set_gooey_param_value_in_widget() to reflect the value change in
+        the widget.
+
+        If a widget prefers or needs to handle updates differently, this
+        method can be reimplemented. Any reimplementation must call
+        self._set_gooey_param_value() though.
+
+        Parameters
+        ----------
+        spec: dict
+          Mapping of parameter names to new values, in the same format
+          and semantics as the return value of get_gooey_param_spec().
+        """
+        # first let a widget reconfigure itself, before a value is set
+        self._init_gooey_from_other_params(spec)
+        if self._gooey_param_name in spec:
+            val = spec[self._gooey_param_name]
+            self._set_gooey_param_value(val)
+            # let widget implementation actually set the value
+            self._set_gooey_param_value_in_widget(val)
 
     def get_gooey_param_spec(self) -> Dict:
         """Called by the command UI generator to get a parameter specification
 
-        Return a mapping of the parameter name to the gathered value or _NoValue
-        when no value was gathered, or the gather value is not different from
-        the default)
-        is a mapping of parameter name to the gather value.
+        Return a mapping of the parameter name to the gathered value or
+        _NoValue when no value was gathered, or the gather value is not
+        different from the default)
         """
-        try:
-            val = self.get_gooey_param_value()
-        except ValueError:
-            # class signals that no value was set
-            return {self._gooey_param_name: _NoValue}
+        val = self._gooey_param_value
         return {self._gooey_param_name: val} \
             if val != self._gooey_param_default \
             else {self._gooey_param_name: _NoValue}
 
+    def emit_gooey_value_changed(self):
+        """Slot to connect "changed" signals of underlying input widgets too
+
+        It emits the standard Gooey `value_changed` signal with the
+        current Gooey `param_spec` as value.
+        """
+        self.value_changed.emit(MappingProxyType(self.get_gooey_param_spec()))
+
+    def _set_gooey_param_value(self, value):
+        """Set a particular value in the widget.
+
+        The `value_changed` signal is emitted a the given value differs
+        from the current value.
+
+        The actual value setting in the widget is performed by
+        _set_gooey_param_value_in_widget() which must be implemented for each
+        widget type.
+        """
+        # what we had
+        self._gooey_param_prev_value = self._gooey_param_value
+        # what we have now
+        self._gooey_param_value = value
+
+        if self._gooey_param_prev_value != value:
+            # an actual change, emit corresponding signal
+            self.emit_gooey_value_changed()
+
+    def _set_gooey_param_value_in_widget(self, value):
+        """Implement to set a particular value in the target widget.
+
+        Any implementation must be able to handle `_NoValue`
+        """
+        raise NotImplementedError
+
     def set_gooey_param_validator(self, validator: Callable) -> None:
         """Set a validator callable that can be used by the widget
         for input validation
@@ -128,29 +148,15 @@ class GooeyParamWidgetMixin:
         # having to integrate potentially lengthy text into the layout
         self.setToolTip(docs)
 
-    def init_gooey_from_other_param(self, spec: Dict) -> None:
-        """Slot to receive changes of other parameter values
+    def _init_gooey_from_other_params(self, spec: Dict) -> None:
+        """Implement to init based on other parameter's values
 
-        Can be implemented to act on context changes that require a
+        Can be reimplemented to act on context changes that require a
         reinitialization of a widget. For example, update a list
         of remotes after changing the reference dataset.
-
-        Parameters
-        ----------
-        spec: dict
-          Mapping of parameter names to new values, in the same format
-          and semantics as the return value of get_gooey_param_spec().
         """
         pass
 
-    def emit_gooey_value_changed(self):
-        """Slot to connect "changed" signals of underlying input widgets too
-
-        It emits the standard Gooey `value_changed` signal with the
-        current Gooey `param_spec` as value.
-        """
-        self.value_changed.emit(MappingProxyType(self.get_gooey_param_spec()))
-
 
 def load_parameter_widget(
         parent: QWidget,
@@ -189,26 +195,19 @@ class ChoiceParamWidget(QComboBox, GooeyParamWidgetMixin):
             # avoid making the impression something could be selected
             self.setPlaceholderText('No known choices')
             self.setDisabled(True)
+        self.currentIndexChanged.connect(self._handle_input)
+
+    def _set_gooey_param_value_in_widget(self, value):
+        self.setCurrentText(self._gooey_map_val2label(value))
+
+    def _handle_input(self):
+        self._set_gooey_param_value(self.currentData())
 
     def _add_item(self, value) -> None:
         # we add items, and we stick their real values in too
         # to avoid tricky conversion via str
         self.addItem(self._gooey_map_val2label(value), userData=value)
 
-    def _set_gooey_param_value(self, value):
-        self.setCurrentText(self._gooey_map_val2label(value))
-
-    def get_gooey_param_value(self):
-        return self.currentData()
-
-    def set_gooey_param_spec(self, name: str, default=_NoValue):
-        super().set_gooey_param_spec(name, default)
-        # QComboBox will report the first choice to be selected by default.
-        # Set the specified default instead.
-        if default is _NoValue:
-            default = None
-        self._set_gooey_param_value(default)
-
     def _gooey_map_val2label(self, val):
         return '--none--' if val is None else str(val)
 
@@ -228,77 +227,64 @@ class PosIntParamWidget(QSpinBox, GooeyParamWidgetMixin):
             #self.setMaximum(sys.maxsize)
             pass
         self._allow_none = allow_none
+        self.valueChanged.connect(self._handle_input)
 
-    def _set_gooey_param_value(self, value):
+    def _set_gooey_param_value_in_widget(self, value):
         # generally assumed to be int and fit in the range
         self.setValue(-1 if value is None and self._allow_none else value)
 
-    def set_gooey_param_spec(self, name: str, default=_NoValue):
-        # QSpinBox' values is set to 0 by default. Hence, we need to overwrite
-        # here.
-        super().set_gooey_param_spec(name, default)
-        if default is _NoValue:
-            default = None
-        self.setValue(default if default is not None else -1)
-
-    def get_gooey_param_value(self):
+    def _handle_input(self):
         val = self.value()
         # convert special value -1 back to None
-        return None if val == -1 and self._allow_none else val
+        self._set_gooey_param_value(
+            None if val == -1 and self._allow_none else val
+        )
 
 
 class BoolParamWidget(QCheckBox, GooeyParamWidgetMixin):
 
-    def __init__(self, parent=None) -> None:
+    def __init__(self, allow_none=False, parent=None) -> None:
         super().__init__(parent)
-        # set to no value initially
-        self._set_gooey_param_value(None)
+        if allow_none:
+            self.setTristate(True)
+        self.stateChanged.connect(self._handle_input)
 
-    def _set_gooey_param_value(self, value):
+    def _set_gooey_param_value_in_widget(self, value):
         if value not in (True, False):
             # if the value is not representable by a checkbox
             # leave it in "partiallychecked". In cases where the
             # default is something like `None`, we can distinguish
             # a user not having set anything different from the default,
             # even if the default is not a bool
-            self.setTristate(True)
             self.setCheckState(Qt.PartiallyChecked)
         else:
             # otherwise flip the switch accordingly
             self.setChecked(value)
 
-    def get_gooey_param_value(self):
+    def _handle_input(self):
         state = self.checkState()
-        if state == Qt.PartiallyChecked:
-            # TODO error if partiallychecked still (means a
-            # value with no default was not set)
-            # a default `validator` could handle that
-            # Mixin picks this up and communicates: nothing was set
-            raise ValueError
-        # convert to bool
-        return state == Qt.Checked
-
-    def set_gooey_param_spec(self, name: str, default=_NoValue):
-        super().set_gooey_param_spec(name, default)
-        self._set_gooey_param_value(default)
+        # convert to bool/None
+        self._set_gooey_param_value(
+            None if state == Qt.PartiallyChecked
+            else state == Qt.Checked
+        )
 
 
 class StrParamWidget(QLineEdit, GooeyParamWidgetMixin):
-    def _set_gooey_param_value(self, value):
-        self.setText(str(value))
-        self.setModified(True)
+    def __init__(self, parent=None):
+        super().__init__(parent)
+        self.setPlaceholderText('Not set')
+        self.textChanged.connect(self._handle_input)
 
-    def set_gooey_param_default(self, value):
-        if value != _NoValue:
-            self.setPlaceholderText(str(value))
+    def _set_gooey_param_value_in_widget(self, value):
+        if value in (_NoValue, None):
+            # we treat both as "unset"
+            self.clear()
+        else:
+            self.setText(str(value))
 
-    def get_gooey_param_value(self):
-        # return the value if it was set be the caller, or modified
-        # by the user -- otherwise stay silent and let the command
-        # use its default
-        if self.isEnabled() and not self.isModified():
-            raise ValueError('Present value was not modified')
-        return self.text()
+    def _handle_input(self):
+        self._set_gooey_param_value(self.text())
 
 
 class PathParamWidget(QWidget, GooeyParamWidgetMixin):
@@ -330,8 +316,10 @@ class PathParamWidget(QWidget, GooeyParamWidgetMixin):
             # to avoid confusions re interpretation of relative paths
             # https://github.com/datalad/datalad-gooey/issues/106
             self._edit.setDisabled(True)
+        self._edit.setPlaceholderText('Not set')
+        self._edit.textChanged.connect(self._handle_input)
+        self._edit.textEdited.connect(self._handle_input)
         hl.addWidget(self._edit)
-        self._edit.editingFinished.connect(self.emit_gooey_value_changed)
 
         # next to the line edit, we place to small button to facilitate
         # selection of file/directory paths by a browser dialog.
@@ -363,24 +351,16 @@ class PathParamWidget(QWidget, GooeyParamWidgetMixin):
             dir_button.clicked.connect(
                 lambda: self._select_path(dirs_only=True))
 
-    def _set_gooey_param_value(self, value):
-        self._edit.setText(str(value))
-        self._edit.setModified(True)
-
-    def set_gooey_param_default(self, value):
-        placeholder = 'Select path'
-        if value not in (_NoValue, None):
-            placeholder += f'(default: {value})'
-        self._edit.setPlaceholderText(placeholder)
-
-    def get_gooey_param_value(self):
-        # return the value if it was set be the caller, or modified
-        # by the user -- otherwise stay silent and let the command
-        # use its default
-        edit = self._edit
-        if not edit.isModified():
-            raise ValueError
-        return edit.text()
+    def _set_gooey_param_value_in_widget(self, value):
+        if value and value is not _NoValue:
+            self._edit.setText(str(value))
+        else:
+            self._edit.clear()
+
+    def _handle_input(self):
+        val = self._edit.text()
+        # treat an empty path as None
+        self._set_gooey_param_value(val if val else None)
 
     def set_gooey_param_docs(self, docs: str) -> None:
         # only use edit tooltip for the docs, and let the buttons
@@ -402,17 +382,24 @@ class PathParamWidget(QWidget, GooeyParamWidgetMixin):
         dialog.finished.connect(self._select_path_receiver)
         dialog.open()
 
-    def _select_path_receiver(self):
+    def _select_path_receiver(self, result_code: int):
         """Internal slot to receive the outcome of _select_path() dialog"""
+        if not result_code:
+            if not self._edit.isEnabled():
+                # if the selection was canceled, clear the path,
+                # otherwise users have no ability to unset a pervious
+                # selection
+                self._set_gooey_param_value_in_widget(_NoValue)
+            # otherwise just keep the present value as-is
+            return
         dialog = self.sender()
         paths = dialog.selectedFiles()
         if paths:
             # ignores any multi-selection
             # TODO prevent or support specifically
-            self.set_gooey_param_value(paths[0])
-            self._edit.setModified(True)
+            self._set_gooey_param_value_in_widget(paths[0])
 
-    def init_gooey_from_other_param(self, spec: Dict) -> None:
+    def _init_gooey_from_other_params(self, spec: Dict) -> None:
         if self._gooey_param_name == 'dataset':
             # prevent update from self
             return
@@ -425,9 +412,8 @@ class CfgProcParamWidget(ChoiceParamWidget):
     """Choice widget with items from `run_procedure(discover=True)`"""
     def __init__(self, choices=None, parent=None):
         super().__init__(parent=parent)
-        self.init_gooey_from_other_param({})
 
-    def init_gooey_from_other_param(self, spec: Dict) -> None:
+    def _init_gooey_from_other_params(self, spec: Dict) -> None:
         if self.count() and 'dataset' not in spec:
             # we have items and no context change is required
             return
@@ -461,7 +447,6 @@ class SiblingChoiceParamWidget(ChoiceParamWidget):
     """Choice widget with items from `siblings()`"""
     def __init__(self, choices=None, parent=None):
         super().__init__(parent=parent)
-        self.init_gooey_from_other_param({})
         self._saw_dataset = False
         self._set_placeholder_msg()
 
@@ -473,7 +458,7 @@ class SiblingChoiceParamWidget(ChoiceParamWidget):
         else:
             self.setPlaceholderText('Select sibling')
 
-    def init_gooey_from_other_param(self, spec: Dict) -> None:
+    def _init_gooey_from_other_params(self, spec: Dict) -> None:
         if 'dataset' not in spec:
             # we have items and no context change is required
             return
@@ -481,24 +466,43 @@ class SiblingChoiceParamWidget(ChoiceParamWidget):
         self._saw_dataset = True
         # the dataset has changed: query!
         # reset first
-        while self.count():
-            self.removeItem(0)
+        self.clear()
         from datalad.distribution.siblings import Siblings
-        for res in Siblings.__call__(
-            dataset=spec['dataset'],
-            action='query',
-            return_type='generator',
-            result_renderer='disabled',
-            on_failure='ignore',
-        ):
-            sibling_name = res.get('name')
-            if (not sibling_name or res.get('status') != 'ok'
-                    or res.get('type') != 'sibling'
-                    or (sibling_name == 'here'
-                        and res.get('path') == spec['dataset'])):
-                # not a good sibling
-                continue
-            self._add_item(sibling_name)
+        from datalad.support.exceptions import (
+            CapturedException,
+            NoDatasetFound,
+        )
+        try:
+            for res in Siblings.__call__(
+                dataset=spec['dataset'],
+                action='query',
+                return_type='generator',
+                result_renderer='disabled',
+                on_failure='ignore',
+            ):
+                sibling_name = res.get('name')
+                if (not sibling_name or res.get('status') != 'ok'
+                        or res.get('type') != 'sibling'
+                        or (sibling_name == 'here'
+                            # be robust with Path objects
+                            and res.get('path') == str(spec['dataset']))):
+                    # not a good sibling
+                    continue
+                self._add_item(sibling_name)
+        except NoDatasetFound as e:
+            CapturedException(e)
+            # TODO this should happen upon validation of the
+            # `dataset` parameter value
+            QMessageBox.critical(
+                self,
+                'No dataset selected',
+                'The path selected for the <code>dataset</code> parameter '
+                'does not point to a valid dataset. '
+                'Please select another path!'
+            )
+            self._saw_dataset = False
+        # always update the placeholder, even when no items were created,
+        # because we have no seen a dataset, and this is the result
+        self._set_placeholder_msg()
         if self.count():
             self.setEnabled(True)
-            self._set_placeholder_msg()

+ 15 - 8
datalad_gooey/tests/test_param_widget.py

@@ -40,21 +40,28 @@ def test_GooeyParamWidgetMixin():
              None),
 
     ):
+        pname = 'peewee'
         # this is how all parameter widgets are instantiated
         parent = QWidget()  # we need parent to stick around,
                             # so nothing gets picked up by GC
         pw = load_parameter_widget(
             parent,
             pw_factory,
-            name='peewee',
+            name=pname,
             docs='EXPLAIN!',
             default=default,
         )
 
-        # If nothing was set yet, we expect `_NoValue` as the "representation of
-        # default" here:
-        assert pw.get_gooey_param_spec() == {'peewee': _NoValue}, \
-            f"Default value not retrieved from {pw_factory.__class__}"
-        pw.set_gooey_param_value(val)
-        # we get the set value back, not the default
-        assert pw.get_gooey_param_spec() == {'peewee': val}
+        # If nothing was set yet, we expect `_NoValue` as the "representation
+        # of default" here:
+        assert pw.get_gooey_param_spec() == {pname: _NoValue}, \
+            f"Default value not retrieved from {pw_factory}"
+        # If nothing other than the default was set yet,
+        # we still expect `_NoValue` as the "representation of default" here:
+        pw.init_gooey_from_params({pname: default})
+        assert pw.get_gooey_param_spec() == {pname: _NoValue}, \
+            f"Default value not retrieved from {pw_factory}"
+        # with a different value set, we get the set value back,
+        # not the default
+        pw.init_gooey_from_params({pname: val})
+        assert pw.get_gooey_param_spec() == {pname: val}