Edit Domain: Use schema-only settings instead of context settings

Orange/widgets/data/oweditdomain.py

@@ -56,6 +56,8 @@
 V = TypeVar("V", bound=Orange.data.Variable)  # pylint: disable=invalid-name
 H = TypeVar("H", bound=Hashable)  # pylint: disable=invalid-name
 
+MAX_HINTS = 1000
+
 
 def unique(sequence: Iterable[H]) -> Iterable[H]:
     """
@@ -1888,13 +1890,11 @@ class Outputs:
     class Error(widget.OWWidget.Error):
         duplicate_var_name = widget.Msg("A variable name is duplicated.")
 
-    settingsHandler = settings.DomainContextHandler()
-    settings_version = 2
+    settings_version = 3
 
-    _domain_change_store = settings.ContextSetting({})
-    _selected_item = settings.ContextSetting(None)  # type: Optional[Tuple[str, int]]
-    _merge_dialog_settings = settings.ContextSetting({})
-    output_table_name = settings.ContextSetting("")
+    _domain_change_hints = settings.Setting({}, schema_only=True)
+    _merge_dialog_settings = settings.Setting({}, schema_only=True)
+    output_table_name = settings.Setting("", schema_only=True)
 
     want_main_area = False
 
@@ -1903,6 +1903,7 @@ def __init__(self):
         self.data = None  # type: Optional[Orange.data.Table]
         #: The current selected variable index
         self.selected_index = -1
+        self._selected_item = None
         self._invalidated = False
         self.typeindex = 0
 
@@ -1960,14 +1961,12 @@ def __init__(self):
     @Inputs.data
     def set_data(self, data):
         """Set input dataset."""
-        self.closeContext()
         self.clear()
         self.data = data
 
         if self.data is not None:
             self.setup_model(data)
             self.le_output_name.setPlaceholderText(data.name)
-            self.openContext(self.data)
             self._editor.set_merge_context(self._merge_dialog_settings)
             self._restore()
         else:
@@ -1983,8 +1982,6 @@ def clear(self):
         assert self.selected_index == -1
         self.selected_index = -1
 
-        self._selected_item = None
-        self._domain_change_store = {}
         self._merge_dialog_settings = {}
 
     def reset_selected(self):
@@ -2006,7 +2003,6 @@ def reset_selected(self):
 
     def reset_all(self):
         """Reset all variables to their original state."""
-        self._domain_change_store = {}
         if self.data is not None:
             model = self.variables_model
             for i in range(model.rowCount()):
@@ -2049,19 +2045,30 @@ def _restore(self, ):
         Restore the edit transform from saved state.
         """
         model = self.variables_model
+        hints = self._domain_change_hints
+        first_key = None
         for i in range(model.rowCount()):
             midx = model.index(i, 0)
             coldesc = model.data(midx, Qt.EditRole)  # type: DataVector
-            tr = self._restore_transform(coldesc.vtype)
+            tr, key = self._restore_transform(coldesc.vtype)
             if tr:
                 model.setData(midx, tr, TransformRole)
+                if first_key is None:
+                    first_key = key
+        # Reduce the number of hints to MAX_HINTS, but keep all current hints
+        # Current hints start with `first_key`.
+        while len(hints) > MAX_HINTS and \
+                (key := next(iter(hints))) is not first_key:
+            del hints[key]  # pylint: disable=unsupported-delete-operation
 
         # Restore the current variable selection
         i = -1
         if self._selected_item is not None:
             for i, vec in enumerate(model):
                 if vec.vtype.name_type() == self._selected_item:
                     break
+            else:
+                self._selected_item = None
         if i == -1 and model.rowCount():
             i = 0
 
@@ -2114,13 +2121,20 @@ def _on_variable_changed(self):
         self._store_transform(var, transform)
         self._invalidate()
 
-    def _store_transform(self, var, transform):
+    def _store_transform(self, var, transform, deconvar=None):
         # type: (Variable, List[Transform]) -> None
-        self._domain_change_store[deconstruct(var)] = [deconstruct(t) for t in transform]
+        deconvar = deconvar or deconstruct(var)
+        # Remove the existing key (if any) to put the new one at the end,
+        # to make sure it comes after the sentinel
+        self._domain_change_hints.pop(deconvar, None)
+        # pylint: disable=unsupported-assignment-operation
+        self._domain_change_hints[deconvar] = \
+            [deconstruct(t) for t in transform]
 
     def _restore_transform(self, var):
         # type: (Variable) -> List[Transform]
