diff --git a/src/magicgui/schema/_guiclass.py b/src/magicgui/schema/_guiclass.py index bd2ca157..a51ccb74 100644 --- a/src/magicgui/schema/_guiclass.py +++ b/src/magicgui/schema/_guiclass.py @@ -13,6 +13,7 @@ import warnings from dataclasses import Field, dataclass, field, is_dataclass from typing import TYPE_CHECKING, Any, Callable, ClassVar, TypeVar, overload +from weakref import WeakKeyDictionary from psygnal import SignalGroup, SignalInstance, evented, is_evented from psygnal import __version__ as psygnal_version @@ -22,7 +23,7 @@ from magicgui.widgets.bases import BaseValueWidget, ContainerWidget if TYPE_CHECKING: - from collections.abc import Mapping + from collections.abc import Iterator, Mapping from typing import Protocol from typing_extensions import TypeGuard @@ -318,15 +319,11 @@ def bind_gui_to_instance( widget, PushButton ): name: str = getattr(widget, "name", "") + two_way_obj = TwoWaySetAttr(widget, instance, name) # connect changes in the widget to the instance if hasattr(instance, name): try: - changed.connect_setattr( - instance, - name, - maxargs=1, - **_IGNORE_REF_ERR, # type: ignore - ) + changed.connect(two_way_obj.set_instance_value) except TypeError: warnings.warn( f"Could not bind {name} to {instance}. " @@ -337,10 +334,73 @@ def bind_gui_to_instance( # connect changes from the instance to the widget if name in signals: - signals[name].connect_setattr(widget, "value") + signals[name].connect(two_way_obj.set_widget_value) + +class TwoWaySetAttr: + """Class for setattr operation between widget and instance in guiclass.""" -def unbind_gui_from_instance(gui: ContainerWidget, instance: Any) -> None: + # NOTE: This global instance container is used for two reasons: + # 1. To avoid garbage collection after an instance is connected to a psygnal signal + # instance. + # 2. To disconnect callbacks both ways when it is not to be used anymore. + _all_objs: WeakKeyDictionary[BaseValueWidget, TwoWaySetAttr] = WeakKeyDictionary() + + def __init__( + self, + widget: BaseValueWidget, + instance: Any, + name: str, + ): + if widget in self._all_objs: + raise ValueError(f"widget {widget!r} is used in multiple guiclasses.") + self._widget = widget + self._instance = instance + self._name = name + self._is_setting_widget_value = False + self._is_setting_instance_value = False + self._all_objs[widget] = self + + def set_widget_value(self, value: Any, *_: Any) -> None: + """Set given value to widget.""" + if self._is_setting_widget_value: + # Avoid setting value to widget more than once + return + with self.setting_widget_value(): + self._widget.value = value + + def set_instance_value(self, value: Any) -> None: + """Set given value to instance.""" + if self._is_setting_instance_value: + # Avoid setting value to instance more than once + return + # This method is always called from the `changed` signal of the widget + # so we also need to enter `setting_widget_value` context. + with self.setting_instance_value(), self.setting_widget_value(): + setattr(self._instance, self._name, value) + + @contextlib.contextmanager + def setting_widget_value(self) -> Iterator[None]: + """Widget value is being set in this context.""" + was_setting = self._is_setting_widget_value + self._is_setting_widget_value = True + try: + yield + finally: + self._is_setting_widget_value = was_setting + + @contextlib.contextmanager + def setting_instance_value(self) -> Iterator[None]: + """Instance value is being set in this context.""" + was_setting = self._is_setting_instance_value + self._is_setting_instance_value = True + try: + yield + finally: + self._is_setting_instance_value = was_setting + + +def unbind_gui_from_instance(gui: ContainerWidget, instance: Any | None = None) -> None: """Unbind a gui from an instance. This will disconnect all events that were connected by `bind_gui_to_instance`. @@ -357,7 +417,7 @@ def unbind_gui_from_instance(gui: ContainerWidget, instance: Any) -> None: """ for widget in gui: if isinstance(widget, BaseValueWidget): - widget.changed.disconnect_setattr(instance, widget.name, missing_ok=True) + TwoWaySetAttr._all_objs.pop(widget, None) # Class-based form ... which provides subclassing and inheritance (unlike @guiclass) diff --git a/src/magicgui/widgets/_concrete.py b/src/magicgui/widgets/_concrete.py index d9d7b066..89fa4f35 100644 --- a/src/magicgui/widgets/_concrete.py +++ b/src/magicgui/widgets/_concrete.py @@ -674,10 +674,10 @@ def __init__( button_plus = PushButton(text="+", name="plus") self._insert_widget(0, button_plus) - button_plus.changed.connect(lambda: self._append_value()) + button_plus.changed.connect(lambda: self._append_value(emit=True)) for a in _value: - self._append_value(a) + self._append_value(a, emit=False) self.btn_plus = button_plus @@ -736,7 +736,11 @@ def __delitem__(self, key: int | slice) -> None: ) self.changed.emit(self.value) - def _append_value(self, value: _V | _Undefined = Undefined) -> None: + def _append_value( + self, + value: _V | _Undefined = Undefined, + emit: bool = True, + ) -> None: """Create a new child value widget and append it.""" i = len(self) - 1 @@ -764,7 +768,8 @@ def _remove_me() -> None: widget.value = value widget.changed.connect(lambda: self.changed.emit(self.value)) - self.changed.emit(self.value) + if emit: + self.changed.emit(self.value) def _get_child_widget(self, key: int) -> _ListEditChildWidget[_V]: if key < 0: diff --git a/tests/test_gui_class.py b/tests/test_gui_class.py index 3f457dd4..cb548294 100644 --- a/tests/test_gui_class.py +++ b/tests/test_gui_class.py @@ -221,6 +221,23 @@ class MyGuiClass: assert obj.a.stem == "foo" +def test_list_edit_in_guiclass(): + @guiclass + class MyGuiClass: + x: list[int] + + obj = MyGuiClass([0]) + events_mock = Mock() + gui_mock = Mock() + obj.events.x.connect(events_mock) + obj.gui.x.changed.connect(gui_mock) + events_mock.assert_not_called() + gui_mock.assert_not_called() + obj.gui.x.btn_plus.changed() + events_mock.assert_called_once_with([0, 0], [0]) + gui_mock.assert_called_once_with([0, 0]) + + def test_name_collisions() -> None: """Test that dataclasses can have names colliding with widget attributes."""