From d59c0459202a97a287285535079be36dcf8399d7 Mon Sep 17 00:00:00 2001 From: janezd Date: Sat, 15 Apr 2023 18:45:15 +0200 Subject: [PATCH 1/2] Edit Domain: Use schema-only settings instead of context settings --- Orange/widgets/data/oweditdomain.py | 51 ++++++++----- .../widgets/data/tests/test_oweditdomain.py | 72 ++++++++++++++++++- 2 files changed, 106 insertions(+), 17 deletions(-) diff --git a/Orange/widgets/data/oweditdomain.py b/Orange/widgets/data/oweditdomain.py index ee803fb0ca7..ee447b80c7f 100644 --- a/Orange/widgets/data/oweditdomain.py +++ b/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,12 +2045,21 @@ 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 @@ -2062,6 +2067,8 @@ def _restore(self, ): 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) diff --git a/Orange/widgets/data/tests/test_oweditdomain.py b/Orange/widgets/data/tests/test_oweditdomain.py index c3f9a781e76..419b562cc5a 100644 --- a/Orange/widgets/data/tests/test_oweditdomain.py +++ b/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): From c5fa7b7c74799c91d19e59be6acb77896ce697d9 Mon Sep 17 00:00:00 2001 From: janezd Date: Sun, 16 Apr 2023 20:10:07 +0200 Subject: [PATCH 2/2] Edit Domain: Migrate settings 2 to 3 --- Orange/widgets/data/oweditdomain.py | 33 +++++- .../widgets/data/tests/test_oweditdomain.py | 102 ++++++++++++++++++ 2 files changed, 130 insertions(+), 5 deletions(-) diff --git a/Orange/widgets/data/oweditdomain.py b/Orange/widgets/data/oweditdomain.py index ee447b80c7f..0af6f5cbcca 100644 --- a/Orange/widgets/data/oweditdomain.py +++ b/Orange/widgets/data/oweditdomain.py @@ -42,7 +42,8 @@ ) from Orange.misc.collections import DictMissingConst from Orange.util import frompyfunc -from Orange.widgets import widget, gui, settings +from Orange.widgets import widget, gui +from Orange.widgets.settings import Setting from Orange.widgets.utils import itemmodels, ftry, disconnected from Orange.widgets.utils.buttons import FixedSizeButton from Orange.widgets.utils.itemmodels import signal_blocking @@ -1892,9 +1893,9 @@ class Error(widget.OWWidget.Error): settings_version = 3 - _domain_change_hints = settings.Setting({}, schema_only=True) - _merge_dialog_settings = settings.Setting({}, schema_only=True) - output_table_name = settings.Setting("", schema_only=True) + _domain_change_hints = Setting({}, schema_only=True) + _merge_dialog_settings = Setting({}, schema_only=True) + output_table_name = Setting("", schema_only=True) want_main_area = False @@ -2311,9 +2312,31 @@ 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) + @classmethod + def migrate_settings(cls, settings, version): + if version == 2 and "context_settings" in settings: + contexts = settings["context_settings"] + valuess = [] + for context in contexts: + cls.migrate_context(context, context.values["__version__"]) + valuess.append(context.values) + # Fix the order of keys + hints = dict.fromkeys( + chain(*(values["_domain_change_store"][0] + for values in reversed(valuess))) + ) + settings["output_table_name"] = "" + for values in valuess: + hints.update(values["_domain_change_store"][0]) + new_name, _ = values.pop("output_table_name", ("", -2)) + if new_name: + settings["output_table_name"] = new_name + while len(hints) > MAX_HINTS: + del hints[next(iter(hints))] + settings["_domain_change_hints"] = hints + del settings["context_settings"] def enumerate_columns( table: Orange.data.Table diff --git a/Orange/widgets/data/tests/test_oweditdomain.py b/Orange/widgets/data/tests/test_oweditdomain.py index 419b562cc5a..654f0685db6 100644 --- a/Orange/widgets/data/tests/test_oweditdomain.py +++ b/Orange/widgets/data/tests/test_oweditdomain.py @@ -16,6 +16,7 @@ QStyleOptionViewItem, QDialog, QMenu, QToolTip, QListView from AnyQt.QtTest import QTest, QSignalSpy +from orangewidget.settings import Context from orangewidget.tests.utils import simulate from Orange.data import ( @@ -408,6 +409,107 @@ def data(fr, to): self.assertEqual([var.name for var in outp.domain.attributes], ["v0", "v1", "v2", "xv3", "v4"]) + def test_migrate_settings_hints_2_to_3(self): + settings = { + '__version__': 2, + 'context_settings': + [Context(values={ + '_domain_change_store': ( + {('Categorical', ('a', ('mir1', 'mir4', 'mir2'), (), False)): + [('Rename', ('disease mir',))], + ('Categorical', ('b', ('mir4', 'mir1', 'mir2'), (), False)): + [('Rename', ('disease mirs',))] + }, + -2), + '_merge_dialog_settings': ({}, -4), + '_selected_item': (('1', 0), -2), + 'output_table_name': ('boo', -2), + '__version__': 2}), + Context(values={ + '_domain_change_store': ( + {('Categorical', ('b', ('mir4', 'mir1', 'mir2'), (), False)): + [('Rename', ('disease bmir',))], + ('Categorical', ('c', ('mir4', 'mir1', 'mir2'), (), False)): + [('Rename', ('disease mirs',))] + }, + -2), + '_merge_dialog_settings': ({}, -4), + '_selected_item': (('1', 0), -2), + 'output_table_name': ('far', -2), + '__version__': 2}), + ]} + migrated_hints = { + ('Categorical', ('b', ('mir4', 'mir1', 'mir2'), (), False)): + [('Rename', ('disease bmir',))], + ('Categorical', ('c', ('mir4', 'mir1', 'mir2'), (), False)): + [('Rename', ('disease mirs',))], + ('Categorical', ('a', ('mir1', 'mir4', 'mir2'), (), False)): + [('Rename', ('disease mir',))], + } + widget = self.create_widget(OWEditDomain, stored_settings=settings) + self.assertEqual(widget._domain_change_hints, migrated_hints) + # order matters + self.assertEqual(list(widget._domain_change_hints), list(migrated_hints)) + self.assertEqual(widget.output_table_name, "far") + + def test_migrate_settings_2_to_3_realworld(self): + settings = { + 'controlAreaVisible': True, + '__version__': 2, + 'context_settings': [Context( + values={ + '_domain_change_store': + ({('Real', ('sepal length', (1, 'f'), (), False)): + [('AsString', ())], + ('Real', ('sepal width', (1, 'f'), (), False)): + [('AsTime', ()), ('StrpTime', ('Detect automatically', None, 1, 1))], + ('Real', ('petal width', (1, 'f'), (), False)): + [('Annotate', ((('a', 'b'),),))]}, -2), + '_merge_dialog_settings': ({}, -4), + '_selected_item': (('petal width', 2), -2), + 'output_table_name': ('', -2), + '__version__': 2}, + attributes={'sepal length': 2, 'sepal width': 2, + 'petal length': 2, 'petal width': 2, 'iris': 1}, + metas={} + )] + } + widget = self.create_widget(OWEditDomain, stored_settings=settings) + self.assertEqual( + widget._domain_change_hints, + {('Real', ('sepal length', (1, 'f'), (), False)): + [('AsString', ())], + ('Real', ('sepal width', (1, 'f'), (), False)): + [('AsTime', ()), + ('StrpTime', ('Detect automatically', None, 1, 1))], + ('Real', ('petal width', (1, 'f'), (), False)): + [('Annotate', ((('a', 'b'),),))]} + ) + + def test_migrate_settings_name_2_to_3(self): + settings = { + '__version__': 2, + 'context_settings': + [Context(values={ + '_domain_change_store': ({}, -2), + 'output_table_name': ('boo', -2), + '__version__': 2}), + Context(values={ + '_domain_change_store': ({}, -2), + 'output_table_name': ('far', -2), + '__version__': 2}), + Context(values={ + '_domain_change_store': ({}, -2), + 'output_table_name': ('', -2), + '__version__': 2}), + Context(values={ + '_domain_change_store': ({}, -2), + '__version__': 2}) + ] + } + widget = self.create_widget(OWEditDomain, stored_settings=settings) + self.assertEqual(widget.output_table_name, "far") + class TestEditors(GuiTest): def test_variable_editor(self):