Merge pull request #3321 from janezd/context-migration-reject

Settings migration: Allow rejecting a context

Author: astaric

Orange/widgets/settings.py

@@ -49,7 +49,8 @@
 __all__ = ["Setting", "SettingsHandler", "SettingProvider",
            "ContextSetting", "ContextHandler",
            "DomainContextHandler", "PerfectDomainContextHandler",
-           "ClassValuesContextHandler", "widget_settings_dir"]
+           "ClassValuesContextHandler", "widget_settings_dir",
+           "IncompatibleContext"]
 
 _IMMUTABLES = (str, int, bytes, bool, float, tuple)
 
@@ -624,8 +625,16 @@ def read_defaults_file(self, settings_file):
         self._migrate_contexts(self.global_contexts)
 
     def _migrate_contexts(self, contexts):
-        for context in contexts:
-            self.widget_class.migrate_context(context, context.values.pop(VERSION_KEY, 0))
+        i = 0
+        while i < len(contexts):
+            context = contexts[i]
+            try:
+                self.widget_class.migrate_context(
+                    context, context.values.pop(VERSION_KEY, 0))
+            except IncompatibleContext:
+                del contexts[i]
+            else:
+                i += 1
 
     def write_defaults_file(self, settings_file):
         """Call the inherited method, then add global context to the pickle."""

Orange/widgets/tests/test_context_handler.py

@@ -5,7 +5,7 @@
 from unittest.mock import Mock, patch, call
 from Orange.widgets.settings import (
     ContextHandler, ContextSetting, Context, Setting, SettingsPrinter,
-    VERSION_KEY
+    VERSION_KEY, IncompatibleContext
 )
 
 __author__ = 'anze'
@@ -111,6 +111,26 @@ def test_initialize_migrates_contexts(self):
             handler.initialize(widget, dict(context_settings=deepcopy(contexts)))
         migrate_context.assert_has_calls([call(c, c.values[VERSION_KEY]) for c in contexts])
 
+    def test_migrates_settings_removes_incompatible(self):
+        handler = ContextHandler()
+        handler.bind(SimpleWidget)
+
+        widget = SimpleWidget()
+
+        contexts = [Context(foo=i) for i in (13, 13, 0, 1, 13, 2, 13)]
+
+        def migrate_context(context, _):
+            if context.foo == 13:
+                raise IncompatibleContext()
+
+        with patch.object(SimpleWidget, "migrate_context", migrate_context):
+            handler.initialize(widget, dict(context_settings=contexts))
+            contexts = widget.context_settings
+            self.assertEqual(len(contexts), 3)
+            self.assertTrue(
+                all(context.foo == i
+                    for i, context in enumerate(contexts)))
+
     def test_fast_save(self):
         handler = ContextHandler()
         handler.bind(SimpleWidget)

doc/development/source/tutorial-settings.rst

@@ -316,8 +316,16 @@ using models. Function
 :obj:`Orange.widget.settings.migrate_str_to_variable(settings, names=None)`
 makes the necessary changes to the settings listed in `names`. `names` can be
 a list of setting names, a single string or `None`. In the latter case, all
-settings that may refer to variables (that is two-elements tuples constisting
+settings that may refer to variables (that is two-elements tuples consisting
 of a string and an int) are migrated.
 
+What about situations in which some context settings become inapplicable due
+to some changes in the widget? For instance, a widget that used to accept any
+kind of variables is modified so that it requires a numeric variable?
+Context with categorical variables will match and be reused ... and crash the
+widget. In these (rare) cases, `migrate_context` must raise exception
+:obj:`Orange.widget.settings.IncompatibleContext` and the context will be
+removed.
+
 So take some time, write the migrations and do not forget to bump the
 `settings_version` when you do breaking changes.