Merge pull request #1552 from astaric/fix-edit-domain

[FIX] Fix compatibility with Color widget

Author: janezd

Orange/data/variable.py

@@ -471,10 +471,11 @@ def number_of_decimals(self):
     @property
     def colors(self):
         if self._colors is None:
-            if "colors" in self.attributes:
+            try:
                 col1, col2, black = self.attributes["colors"]
                 self._colors = (hex_to_color(col1), hex_to_color(col2), black)
-            else:
+            except (KeyError, ValueError):
+                # Stored colors were not available or invalid, use defaults
                 self._colors = ((0, 0, 255), (255, 255, 0), False)
         return self._colors
 

Orange/widgets/data/oweditdomain.py

@@ -37,7 +37,7 @@ def get_qualified(module, name):
     return getattr(module, name)
 
 
-def variable_description(var):
+def variable_description(var, skip_attributes=False):
     """Return a variable descriptor.
 
     A descriptor is a hashable tuple which should uniquely define
@@ -46,18 +46,21 @@ def variable_description(var):
 
     """
     var_type = type(var)
+    attributes = ()
+    if not skip_attributes:
+        attributes = tuple(sorted(var.attributes.items()))
     if var.is_discrete:
         return (var_type.__module__,
                 var_type.__name__,
                 var.name,
                 (("values", tuple(var.values)),),
-                tuple(sorted(var.attributes.items())))
+                attributes)
     else:
         return (var_type.__module__,
                 var_type.__name__,
                 var.name,
                 (),
-                tuple(sorted(var.attributes.items())))
+                attributes)
 
 
 def variable_from_description(description, compute_value=None):
@@ -101,12 +104,13 @@ def set_dict(self, dict):
             self.appendRow([key_item, value_item])
 
     def get_dict(self):
-        dict = {}
-        for row in range(self.rowCount()):
-            key_item = self.item(row, 0)
-            value_item = self.item(row, 1)
-            dict[str(key_item.text())] = str(value_item.text())
-        return dict
+        # Use the same functionality that parses attributes
+        # when reading text files
+        return Orange.data.Flags([
+            "{}={}".format(self.item(row, 0).text(),
+                           self.item(row, 1).text())
+            for row in range(self.rowCount())
+        ]).attributes
 
 
 class VariableEditor(QWidget):
@@ -423,7 +427,7 @@ def reset_selected(self):
         ind = self.selected_var_index()
         if ind >= 0:
             var = self.input_vars[ind]
-            desc = variable_description(var)
+            desc = variable_description(var, skip_attributes=True)
             if desc in self.domain_change_hints:
                 del self.domain_change_hints[desc]
 
@@ -454,7 +458,7 @@ def _initialize(self):
     def _restore(self):
         # Restore the variable states from saved settings.
         def transform(var):
