From ebe8975aa3b622c06773a7381968b8f964b598eb Mon Sep 17 00:00:00 2001 From: YooSunyoung Date: Tue, 13 Aug 2024 10:08:41 +0200 Subject: [PATCH 1/5] Optional widget wrapper. [skip ci] --- src/ess/reduce/widgets/__init__.py | 17 ++++++ src/ess/reduce/widgets/_optional_widget.py | 70 ++++++++++++++++++++++ 2 files changed, 87 insertions(+) create mode 100644 src/ess/reduce/widgets/_optional_widget.py diff --git a/src/ess/reduce/widgets/__init__.py b/src/ess/reduce/widgets/__init__.py index 8627c87a..d33754cc 100644 --- a/src/ess/reduce/widgets/__init__.py +++ b/src/ess/reduce/widgets/__init__.py @@ -23,6 +23,7 @@ from ._vector_widget import VectorWidget from ._bounds_widget import BoundsWidget from ._switchable_widget import SwitchWidget +from ._optional_widget import OptionalWidget class EssWidget(Protocol): @@ -56,7 +57,23 @@ def wrapper(param: Parameter) -> widgets.Widget: return wrapper +def optional_widget( + func: Callable[[Parameter], widgets.Widget], +) -> Callable[[Parameter], widgets.Widget]: + """Wrap a widget in a optional widget.""" + + @wraps(func) + def wrapper(param: Parameter) -> widgets.Widget: + widget = func(param) + if param.optional: + return OptionalWidget(widget, name=param.name) + return widget + + return wrapper + + @switchable_widget +@optional_widget # optional_widget should be applied first @singledispatch def create_parameter_widget(param: Parameter) -> widgets.Widget: """Create a widget for a parameter depending on the ``param`` type. diff --git a/src/ess/reduce/widgets/_optional_widget.py b/src/ess/reduce/widgets/_optional_widget.py new file mode 100644 index 00000000..4dec4ffb --- /dev/null +++ b/src/ess/reduce/widgets/_optional_widget.py @@ -0,0 +1,70 @@ +# SPDX-License-Identifier: BSD-3-Clause +# Copyright (c) 2024 Scipp contributors (https://github.com/scipp) +from typing import Any + +from ipywidgets import HTML, HBox, Layout, RadioButtons, Widget + +from ._config import default_style + + +class OptionalWidget(HBox): + """Wrapper widget to handle optional widgets. + + When you retrieve the value of this widget, + it will return the value of the wrapped widget. + The parameter should be set as ``None`` or the actual value. + """ + + def __init__(self, wrapped: Widget, name: str = '', **kwargs) -> None: + self.name = name + self.wrapped = wrapped + self._option_box = RadioButtons( + description="", + style=default_style, + layout=Layout(width="auto", min_width="80px"), + options={str(None): None, "": self.name}, + default_value=None, + tooltips={ + str(None): "The parameter should be set as None", + name: "Click to enable", + }, + ) + if hasattr(wrapped, "disabled"): + # Disable the wrapped widget by default if possible + # since the option box is set to None by default + wrapped.disabled = True + + # Make the wrapped radio box horizontal + self.add_class("widget-optional") + _style_html = HTML( + "", + layout=Layout(display="none"), + ) + + def disable_wrapped(change) -> None: + if change["new"] is None: + if hasattr(wrapped, "disabled"): + wrapped.disabled = True + else: + if hasattr(wrapped, "disabled"): + wrapped.disabled = False + + self._option_box.observe(disable_wrapped, names="value") + + super().__init__([self._option_box, wrapped, _style_html], **kwargs) + + @property + def value(self) -> Any: + if self._option_box.value is None: + self._option_box.value = None + return None + return self.wrapped.value + + @value.setter + def value(self, value: Any) -> None: + if value is None: + self._option_box.value = None + else: + self._option_box.value = self.name + self.wrapped.value = value From 674adadbf80defc457c30e606eafc9c79b9c8086 Mon Sep 17 00:00:00 2001 From: YooSunyoung Date: Tue, 13 Aug 2024 10:39:43 +0200 Subject: [PATCH 2/5] Expose Optional widget. --- src/ess/reduce/widgets/__init__.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/ess/reduce/widgets/__init__.py b/src/ess/reduce/widgets/__init__.py index d33754cc..da6694e0 100644 --- a/src/ess/reduce/widgets/__init__.py +++ b/src/ess/reduce/widgets/__init__.py @@ -168,9 +168,10 @@ def vector_parameter_widget(param: VectorParameter): __all__ = [ 'BoundsWidget', - 'LinspaceWidget', 'EssWidget', - 'VectorWidget', + 'LinspaceWidget', + 'OptionalWidget', 'SwitchWidget', + 'VectorWidget', 'create_parameter_widget', ] From 5d5c63f9a639f875545f71efbe8cc8f0c827a0b2 Mon Sep 17 00:00:00 2001 From: YooSunyoung Date: Tue, 13 Aug 2024 10:40:03 +0200 Subject: [PATCH 3/5] Remove unused arguments. --- src/ess/reduce/widgets/_optional_widget.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/ess/reduce/widgets/_optional_widget.py b/src/ess/reduce/widgets/_optional_widget.py index 4dec4ffb..26f5dd7e 100644 --- a/src/ess/reduce/widgets/_optional_widget.py +++ b/src/ess/reduce/widgets/_optional_widget.py @@ -23,12 +23,8 @@ def __init__(self, wrapped: Widget, name: str = '', **kwargs) -> None: style=default_style, layout=Layout(width="auto", min_width="80px"), options={str(None): None, "": self.name}, - default_value=None, - tooltips={ - str(None): "The parameter should be set as None", - name: "Click to enable", - }, ) + self._option_box.value = None if hasattr(wrapped, "disabled"): # Disable the wrapped widget by default if possible # since the option box is set to None by default From 0fe8fe8595155497aa529fb5aadfef1b37656ff8 Mon Sep 17 00:00:00 2001 From: YooSunyoung Date: Tue, 13 Aug 2024 10:40:15 +0200 Subject: [PATCH 4/5] Add optional widget tests. --- tests/widget_test.py | 68 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 67 insertions(+), 1 deletion(-) diff --git a/tests/widget_test.py b/tests/widget_test.py index 422f6054..56edb721 100644 --- a/tests/widget_test.py +++ b/tests/widget_test.py @@ -7,12 +7,14 @@ import sciline as sl from ess.reduce.parameter import Parameter, parameter_registry from ess.reduce.ui import WorkflowWidget, workflow_widget -from ess.reduce.widgets import SwitchWidget, create_parameter_widget +from ess.reduce.widgets import OptionalWidget, SwitchWidget, create_parameter_widget from ess.reduce.workflow import register_workflow, workflow_registry from ipywidgets import FloatText, IntText SwitchableInt = NewType('SwitchableInt', int) SwitchableFloat = NewType('SwitchableFloat', float) +OptionalInt = int | None +OptionalFloat = float | None class IntParam(Parameter): ... @@ -25,6 +27,8 @@ class FloatParam(Parameter): ... parameter_registry[SwitchableFloat] = FloatParam('_', '_', 2.0, switchable=True) parameter_registry[int] = IntParam('_', '_', 1) parameter_registry[float] = FloatParam('_', '_', 2.0) +parameter_registry[OptionalInt] = IntParam('_', '_', 1, optional=True) +parameter_registry[OptionalFloat] = FloatParam('_', '_', 2.0, optional=True) @create_parameter_widget.register(IntParam) @@ -67,6 +71,11 @@ def provider_with_switch(a: SwitchableInt, b: SwitchableFloat) -> str: return f"{a} + {b}" +def provider_with_optional(a: OptionalInt, b: OptionalFloat) -> str: + parts = [] if a is None else [str(a)] + return ' + '.join([*parts] if b is None else [*parts, str(b)]) + + def _get_param_widget(widget: WorkflowWidget, param_type: type) -> Any: return widget.parameter_box._input_widgets[param_type].children[0] @@ -148,6 +157,63 @@ def test_collect_values_from_enabled_switchable_widget() -> None: assert widget.parameter_box.value == {SwitchableFloat: 0.2} +def test_switchable_optional_parameter_switchable_first() -> None: + dummy_param = Parameter('a', 'a', 1, switchable=True, optional=True) + dummy_widget = create_parameter_widget(dummy_param) + assert isinstance(dummy_widget, SwitchWidget) + assert isinstance(dummy_widget.wrapped, OptionalWidget) + + +def test_optional_widget_dispatch() -> None: + optional_param = Parameter('a', 'a', 1, optional=True) + assert isinstance(create_parameter_widget(optional_param), OptionalWidget) + non_optional_param = Parameter('b', 'b', 2, optional=False) + assert not isinstance(create_parameter_widget(non_optional_param), OptionalWidget) + + +def test_optional_parameter_optional_widget() -> None: + widget = _ready_widget(providers=[provider_with_optional], output_selections=[str]) + + int_widget = _get_param_widget(widget, OptionalInt) + float_widget = _get_param_widget(widget, OptionalFloat) + + assert isinstance(int_widget, OptionalWidget) + assert isinstance(float_widget, OptionalWidget) + + assert float_widget.value is None + assert int_widget.value is None + + +def test_collect_values_from_optional_widget() -> None: + widget = _ready_widget(providers=[provider_with_optional], output_selections=[str]) + + float_widget = _get_param_widget(widget, OptionalFloat) + float_widget.value = 0.2 + + assert widget.parameter_box.value == {OptionalFloat: 0.2, OptionalInt: None} + + +def test_collect_values_from_optional_widget_compute_result() -> None: + result_registry = {} + widget = _ready_widget( + providers=[provider_with_optional], + output_selections=[str], + result_registry=result_registry, + ) + + float_widget = _get_param_widget(widget, OptionalFloat) + float_widget.value = 0.2 + widget.result_box.run_button.click() + + assert result_registry == {str: '0.2'} + + int_widget = _get_param_widget(widget, OptionalInt) + int_widget.value = 2 + widget.result_box.run_button.click() + + assert result_registry == {str: '2 + 0.2'} + + def dummy_workflow_constructor() -> sl.Pipeline: return sl.Pipeline([strict_provider]) From 057b8ce0a092a439bb6463b3034253a392608dca Mon Sep 17 00:00:00 2001 From: YooSunyoung Date: Tue, 13 Aug 2024 12:05:12 +0200 Subject: [PATCH 5/5] Explain switchable and optional widget --- docs/developer/gui.ipynb | 109 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 109 insertions(+) diff --git a/docs/developer/gui.ipynb b/docs/developer/gui.ipynb index 5d43b88e..fafeb0cc 100644 --- a/docs/developer/gui.ipynb +++ b/docs/developer/gui.ipynb @@ -195,6 +195,115 @@ "source": [ "ess_widget" ] + }, + { + "cell_type": "markdown", + "metadata": {}, + "source": [ + "## Wrapper Widgets\n", + "\n", + "In order to handle special cases of parameter settings, we have wrapper widgets.\n", + "\n", + "Each wrapper widget is associated with certain attribute of ``Parameter`` object.\n", + "\n", + "They are implemented as a decorator around widget type dispatch function like below.\n", + "\n", + "It is because of ``@singledispatch`` decorator.\n", + "\n", + "```python\n", + "# In ess.reduce.widgets module\n", + "@switchable_widget\n", + "@optional_widget # optional_widget should be applied first\n", + "@singledispatch\n", + "def create_parameter_widget(param: Parameter) -> widgets.Widget: ...\n", + "```\n", + "\n", + "### Switchable Widget: ``Parameter.switchable``\n", + "\n", + "Widgets are wrapped in ``SwitchableWidget`` if ``Parameter`` is ``switchable``.\n", + "\n", + "The wrapped parameter input widget can be turned off and on.\n", + "\n", + "If the widget is `enabled`(on), the workflow-compute handling widget should set the value as a parameter into the `Pipeline(workflow)`,\n", + "but if the widget is not `enabled`(off), the workflow-compute handling widget should skip setting the value as a parameter.\n", + "\n", + "It means it will either use the default parameter that was set or computed by providers." + ] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "from ess.reduce.widgets import create_parameter_widget\n", + "from ess.reduce.parameter import Parameter\n", + "\n", + "switchable_parameter = Parameter(\n", + " name='SwitchableParameter', description=\"\", default=\"\", switchable=True\n", + ")\n", + "switchable_widget = create_parameter_widget(switchable_parameter)\n", + "switchable_widget.enabled = True\n", + "switchable_widget" + ] + }, + { + "cell_type": "markdown", + "metadata": {}, + "source": [ + "### Optional Widget: ``Parameter.optional``\n", + "\n", + "Widgets are wrapped in a ``OptionalWidget`` if ``Parameter`` is ``optional``.\n", + "\n", + "The wrapped parameter input widget can select ``None`` as a value.\n", + "\n", + "If ``None`` is selected, the workflow-compute handling widget should set ``None`` as a parameter into the `Pipeline(workflow)`,\n", + "but if the widget is not ``None``, the workflow-compute handling widget will retrieve the value from the wrapped widget.\n", + "\n", + "This wrapper is for the providers expecting optional arguments and handle the ``None`` itself." + ] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "from ess.reduce.widgets import create_parameter_widget\n", + "from ess.reduce.parameter import Parameter\n", + "\n", + "optional_parameter = Parameter(\n", + " name='OptionalParameter', description=\"\", default=\"\", optional=True\n", + ")\n", + "optional_widget = create_parameter_widget(optional_parameter)\n", + "optional_widget.value = \"Test String\"\n", + "optional_widget" + ] + }, + { + "cell_type": "markdown", + "metadata": {}, + "source": [ + "If ``Parameter`` object is both ``switchable`` and ``optional``, the widget is wrapped both in ``SwitchWidget`` and ``OptionalWidget``." + ] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "from ess.reduce.widgets import create_parameter_widget\n", + "from ess.reduce.parameter import Parameter\n", + "\n", + "switchable_and_optional_widget = create_parameter_widget(\n", + " Parameter(\n", + " name='Parameter', description=\"\", default=\"\", switchable=True, optional=True\n", + " )\n", + ")\n", + "switchable_and_optional_widget.enabled = True\n", + "switchable_and_optional_widget" + ] } ], "metadata": {