[WIP] (store|retrieve)SpecificSettings called by non-context settings

Orange/widgets/data/owcolor.py

@@ -367,6 +367,8 @@ def set_data(self, data):
 
     def storeSpecificSettings(self):
         # Store the colors that were changed -- but not others
+        if self.current_context is None:
+            return
         self.current_context.disc_data = \
             [(var.name, var.values, "colors" in var.attributes and var.colors)
              for var in self.disc_colors]

Orange/widgets/data/owfile.py

@@ -434,6 +434,8 @@ def _describe(self, table):
         return text
 
     def storeSpecificSettings(self):
+        if self.current_context is None:
+            return
         self.current_context.modified_variables = self.variables[:]
 
     def retrieveSpecificSettings(self):

Orange/widgets/data/owpythonscript.py

@@ -21,7 +21,7 @@
 from Orange.base import Learner, Model
 from Orange.widgets import widget, gui
 from Orange.widgets.utils import itemmodels
-from Orange.widgets.settings import Setting, SettingsHandler
+from Orange.widgets.settings import Setting
 from Orange.widgets.utils.widgetpreview import WidgetPreview
 from Orange.widgets.widget import OWWidget, Input, Output
 
@@ -364,14 +364,6 @@ def select_row(view, row):
                     QItemSelectionModel.ClearAndSelect)
 
 
-class PrepareSavingSettingsHandler(SettingsHandler):
-    """Calls storeSpecificSettings, which is currently not called from non-context handlers."""
-
-    def pack_data(self, widget):
-        widget.storeSpecificSettings()
-        return super().pack_data(widget)
-
-
 class OWPythonScript(widget.OWWidget):
     name = "Python Script"
     description = "Write a Python script and run it on input data or models."
@@ -397,8 +389,6 @@ class Outputs:
 
     signal_names = ("data", "learner", "classifier", "object")
 
-    settingsHandler = PrepareSavingSettingsHandler()
-
     libraryListSource = \
         Setting([Script("Hello world", "print('Hello world')\n")])
     currentScriptIndex = Setting(0)

Orange/widgets/settings.py

@@ -489,6 +489,12 @@ def _add_defaults(self, data):
         new_data.update(data)
         return new_data
 
+    def _prepare_defaults(self, widget):
+        self.defaults = self.provider.pack(widget)
+        for setting, data, _ in self.provider.traverse_settings(data=self.defaults):
+            if setting.schema_only:
+                data.pop(setting.name, None)
+
     def pack_data(self, widget):
         """
         Pack the settings for the given widget. This method is used when
@@ -504,6 +510,7 @@ def pack_data(self, widget):
         ----------
         widget : OWWidget
         """
+        widget.storeSpecificSettings()
         packed_settings = self.provider.pack(widget)
         packed_settings[VERSION_KEY] = self.widget_class.settings_version
         return packed_settings
@@ -517,10 +524,8 @@ def update_defaults(self, widget):
         ----------
         widget : OWWidget
         """
-        self.defaults = self.provider.pack(widget)
-        for setting, data, _ in self.provider.traverse_settings(data=self.defaults):
-            if setting.schema_only:
-                data.pop(setting.name, None)
+        widget.storeSpecificSettings()
+        self._prepare_defaults(widget)
         self.write_defaults()
 
     def fast_save(self, widget, name, value):
@@ -675,6 +680,9 @@ def update_defaults(self, widget):
         Merge the widgets local contexts into the global contexts and persist
         the settings (including the contexts) to disk.
         """
+        # call storeSpecificSettings before settings_from_widget
+        widget.storeSpecificSettings()
+
         self.settings_from_widget(widget)
         globs = self.global_contexts
         assert widget.context_settings is not globs
@@ -683,7 +691,10 @@ def update_defaults(self, widget):
         globs.sort(key=lambda c: -c.time)
         del globs[self.MAX_SAVED_CONTEXTS:]
 
-        super().update_defaults(widget)
+        # Save non-context settings. We do not call super().update_defaults not
+        # to call storeSpecificSettings.
+        self._prepare_defaults(widget)
+        self.write_defaults()
 
     def new_context(self, *args):
         """Create a new context."""
@@ -695,6 +706,7 @@ def open_context(self, widget, *args):
         widget.current_context, is_new = \
             self.find_or_create_context(widget, *args)
         if is_new:
