Merge pull request #175 from bpoldrack/fix-param-default

Fix defaults for parameter widgets

datalad_gooey/param_widgets.py

@@ -40,7 +40,7 @@ class GooeyParamWidgetMixin:
     """
 
     value_changed = Signal(MappingProxyType)
-    """Signal to be emited whenever a parameter value is changed. The signal
+    """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):
@@ -110,7 +110,7 @@ class GooeyParamWidgetMixin:
             return {self._gooey_param_name: _NoValue}
         return {self._gooey_param_name: val} \
             if val != self._gooey_param_default \
-            else {}
+            else {self._gooey_param_name: _NoValue}
 
     def set_gooey_param_validator(self, validator: Callable) -> None:
         """Set a validator callable that can be used by the widget
@@ -179,6 +179,9 @@ class ChoiceParamWidget(QComboBox, GooeyParamWidgetMixin):
     def __init__(self, choices=None, parent=None):
         super().__init__(parent)
         self.setInsertPolicy(QComboBox.NoInsert)
+        # TODO: We may need to always have --none-- as an option.
+        #       That's b/c of specs like EnsureChoice('a', 'b') | EnsureNone()
+        #       with default None.
         if choices:
             for c in choices:
                 self._add_item(c)
@@ -198,6 +201,14 @@ class ChoiceParamWidget(QComboBox, GooeyParamWidgetMixin):
     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)
 
@@ -222,6 +233,14 @@ class PosIntParamWidget(QSpinBox, GooeyParamWidgetMixin):
         # 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):
         val = self.value()
         # convert special value -1 back to None
@@ -229,6 +248,12 @@ class PosIntParamWidget(QSpinBox, GooeyParamWidgetMixin):
 
 
 class BoolParamWidget(QCheckBox, GooeyParamWidgetMixin):
+
+    def __init__(self, parent=None) -> None:
+        super().__init__(parent)
+        # set to no value initially
+        self._set_gooey_param_value(None)
+
     def _set_gooey_param_value(self, value):
         if value not in (True, False):
             # if the value is not representable by a checkbox
@@ -248,11 +273,15 @@ class BoolParamWidget(QCheckBox, GooeyParamWidgetMixin):
             # TODO error if partiallychecked still (means a
             # value with no default was not set)
             # a default `validator` could handle that
-            # Mixin pics this up and communicates: nothing was set
+            # 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)
+
 
 class StrParamWidget(QLineEdit, GooeyParamWidgetMixin):
     def _set_gooey_param_value(self, value):
@@ -267,7 +296,7 @@ class StrParamWidget(QLineEdit, GooeyParamWidgetMixin):
         # 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() :
+        if self.isEnabled() and not self.isModified():
             raise ValueError('Present value was not modified')
         return self.text()
 

datalad_gooey/tests/test_dataladcmd_ui.py

@@ -36,7 +36,8 @@ def test_GooeyDataladCmdUI(gooey_app, *, qtbot):
         qtbot.mouseClick(ok_button, Qt.LeftButton)
 
     assert_equal(blocker.args[0], 'wtf')
-    assert_in("decor", blocker.args[1].keys())
+    # no parameters given, means none passed via signal:
+    assert_equal({}, blocker.args[1])
 
     # reset_form
     cmdui.reset_form()

datalad_gooey/tests/test_param_widget.py

@@ -1,6 +1,8 @@
 import functools
 from pathlib import Path
 
+from PySide6.QtWidgets import QWidget
+
 from ..param_widgets import (
     BoolParamWidget,
     StrParamWidget,
@@ -10,7 +12,7 @@ from ..param_widgets import (
     load_parameter_widget,
 )
 from ..param_multival_widget import MultiValueInputWidget
-
+from ..utils import _NoValue
 
 
 def test_GooeyParamWidgetMixin():
@@ -18,26 +20,41 @@ def test_GooeyParamWidgetMixin():
     # through the GooeyParamWidgetMixin API
 
     for pw_factory, val, default in (
-            (BoolParamWidget, True, False),
+            (BoolParamWidget, False, True),
+            (BoolParamWidget, False, None),
             (PosIntParamWidget, 4, 1),
+            (functools.partial(PosIntParamWidget, True), 4, None),
             (StrParamWidget, 'dummy', 'mydefault'),
             (functools.partial(ChoiceParamWidget, ['a', 'b', 'c']), 'b', 'c'),
             (PathParamWidget, str(Path.cwd()), 'mypath'),
+            (PathParamWidget, str(Path.cwd()), None),
             # cannot include MultiValueInputWidget, leads to Python segfault
             # on garbage collection?!
-            #(functools.partial(
-            #    MultiValueInputWidget, PathParamWidget),
-            # [str(Path.cwd()), 'temp'],
-            # 'mypath'),
+            (functools.partial(
+                MultiValueInputWidget, PathParamWidget),
+             [str(Path.cwd()), 'temp'],
+             'mypath'),
+            (functools.partial(
+                MultiValueInputWidget, PathParamWidget),
+             [str(Path.cwd()), 'temp'],
+             None),
+
     ):
         # 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(
-            None,
+            parent,
             pw_factory,
             name='peewee',
             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}