Repository: hanjinliu/magic-class (language: python)

Issue:
"choices" option always refers to the first GUI when multiple GUIs are created from the same magic class
In the following code,

```python
from magicclass import magicclass, field

@magicclass
class A:
    def _get_choices(self, w=None):
        return [id(self), id(w)]
    choices = field(widget_type="Select", options={"choices": _get_choices})

a0 = A()
a1 = A()
```

The `choices` in `a1` is created from `a0._get_choices()`.


Discussion / hints:

Fix patch:
diff --git a/magicclass/fields.py b/magicclass/fields.py
index d167afd0..99224d96 100644
--- a/magicclass/fields.py
+++ b/magicclass/fields.py
@@ -1,4 +1,5 @@
 from __future__ import annotations
+from contextlib import contextmanager
 from typing import Any, TYPE_CHECKING, Callable, TypeVar, overload, Generic, Union
 from typing_extensions import Literal
 from pathlib import Path
@@ -80,13 +81,19 @@ def copy(self) -> MagicField:
             self.default, self.default_factory, self.metadata, self.name, self.record
         )
 
-    def _resolve_choices(self, obj: Any):
+    @contextmanager
+    def _resolve_choices(self, obj: Any) -> dict[str, Any]:
         """If method is given as choices, get generate method from it."""
         from .gui._base import _is_instance_method, _method_as_getter
 
-        _arg_choices = self.options["choices"]
+        _arg_choices = self.options.get("choices", None)
         if _is_instance_method(obj, _arg_choices):
             self.options["choices"] = _method_as_getter(obj, _arg_choices)
