Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIX] Edit Domain no longer forgets its settings #6415

Merged
merged 2 commits into from
May 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 59 additions & 17 deletions Orange/widgets/data/oweditdomain.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -56,6 +57,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]:
"""
Expand Down Expand Up @@ -1888,13 +1891,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 = Setting({}, schema_only=True)
_merge_dialog_settings = Setting({}, schema_only=True)
output_table_name = Setting("", schema_only=True)

want_main_area = False

Expand All @@ -1903,6 +1904,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

Expand Down Expand Up @@ -1960,14 +1962,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:
Expand All @@ -1983,8 +1983,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):
Expand All @@ -2006,7 +2004,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()):
Expand Down Expand Up @@ -2049,19 +2046,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

Expand Down Expand Up @@ -2114,13 +2122,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_:
Expand All @@ -2131,7 +2146,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)
Expand Down Expand Up @@ -2295,6 +2314,29 @@ def migrate_context(cls, context, version):
store.append((deconstruct(src), [deconstruct(tr) for tr in trs]))
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
Expand Down
174 changes: 173 additions & 1 deletion Orange/widgets/data/tests/test_oweditdomain.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -325,7 +326,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
Expand All @@ -338,6 +339,177 @@ 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"])

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):
Expand Down