Browse Source

Fix defaults for parameter widgets

This adds a test for parameter widgets to deliver a representation of
the default value (currently `_NoValue`) after initialization with the
parameter specification and fixes several widget classes that failed
this test, delivering the native default of the respective Qt widget
instead.
It also fixes the Mixin implementation of `get_gooey_param_spec` to
deliver the same representation of default under all conditions.

Alternatively, `ChoiceParamWidget` and `PosIntParamWidget` could
implement an equivalent of the `StrParamWidget`'s `setModified` approach
by wiring up the respective signals, but this simpler approach of just
setting the default as the current value also serves the purpose of a
visual representation of the default for those widget types.
Benjamin Poldrack 2 years ago
parent
commit
b48036f9b1

+ 29 - 4
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,7 +273,7 @@ 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
@@ -267,7 +292,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()
 

+ 2 - 1
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()

+ 18 - 6
datalad_gooey/tests/test_param_widget.py

@@ -10,7 +10,7 @@ from ..param_widgets import (
     load_parameter_widget,
 )
 from ..param_multival_widget import MultiValueInputWidget
-
+from ..utils import _NoValue
 
 
 def test_GooeyParamWidgetMixin():
@@ -18,17 +18,25 @@ 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
         pw = load_parameter_widget(
@@ -38,6 +46,10 @@ def test_GooeyParamWidgetMixin():
             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}