Merge Data: Implement migrations, add tests

Orange/widgets/data/owmergedata.py

@@ -189,14 +189,15 @@ def open_context(self, widget, domain1, domain2):
 
     @staticmethod
     def encode_variables(variables):
-        return [(v.name, vartype(v)) if isinstance(v, Variable) else (v, -1)
+        return [(v.name, 100 + vartype(v))
+                if isinstance(v, Variable) else (v, 100)
                 for v in variables]
 
     @staticmethod
     def decode_pair(widget, pair):
         left_domain = widget.data and widget.data.domain
         right_domain = widget.extra_data and widget.extra_data.domain
-        return tuple(var[0] if var[1] == -1 else domain[var[0]]
+        return tuple(var[0] if var[1] == 100 else domain[var[0]]
                      for domain, var in zip((left_domain, right_domain), pair))
 
     def _encode_domain(self, domain):
@@ -221,8 +222,7 @@ def settings_to_widget(self, widget, *_args):
 
     def match(self, context, variables1, variables2):
         def matches(part, variables):
-            return all(isinstance(var, int)
-                       or variables.get(var[0], -1) == var[1]
+            return all(var[1] == 100 or variables.get(var[0], -1) == var[1]
                        for var in part)
 
         if (variables1, variables2) == (context.variables1, context.variables2):
@@ -676,12 +676,21 @@ def mig_value(x):
                 del settings[f"attr_{oper}_data"]
                 del settings[f"attr_{oper}_extra"]
 
-        # migrating non-context settings to context settings would be a mess
-        if hasattr(settings, "attr_pairs"):
-            del settings["attr_pairs"]
+        if not version or version < 2 and "attr_pairs" in settings:
+            attr_pairs = settings.pop("attr_pairs")
+            attr_pairs = [tuple((var, 100) if isinstance(var, str) else var
+                                for var in pair)
+                          for pair in attr_pairs]
+            context = ContextHandler().new_context()
+            context.variables1 = \
+                dict(var for var, _ in attr_pairs if var[1] > 100)
+            context.variables2 = \
+                dict(var for _, var in attr_pairs if var[1] > 100)
+            context.values["attr_pairs"] = attr_pairs
+            settings["context_settings"] = [context]
 
 
 if __name__ == "__main__":  # pragma: no cover
     WidgetPreview(OWMergeData).run(
-        setData=Orange.data.Table("tests/data-gender-region"),
-        setExtraData=Orange.data.Table("tests/data-regions"))
+        set_data=Orange.data.Table("tests/data-gender-region"),
+        set_extra_data=Orange.data.Table("tests/data-regions"))

Orange/widgets/data/tests/test_owmergedata.py

@@ -4,6 +4,7 @@
 # pylint: disable=too-many-lines,too-many-public-methods
 from itertools import chain
 import unittest
+from unittest.mock import Mock
 
 import numpy as np
 import scipy.sparse as sp
@@ -12,9 +13,11 @@
 
 from Orange.data import Table, Domain, DiscreteVariable, StringVariable, \
     ContinuousVariable
-from Orange.widgets.data.owmergedata import OWMergeData, INSTANCEID, INDEX
+from Orange.widgets.data.owmergedata import OWMergeData, INSTANCEID, INDEX, \
+    MergeDataContextHandler
 from Orange.widgets.tests.base import WidgetTest
 from Orange.tests import test_filename
+from orangewidget.settings import VERSION_KEY
 
 
 class TestOWMergeData(WidgetTest):
@@ -318,29 +321,44 @@ def test_match_settings(self):
         self.assertEqual(widget.attr_pairs, attr_pairs)
 
     def test_migrate_settings(self):
-        attr1, attr2, attr3, attr4, attr5 = [object() for _ in range(5)]
-        orig_settings = dict(
-            attr_augment_data=attr1, attr_augment_extra=attr2,
-            attr_merge_data=attr3, attr_merge_extra=attr4,
-            attr_combine_data=attr5, attr_combine_extra='Position (index)')
+        def create_and_send(settings):
+            widget = self.create_widget(OWMergeData, stored_settings=settings)
+            for signal in (widget.Inputs.data, widget.Inputs.extra_data):
+                self.send_signal(signal, self.dataA)
+            return widget
+
+        domainA = self.dataA.domain
+        attr1, attr2, attr3 = domainA.variables
+        attr4, attr5 = domainA.metas
 
-        widget = self.create_widget(
-            OWMergeData, stored_settings=dict(merging=0, **orig_settings))
+        # Migration from version == None
+        orig_settings = dict(
+            attr_augment_data=(attr1.name, 101),
+            attr_augment_extra=(attr2.name, 101),
+            attr_merge_data=(attr3.name, 101),
+            attr_merge_extra=(attr4.name, 101),
+            attr_combine_data=(attr5.name, 103),
+            attr_combine_extra='Position (index)')
+
+        widget = create_and_send(dict(merging=0, **orig_settings))
         self.assertEqual(widget.attr_pairs, ([(attr1, attr2)]))
 
-        widget = self.create_widget(
-            OWMergeData, stored_settings=dict(merging=1, **orig_settings))
+        widget = create_and_send(dict(merging=1, **orig_settings))
         self.assertEqual(widget.attr_pairs, ([(attr3, attr4)]))
 
-        widget = self.create_widget(
-            OWMergeData, stored_settings=dict(merging=2, **orig_settings))
+        widget = create_and_send(dict(merging=2, **orig_settings))
         self.assertEqual(widget.attr_pairs, ([(attr5, INDEX)]))
 
         orig_settings["attr_combine_extra"] = "Source position (index)"
-        widget = self.create_widget(
-            OWMergeData, stored_settings=dict(merging=2, **orig_settings))
+        widget = create_and_send(dict(merging=2, **orig_settings))
         self.assertEqual(widget.attr_pairs, ([(attr5, INSTANCEID)]))
 
+        # Migration from version 1
+        settings = {"attr_pairs": [((attr1.name, 101), (attr2.name, 101))],
+                    VERSION_KEY: 1}
+        widget = create_and_send(settings)
+        self.assertEqual(widget.attr_pairs, ([(attr1, attr2)]))
+
     def test_report(self):
         widget = self.widget
         boxes = widget.attr_boxes
@@ -971,5 +989,22 @@ def test_keep_non_duplicate_variables_missing_rows(self):
                              ["A", "B", "C"])
 
 
+class MergeDataContextHandlerTest(unittest.TestCase):
+    # These units are too small to test individually, so they are tested
+    # within their function in the widget.
+
+    # The following test only covers obscure cases that seem to appear only
+    # within the context of some tests and can't appear in real world.
+    def test_malformed_contexts(self):
+        widget = Mock()
+        handler = MergeDataContextHandler()
+        # pylint: disable=protected-access
+        self.assertEqual(handler._encode_domain(None), {})
+
+        widget.current_context = None
+        handler.settings_from_widget(widget)  # mustn't crash
+        handler.settings_to_widget(widget)  # mustn't crash
+
+
 if __name__ == "__main__":
     unittest.main()