+            widget.storeSpecificSettings()
             try:
                 self.settings_from_widget(widget, *args)
             except TypeError:
@@ -807,6 +819,7 @@ def close_context(self, widget):
         if widget.current_context is None:
             return
 
+        widget.storeSpecificSettings()
         self.settings_from_widget(widget)
         widget.current_context = None
 
@@ -838,8 +851,6 @@ def settings_from_widget(self, widget, *args):
         if context is None:
             return
 
-        widget.storeSpecificSettings()
-
         def packer(setting, instance):
             if isinstance(setting, ContextSetting) and hasattr(instance, setting.name):
                 value = getattr(instance, setting.name)

Orange/widgets/tests/test_context_handler.py

@@ -22,6 +22,9 @@ class SimpleWidget:
     migrate_settings = Mock()
     migrate_context = Mock()
 
+    def __init__(self):
+        self.storeSpecificSettings = Mock()
+
 
 class DummyContext(Context):
     id = 0
@@ -174,7 +177,6 @@ def test_find_or_create_context(self):
     def test_pack_settings_stores_version(self):
         handler = ContextHandler()
         handler.bind(SimpleWidget)
-
         widget = SimpleWidget()
         handler.initialize(widget)
         widget.context_setting = [DummyContext() for _ in range(3)]
@@ -184,6 +186,14 @@ def test_pack_settings_stores_version(self):
         for c in settings["context_settings"]:
             self.assertIn(VERSION_KEY, c.values)
 
+    def test_pack_settings_storeSpecificSettings(self):
+        handler = ContextHandler()
+        handler.bind(SimpleWidget)
+        widget = SimpleWidget()
+        handler.initialize(widget)
+        handler.pack_data(widget)
+        self.assertEqual(1, widget.storeSpecificSettings.call_count)
+
     def test_write_defaults_stores_version(self):
         handler = ContextHandler()
         handler.bind(SimpleWidget)
@@ -202,11 +212,18 @@ def test_write_defaults_stores_version(self):
             for c in contexts:
                 self.assertEqual(c.values.get("__version__", 0xBAD), 1)
 
+    def test_update_defaults_storeSpecificSettings(self):
+        handler = ContextHandler()
+        handler.bind(SimpleWidget)
+        widget = SimpleWidget()
+        handler.initialize(widget)
+        handler.update_defaults(widget)
+        self.assertEqual(1, widget.storeSpecificSettings.call_count)
+
     def test_close_context(self):
         handler = ContextHandler()
         handler.bind(SimpleWidget)
         widget = SimpleWidget()
-        widget.storeSpecificSettings = Mock()
         handler.initialize(widget)
         widget.schema_only_setting = 0xD06F00D
         widget.current_context = handler.new_context()

Orange/widgets/tests/test_settings_handler.py

@@ -342,6 +342,22 @@ def override_default_settings(self, widget, defaults=None):
         if os.path.isfile(filename):
             os.remove(filename)
 
+    def test_pack_settings_storeSpecificSettings(self):
+        handler = SettingsHandler()
+        handler.bind(SimpleWidget)
+        widget = SimpleWidget()
+        handler.initialize(widget)
+        handler.pack_data(widget)
+        self.assertEqual(1, widget.storeSpecificSettings.call_count)
+
+    def test_update_defaults_storeSpecificSettings(self):
+        handler = SettingsHandler()
+        handler.bind(SimpleWidget)
+        widget = SimpleWidget()
+        handler.initialize(widget)
+        handler.update_defaults(widget)
+        self.assertEqual(1, widget.storeSpecificSettings.call_count)
+
 
 class Component:
     int_setting = Setting(42)
@@ -360,6 +376,7 @@ class SimpleWidget:
 
     def __init__(self):
         self.component = Component()
+        self.storeSpecificSettings = Mock()
 
     migrate_settings = Mock()
     migrate_context = Mock()

Orange/widgets/widget.py

@@ -875,8 +875,8 @@ def storeSpecificSettings(self):
         """
         Store data that is not registered as setting.
 
-        This method is called by
-        `Orange.widgets.settings.ContextHandler.settings_from_widget`.
+        This method is called before a widget storing data (by `pack_data`
+        or `update_defaults` from `Orange.widgets.settings.SettingHandler`).
         Widgets may define it to store any data that is not stored in widget
         attributes. See :obj:`Orange.widgets.data.owcolor.OWColor` for an
         example.