-        tr_ = self._domain_change_store.get(deconstruct(var), [])
+        key = deconstruct(var)
+        tr_ = self._domain_change_hints.get(key, [])
         tr = []
 
         for t in tr_:
@@ -2131,7 +2145,11 @@ def _restore_transform(self, var):
                     "Failed to restore transform: {}, {!r}".format(t, err),
                     UserWarning, stacklevel=2
                 )
-        return tr
+        if tr:
+            self._store_transform(var, tr, key)
+        else:
+            key = None
+        return tr, key
 
     def _invalidate(self):
         self._set_modified(True)
@@ -2293,6 +2311,7 @@ def migrate_context(cls, context, version):
                         trs.append(CategoriesMapping(
                             list(zip(src.categories, dst.categories))))
                 store.append((deconstruct(src), [deconstruct(tr) for tr in trs]))
+            # TODO: migrate directly to non-context hints
             context.values["_domain_change_store"] = (dict(store), -2)
 
 

Orange/widgets/data/tests/test_oweditdomain.py

@@ -325,7 +325,7 @@ def test_restore(self):
         w = self.widget
 
         def restore(state):
-            w._domain_change_store = state
+            w._domain_change_hints = state
             w._restore()
 
         model = w.variables_model
@@ -338,6 +338,76 @@ def restore(state):
         tr = model.data(model.index(4), TransformRole)
         self.assertEqual(tr, [AsString(), Rename("Z")])
 
+    def test_hint_keeping(self):
+        editor: ContinuousVariableEditor = self.widget.findChild(ContinuousVariableEditor)
+        name_edit = editor.name_edit
+        model = self.widget.domain_view.model()
+
+        def rename(fr, to):
+            for idx in range(fr, to):
+                self.widget.domain_view.setCurrentIndex(model.index(idx))
+                cur_text = name_edit.text()
+                if cur_text[0] != "x":
+                    name_edit.setText("x" + cur_text)
+                editor.on_name_changed()
+
+        def data(fr, to):
+            return Table.from_numpy(Domain(vars[fr:to]),
+                                    np.zeros((1, to - fr)))
+
+
+        vars = [ContinuousVariable(f"v{i}") for i in range(1020)]
+        self.send_signal(data(0, 5))
+        rename(2, 4)
+
+        self.send_signal(None)
+        self.assertIsNone(self.get_output())
+
+        self.send_signal(data(3, 7))
+        outp = self.get_output()
+        self.assertEqual([var.name for var in outp.domain.attributes],
+                         ["xv3", "v4", "v5", "v6"])
+
+        self.send_signal(data(0, 5))
+        outp = self.get_output()
+        self.assertEqual([var.name for var in outp.domain.attributes],
+                         ["v0", "v1", "xv2", "xv3", "v4"])
+
+        self.send_signal(data(3, 7))
+        outp = self.get_output()
+        self.assertEqual([var.name for var in outp.domain.attributes],
+                         ["xv3", "v4", "v5", "v6"])
+
+        # This is too large: widget should retain just hints related to
+        # the current data
+        self.send_signal(data(3, 1020))
+        outp = self.get_output()
+        self.assertEqual([var.name for var in outp.domain.attributes[:4]],
+                         ["xv3", "v4", "v5", "v6"])
+        rename(5, 1017)
+        self.widget.commit()
+        outp = self.get_output()
+        self.assertEqual([var.name for var in outp.domain.attributes[-3:]],
+                         ["xv1017", "xv1018", "xv1019"])
+
+        self.send_signal(None)
+        self.assertIsNone(self.get_output())
+
+        # Tests that hints for the current data are kept
+        # - including the earliest (v3) and latest (v1019)
+        self.send_signal(data(3, 1020))
+        outp = self.get_output()
+        self.assertEqual([var.name for var in outp.domain.attributes[:4]],
+                         ["xv3", "v4", "v5", "v6"])
+        self.assertEqual([var.name for var in outp.domain.attributes[-3:]],
+                         ["xv1017", "xv1018", "xv1019"])
+
+        # Tests that older hints are dropped: v2 should be lost
+        self.send_signal(data(0, 5))
+        outp = self.get_output()
+        self.assertEqual([var.name for var in outp.domain.attributes],
+                         ["v0", "v1", "v2", "xv3", "v4"])
+
 
 class TestEditors(GuiTest):
     def test_variable_editor(self):