biolab / orange-widget-base

Base widget and workflow definitions for Orange
Other
25 stars 56 forks source link

Error caused by `_bind_messages` when widget class includes a DataFrame-type attribute #272

Closed hoywu closed 4 months ago

hoywu commented 5 months ago
Orange version

orange-widget-base 4.23.0

Expected behavior

Users should be able to define the attributes they need in the widget class

Actual behavior

When a DataFrame attribute is defined within a widget class, clicking on the widget in Orange will result in pandas raising a ValueError, rendering the widget unusable.

Steps to reproduce the behavior
  1. git clone https://github.com/biolab/orange3-example-addon.git
  2. Add df = pandas.DataFrame() to MyWidget class in orangecontrib/example/widgets/mywidget.py
  3. Install it (pip install -e .)
  4. Open Orange and click this widget
Additional info (worksheets, data, screenshots, ...)
Error msg ``` bash CRITICAL:orangecanvas.scheme.widgetmanager: Traceback (most recent call last): File "venv/lib/python3.10/site-packages/orangecanvas/scheme/widgetmanager.py", line 243, in __add_widget_for_node w = self.create_widget_for_node(node) File "venv/lib/python3.10/site-packages/orangewidget/workflow/widgetsscheme.py", line 300, in create_widget_for_node widget = self.create_widget_instance(node) File "venv/lib/python3.10/site-packages/orangewidget/workflow/widgetsscheme.py", line 403, in create_widget_instance item.widget = widget = klass.__new__( File "venv/lib/python3.10/site-packages/orangewidget/widget.py", line 314, in __new__ WidgetMessagesMixin.__init__(self) File "venv/lib/python3.10/site-packages/orangewidget/utils/messages.py", line 345, in __init__ super().__init__() File "venv/lib/python3.10/site-packages/orangewidget/utils/messages.py", line 310, in __init__ bound_group = group_class(self) File "venv/lib/python3.10/site-packages/orangewidget/utils/messages.py", line 180, in __init__ self._bind_messages() File "venv/lib/python3.10/site-packages/orangewidget/utils/messages.py", line 198, in _bind_messages if group in widget_class.__dict__.values(): File "venv/lib/python3.10/site-packages/pandas/core/generic.py", line 1577, in __nonzero__ raise ValueError( ValueError: The truth value of a DataFrame is ambiguous. Use a.empty, a.bool(), a.item(), a.any() or a.all(). ---------------------------- ValueError Exception ----------------------------- Traceback (most recent call last): File "venv/lib/python3.10/site-packages/orangecanvas/scheme/widgetmanager.py", line 404, in __process_init_queue self.ensure_created(node) File "venv/lib/python3.10/site-packages/orangecanvas/scheme/widgetmanager.py", line 350, in ensure_created self.__add_widget_for_node(node) File "venv/lib/python3.10/site-packages/orangecanvas/scheme/widgetmanager.py", line 243, in __add_widget_for_node w = self.create_widget_for_node(node) File "venv/lib/python3.10/site-packages/orangewidget/workflow/widgetsscheme.py", line 300, in create_widget_for_node widget = self.create_widget_instance(node) File "venv/lib/python3.10/site-packages/orangewidget/workflow/widgetsscheme.py", line 403, in create_widget_instance item.widget = widget = klass.__new__( File "venv/lib/python3.10/site-packages/orangewidget/widget.py", line 314, in __new__ WidgetMessagesMixin.__init__(self) File "venv/lib/python3.10/site-packages/orangewidget/utils/messages.py", line 345, in __init__ super().__init__() File "venv/lib/python3.10/site-packages/orangewidget/utils/messages.py", line 310, in __init__ bound_group = group_class(self) File "venv/lib/python3.10/site-packages/orangewidget/utils/messages.py", line 180, in __init__ self._bind_messages() File "venv/lib/python3.10/site-packages/orangewidget/utils/messages.py", line 198, in _bind_messages if group in widget_class.__dict__.values(): File "venv/lib/python3.10/site-packages/pandas/core/generic.py", line 1577, in __nonzero__ raise ValueError( ValueError: The truth value of a DataFrame is ambiguous. Use a.empty, a.bool(), a.item(), a.any() or a.all(). ------------------------------------------------------------------------------- ```
janezd commented 4 months ago

You have put df = pandas.DataFrame() in the class definition, that is, outside of any method, as a class attribute.

This is a bug in Orange in the sense that a widget should not crash because of this. On the other hand, I'd avoid defining attributes in this way.

The exception is caused by if group in widget_class.__dict__.values() in message binding. group is compared (apparently as __eq__) with a pandas data frame, which -- like numpy -- works element-wise and then doesn't allow for truth testing.

This code is 8 years old, but I guess group must be a class so changing the condition to

                if group in (value for value in widget_class.__dict__.values()
                             if isinstance(value, type)):

should solve this problem. (The widget no longer crashes, but I may have overlooked something and the condition is too strict.)

hoywu commented 4 months ago

On the other hand, I'd avoid defining attributes in this way.

Absolutely. This is a bad programming practice.

But the error caused by this is quite confusing. It may be difficult for people to realize through Traceback that this error is caused by an inappropriate class attribute.

So I was wondering if there was some way to prompt the user to check. (If not, at least people can find this issue now)

Thanks for the response! :)

janezd commented 4 months ago

But the error caused by this is quite confusing. It may be difficult for people to realize through Traceback that this error is caused by an inappropriate class attribute.

I was difficult for me to understand what is going on, too. :)

The code was indeed 8 years old, but the line that caused the problem was from the last year, when I was fixing another issue.

Thanks for the catch. #273 should fix it.

hoywu commented 4 months ago

I can confirm issue has been resolved👍

Thanks for the quick fix!