-            vdesc = variable_description(var)
+            vdesc = variable_description(var, skip_attributes=True)
             if vdesc in self.domain_change_hints:
                 return variable_from_description(
                     self.domain_change_hints[vdesc],
@@ -513,8 +517,8 @@ def _on_variable_changed(self):
 
 
         # Store the transformation hint.
-        self.domain_change_hints[variable_description(old_var)] = \
-                    variable_description(new_var)
+        old_var_desc = variable_description(old_var, skip_attributes=True)
+        self.domain_change_hints[old_var_desc] = variable_description(new_var)
 
         self._invalidate()
 

Orange/widgets/data/tests/test_owcolor.py

@@ -0,0 +1,29 @@
+# Test methods with long descriptive names can omit docstrings
+# pylint: disable=missing-docstring
+from Orange.data import Table, ContinuousVariable, Domain
+from Orange.widgets.data.owcolor import OWColor
+from Orange.widgets.tests.base import WidgetTest
+
+
+class TestOWColor(WidgetTest):
+    def setUp(self):
+        self.widget = self.create_widget(OWColor)
+
+        self.iris = Table("iris")
+
+    def test_reuse_old_settings(self):
+        self.send_signal("Data", self.iris)
+
+        assert isinstance(self.widget, OWColor)
+        self.widget.saveSettings()
+
+        w = self.create_widget(OWColor, reset_default_settings=False)
+        self.send_signal("Data", self.iris, widget=w)
+
+    def test_invalid_input_colors(self):
+        a = ContinuousVariable("a")
+        a.attributes["colors"] = "invalid"
+        a.colors
+        t = Table(Domain([a]))
+
+        self.send_signal("Data", t)

Orange/widgets/data/tests/test_oweditdomain.py

@@ -2,9 +2,15 @@
 # pylint: disable=missing-docstring
 
 from unittest import TestCase
+import numpy as np
 
-from Orange.data import ContinuousVariable, DiscreteVariable
-from Orange.widgets.data.oweditdomain import EditDomainReport
+from PyQt4.QtCore import QModelIndex, Qt
+
+from Orange.data import ContinuousVariable, DiscreteVariable, Table, Domain
+from Orange.widgets.data.oweditdomain import EditDomainReport, OWEditDomain, \
+    ContinuousVariableEditor
+from Orange.widgets.data.owcolor import OWColor, ColorRole
+from Orange.widgets.tests.base import WidgetTest
 
 SECTION_NAME = "NAME"
 
@@ -69,3 +75,66 @@ def assertNotEmpty(self, iterable):
             next(iter(iterable))
         except StopIteration:
             self.fail("Iterator did not produce any lines")
+
+
+class TestOWEditDomain(WidgetTest):
+    def setUp(self):
+        self.widget = self.create_widget(OWEditDomain)
+        self.iris = Table("iris")
+
+    def test_input_data(self):
+        """Check widget's data with data on the input"""
+        self.assertEqual(self.widget.data, None)
+        self.send_signal("Data", self.iris)
+        self.assertEqual(self.widget.data, self.iris)
+
+    def test_input_data_disconnect(self):
+        """Check widget's data after disconnecting data on the input"""
+        self.send_signal("Data", self.iris)
+        self.assertEqual(self.widget.data, self.iris)
+        self.send_signal("Data", None)
+        self.assertEqual(self.widget.data, None)
+
+    def test_output_data(self):
+        """Check data on the output after apply"""
+        self.send_signal("Data", self.iris)
+        output = self.get_output("Data")
+        np.testing.assert_array_equal(output.X, self.iris.X)
+        np.testing.assert_array_equal(output.Y, self.iris.Y)
+        self.assertEqual(output.domain, self.iris.domain)
+
+    def test_input_from_owcolor(self):
+        """Check widget's data sent from OWColor widget"""
+        owcolor = self.create_widget(OWColor)
+        self.send_signal("Data", self.iris, widget=owcolor)
+        owcolor.disc_model.setData(QModelIndex(), (250, 97, 70, 255), ColorRole)
+        owcolor.cont_model.setData(
+            QModelIndex(), ((255, 80, 114, 255), (255, 255, 0, 255), False),
+            ColorRole)
+        owcolor_output = self.get_output("Data", owcolor)
+        self.send_signal("Data", owcolor_output)
+        self.assertEqual(self.widget.data, owcolor_output)
+        self.assertIsNotNone(self.widget.data.domain.class_vars[-1].colors)
+
+    def test_list_attributes_remain_lists(self):
+        a = ContinuousVariable("a")
+        a.attributes["list"] = [1, 2, 3]
+        d = Domain([a])
+        t = Table(d)
+
+        self.send_signal("Data", t)
+
+        assert isinstance(self.widget, OWEditDomain)
+        # select first variable
+        idx = self.widget.domain_view.model().index(0)
+        self.widget.domain_view.setCurrentIndex(idx)
+
+        # change first attribute value
+        editor = self.widget.editor_stack.findChild(ContinuousVariableEditor)
+        assert isinstance(editor, ContinuousVariableEditor)
+        idx = editor.labels_model.index(0, 1)
+        editor.labels_model.setData(idx, "[1, 2, 4]", Qt.EditRole)
+
+        self.widget.unconditional_commit()
+        t2 = self.get_output("Data")
+        self.assertEqual(t2.domain["a"].attributes["list"], [1, 2, 4])

Orange/widgets/settings.py

@@ -1083,6 +1083,13 @@ def new_context(self, domain, attributes, class_vars, metas):
         context.class_vars = class_vars
         return context
 
+    def clone_context(self, old_context, *args):
+        """Copy of context is always valid, since widgets are using
+        the same domain."""
+        context = self.new_context(*args)
+        context.values = copy.deepcopy(old_context.values)
+        return context
+
     def encode_domain(self, domain):
         """Encode domain into tuples (name, type)
         A tuple is returned for each of attributes, class_vars and metas.

Orange/widgets/tests/base.py

@@ -89,7 +89,7 @@ def tearDownClass(cls):
             w.onDeleteWidget()
             sip.delete(w)
 
-    def create_widget(self, cls, stored_settings=None):
+    def create_widget(self, cls, stored_settings=None, reset_default_settings=True):
         """Create a widget instance.
 
         Parameters
@@ -98,12 +98,16 @@ def create_widget(self, cls, stored_settings=None):
             Widget class to instantiate
         stored_settings : dict
             Default values for settings
+        reset_default_settings : bool
+            If set, widget will start with default values for settings,
+            if not, values accumulated through the session will be used
 
         Returns
         -------
         Widget instance : cls
         """
-        self.reset_default_settings(cls)
+        if reset_default_settings:
+            self.reset_default_settings(cls)
         widget = cls.__new__(cls, signal_manager=self.signal_manager,
                              stored_settings=stored_settings)
         widget.__init__()
@@ -138,6 +142,15 @@ def process_events():
         """
         app.processEvents()
 
+    def show(self, widget=None):
+        """Show widget in interactive mode.
+
+        Useful for debugging tests, as widget can be inspected manually.
+        """
+        widget = widget or self.widget
+        widget.show()
+        app.exec()
+
     def send_signal(self, input_name, value, id=None, widget=None):
         """ Send signal to widget by calling appropriate triggers.