magicgui icon indicating copy to clipboard operation
magicgui copied to clipboard

`IOT instruction` with list in `guiclass`

Open multimeric opened this issue 1 year ago • 5 comments

This relates to #674 as I'm testing guiclass.

Code

from dataclasses import field
from magicgui.experimental import guiclass

@guiclass
class MyDataclass:
    colors: list[str] = field(default_factory=list)

if __name__ == '__main__':
    obj = MyDataclass()
    obj.gui.show(run=True)

Environment (please complete the following information):

  • OS: Ubuntu 22.04
  • backend: pyqt5-qt5==5.15.16
  • magicgui version: magicgui==0.10.0

Steps

  1. Run the app
  2. Click on the plus to add a new list item
  3. Click on the minus button

Image

Result

Traceback (most recent call last):
  File "src/psygnal/_signal.py", line 1279, in _run_emit_loop
  File "src/psygnal/_signal.py", line 1308, in _run_emit_loop_immediate
  File "src/psygnal/_weak_callback.py", line 355, in cb
  File "/home/migwell/.local/share/QGIS/QGIS3/profiles/default/python/plugins/MultiBandRaster/.venv/lib/python3.10/site-packages/magicgui/widgets/_concrete.py", line 752, in _remove_me
    self._pop_widget(self.index(widget))
  File "/usr/lib/python3.10/_collections_abc.py", line 1080, in index
    raise ValueError
ValueError

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/migwell/.local/share/QGIS/QGIS3/profiles/default/python/plugins/MultiBandRaster/.venv/lib/python3.10/site-packages/magicgui/widgets/bases/_value_widget.py", line 69, in _on_value_change
    self.changed.emit(value)
  File "src/psygnal/_signal.py", line 1201, in emit
  File "src/psygnal/_signal.py", line 1296, in _run_emit_loop
  File "src/psygnal/_signal.py", line 1279, in _run_emit_loop
  File "src/psygnal/_signal.py", line 1308, in _run_emit_loop_immediate
  File "src/psygnal/_weak_callback.py", line 355, in cb
  File "/home/migwell/.local/share/QGIS/QGIS3/profiles/default/python/plugins/MultiBandRaster/.venv/lib/python3.10/site-packages/magicgui/widgets/_concrete.py", line 752, in _remove_me
    self._pop_widget(self.index(widget))
  File "/usr/lib/python3.10/_collections_abc.py", line 1080, in index
    raise ValueError
psygnal._exceptions.EmitLoopError: 

While emitting signal 'magicgui.widgets.PushButton.changed', an error occurred in a callback:

  ValueError: 
  ------------

  SIGNAL EMISSION: 
    /home/migwell/.local/share/QGIS/QGIS3/profiles/default/python/plugins/MultiBandRaster/.venv/lib/python3.10/site-packages/magicgui/backends/_qtpy/application.py:29 in _mgui_run
      return app.exec_()
    /home/migwell/.local/share/QGIS/QGIS3/profiles/default/python/plugins/MultiBandRaster/.venv/lib/python3.10/site-packages/magicgui/widgets/bases/_value_widget.py:69 in _on_value_change
      self.changed.emit(value)  # <-- SIGNAL WAS EMITTED HERE

  CALLBACK CHAIN:
    src/psygnal/_signal.py:1279 in _run_emit_loop
    ... 3 more frames ...
    /usr/lib/python3.10/_collections_abc.py:1080 in index
      raise ValueError  # <-- ERROR OCCURRED HERE 

      Local variables:
               value = _ListEditChildWidget(value='', annotation=None, name='')
               start = 0
                stop = None
                   i = 2
                   v = PushButton(value=False, annotation=None, name='plus')
[1]    759116 IOT instruction (core dumped)   

multimeric avatar Feb 23 '25 14:02 multimeric

Hey @hanjinliu, would you mind taking a look at this? I think it's an issue in the ListWidget here, and you know that widget better than I do

tlambert03 avatar Feb 23 '25 17:02 tlambert03

@tlambert03 sure, I'll take a look at it in a few days.

hanjinliu avatar Feb 23 '25 23:02 hanjinliu

I guess this error is related to the implementation of guiclass. @multimeric , could you please run following code and see if it works fine? I did not get the error in my environment.

from dataclasses import field, dataclass
from magicgui.schema._ui_field import build_widget

@dataclass
class A:
    x: list[str] = field(default_factory=list)
    i: int = 2

a0 = build_widget(A)
a0.show(True)

hanjinliu avatar Feb 25 '25 21:02 hanjinliu

if you think it's specific to guiclass (and how it interacts with the list widget), could it be related to the signature of the callback that is connected to a change event within the list widget? one "gotcha" in the psygnal changed event is that it provides two arguments (both the new value and the old value) ... so that's one thing that comes to mind here, in case anything was expecting a single value anywhere in the list widget?

(edit: that's one thing that will be different about using build_widget directly... there will be no change events connected and you're unlikely to see the _value_widget.py:69 in _on_value_change error in the traceback above)

tlambert03 avatar Feb 25 '25 22:02 tlambert03

I found that this issue was a combination of two bugs.

1. Bug when using ListEdit with @guiclass

The changed signal of ListEdit is emited twice when it is created by @guiclass.

from dataclasses import field
from magicgui.experimental import guiclass

@guiclass
class A:
    x: list[int] = field(default_factory=list)

a = A()
a.events.x.connect(lambda v: print("events", v))
a.gui.x.changed.connect(lambda v: print("x.changed", v))
a.gui.show(True)

I don't know why this happens for now, but I think some unexpected recursion is happening between guiclass events and widget changed signal. I will send a PR after I found a way to fix it.

2. Bug in Container widget with Qt backend

When a child widget is added to a Container widget, and immediately removed, the child widget survives and will be popped up as a separate window because it no longer has a parent. You can run following code blocks to see what happens.

%gui qt
from magicgui.widgets import Container, LineEdit

c0 = Container(widgets=[LineEdit(value="a")])
c0.show()
c0.append(LineEdit(value="b"))
del c0[-1]

This is why there's additional window created as the screenshot attached by @multimeric .

hanjinliu avatar Feb 26 '25 07:02 hanjinliu