+        try:
+            yield self
+        finally:
+            if _arg_choices is not None:
+                self.options["choices"] = _arg_choices
 
     def get_widget(self, obj: Any) -> _W:
         """
@@ -98,10 +105,9 @@ def get_widget(self, obj: Any) -> _W:
         if obj_id in self.guis.keys():
             widget = self.guis[obj_id]
         else:
-            if "choices" in self.options:
-                self._resolve_choices(obj)
-            widget = self.to_widget()
-            self.guis[obj_id] = widget
+            with self._resolve_choices(obj):
+                widget = self.to_widget()
+                self.guis[obj_id] = widget
             if self.parent_class is None:
                 self.parent_class = objtype
             widget.enabled = self.enabled
@@ -118,10 +124,9 @@ def get_action(self, obj: Any) -> AbstractAction:
         if obj_id in self.guis.keys():
             action = self.guis[obj_id]
         else:
-            if "choices" in self.options:
-                self._resolve_choices(obj)
-            action = self.to_action()
-            self.guis[obj_id] = action
+            with self._resolve_choices(obj):
+                action = self.to_action()
+                self.guis[obj_id] = action
             if self.parent_class is None:
                 self.parent_class = objtype
             action.enabled = self.enabled
@@ -567,6 +572,8 @@ def _get_field(
     record: bool,
     field_class: type[MagicField],
 ) -> type[MagicField]:
+    if not isinstance(options, dict):
+        raise TypeError(f"Field options must be a dict, got {type(options)}")
     options = options.copy()
     metadata = dict(widget_type=widget_type, options=options)
 
diff --git a/magicclass/gui/_base.py b/magicclass/gui/_base.py
index 62cfd2e5..972f515c 100644
--- a/magicclass/gui/_base.py
+++ b/magicclass/gui/_base.py
@@ -23,7 +23,7 @@
 from qtpy.QtGui import QIcon
 
 from psygnal import Signal
-from magicgui.signature import MagicParameter
+from magicgui.signature import MagicParameter, split_annotated_type
 from magicgui.widgets import (
     FunctionGui,
     FileEdit,
@@ -536,35 +536,43 @@ def _create_widget_from_method(self, obj: MethodType):
         widget._doc = func.__doc__
 
         # This block enables instance methods in "bind" or "choices" of ValueWidget.
-        all_params = []
+        all_params: list[inspect.Parameter] = []
         for param in func.__signature__.parameters.values():
             if isinstance(param.annotation, _AnnotatedAlias):
                 # TODO: after magicgui supports pydantic, something needs update here.
                 param = MagicParameter.from_parameter(param)
 
             if isinstance(param, MagicParameter):
-                _arg_bind = param.options.get("bind", None)
-                _arg_choices = param.options.get("choices", None)
+                _param = MagicParameter(
+                    name=param.name,
+                    default=param.default,
+                    annotation=split_annotated_type(param.annotation)[0],
+                    gui_options=param.options.copy(),
+                )
+                _arg_bind = _param.options.get("bind", None)
+                _arg_choices = _param.options.get("choices", None)
 
                 # If bound method is a class method, use self.method(widget).
                 if _is_instance_method(self, _arg_bind):
-                    param.options["bind"] = _method_as_getter(self, _arg_bind)
+                    _param.options["bind"] = _method_as_getter(self, _arg_bind)
 
                 # If a MagicFiled is bound, bind the value of the connected widget.
                 elif isinstance(_arg_bind, MagicField):
-                    param.options["bind"] = _field_as_getter(self, _arg_bind)
+                    _param.options["bind"] = _field_as_getter(self, _arg_bind)
 
                 # If choices are provided by a class method, use self.method(widget).
                 if _is_instance_method(self, _arg_choices):
-                    param.options["choices"] = _method_as_getter(self, _arg_choices)
+                    _param.options["choices"] = _method_as_getter(self, _arg_choices)
 
-            all_params.append(param)
+            else:
+                _param = param
 
-        func.__signature__ = func.__signature__.replace(parameters=all_params)
+            all_params.append(_param)
 
+        func.__signature__ = func.__signature__.replace(parameters=all_params)
         # Get the number of parameters except for empty widgets.
         # With these lines, "bind" method of magicgui works inside magicclass.
-        fgui = FunctionGuiPlus.from_callable(obj)
+        fgui = FunctionGuiPlus.from_callable(func)
         n_empty = len([_widget for _widget in fgui if isinstance(_widget, EmptyWidget)])
         nparams = _n_parameters(func) - n_empty
 
@@ -1077,22 +1085,21 @@ def _method_as_getter(self, getter: Callable):
     if _LOCALS in qualname:
         qualname = qualname.split(_LOCALS)[-1]
     *clsnames, funcname = qualname.split(".")
+    ins = self
+    self_cls = ins.__class__.__name__
+    if self_cls not in clsnames:
+        ns = ".".join(clsnames)
+        raise ValueError(
+            f"Method {funcname} is in namespace {ns}, so it is invisible "
+            f"from magicclass {self.__class__.__qualname__}"
+        )
+    i = clsnames.index(self_cls) + 1
 
-    def _func(w):
-        ins = self
-        self_cls = ins.__class__.__name__
-        if self_cls not in clsnames:
-            ns = ".".join(clsnames)
-            raise ValueError(
-                f"Method {funcname} is in namespace {ns}, so it is invisible "
-                f"from magicclass {self.__class__.__qualname__}"
-            )
-        i = clsnames.index(self_cls) + 1
+    for clsname in clsnames[i:]:
+        ins = getattr(ins, clsname)
 
-        for clsname in clsnames[i:]:
-            ins = getattr(ins, clsname)
-        getter = getattr(ins, funcname)
-        return getter(w)
+    def _func(w):
+        return getter(ins, w)
 
     return _func
 


Test patch:
diff --git a/tests/test_bind.py b/tests/test_bind.py
index 1e0b6070..78abd5bb 100644
--- a/tests/test_bind.py
+++ b/tests/test_bind.py
@@ -161,3 +161,22 @@ def func(self, x: Bound[b.x]):
     assert ui._a == 1
     ui["func"].changed()
     assert ui._a == 10
+
+def test_multi_gui():
+    @magicclass
+    class A:
+        def _bind(self, w=None):
+            return id(self)
+
+        def f(self, a: Bound[_bind]):
+            self._a = a
+
+    a0 = A()
+    a1 = A()
+
+
+    a0["f"].changed()
+    a1["f"].changed()
+
+    assert a0._a == id(a0)
+    assert a1._a == id(a1)
diff --git a/tests/test_choices.py b/tests/test_choices.py
index 3b5c5925..48612a11 100644
--- a/tests/test_choices.py
+++ b/tests/test_choices.py
@@ -104,3 +104,27 @@ def _get_choices(self, w=None):
     assert ui[0].choices == (3,)
     assert ui[1].choices == (3,)
     assert ui[2].choices == (0, 1)
+
+def test_multi_gui():
+    @magicclass
+    class A:
+        def _get_choices(self, w=None):
+            return [id(self), id(w)]
+        choices = field(widget_type="Select", options={"choices": _get_choices})
+        @set_options(c={"choices": _get_choices})
+        def f(self, c):
+            pass
+
+    a0 = A()
+    a1 = A()
+
+    assert a0.choices.choices[0] == id(a0)
+    assert a1.choices.choices[0] == id(a1)
+    assert a0.choices.choices[1] != a1.choices.choices[1]
+
+    a0["f"].changed()
+    a1["f"].changed()
+
+    assert a0["f"].mgui.c.choices[0] == id(a0)
+    assert a1["f"].mgui.c.choices[0] == id(a1)
+    assert a0["f"].mgui.c.choices[1] != a1["f"].mgui.c.choices[1]