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] oweditdomain: Hide "categories mapping" warning on change #6865

Merged
merged 4 commits into from
Oct 3, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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
65 changes: 49 additions & 16 deletions Orange/widgets/data/oweditdomain.py
Original file line number Diff line number Diff line change
Expand Up @@ -1577,6 +1577,12 @@ def variable_icon(var):
#: (`List[Union[ReinterpretTransform, Transform]]`)
TransformRole = Qt.UserRole + 42

#: Any warnings applying to the transform (`list[tuple[Msg, str]]`)
RestoreWarningRole = TransformRole + 1

#: Hint key that was used to load stored settings.
RestoreHintKey = RestoreWarningRole + 1


class VariableEditDelegate(QStyledItemDelegate):
ReinterpretNames = {
Expand Down Expand Up @@ -1624,9 +1630,16 @@ def initStyleOption(self, option, index):
option.font.setItalic(True)

multiplicity = index.data(MultiplicityRole)
warnings_ = index.data(RestoreWarningRole)

def set_color(palette: QPalette, color):
palette.setBrush(QPalette.Text, QBrush(color))
palette.setBrush(QPalette.HighlightedText, QBrush(color))

if isinstance(multiplicity, int) and multiplicity > 1:
option.palette.setBrush(QPalette.Text, QBrush(Qt.red))
option.palette.setBrush(QPalette.HighlightedText, QBrush(Qt.red))
set_color(option.palette, Qt.red)
elif warnings_:
set_color(option.palette, Qt.yellow)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a minor comment: yellow looks nice in dark mode, but is difficult to see on white background. Could you change it to a color that is visible in both modes, or choose different colors for different modes?

Screenshot 2024-09-20 at 13 57 03


def helpEvent(self, event: QHelpEvent, view: QAbstractItemView,
option: QStyleOptionViewItem, index: QModelIndex) -> bool:
Expand Down Expand Up @@ -2123,16 +2136,18 @@ def reset_selected(self):
modified = []
for ind in self.selected_var_indices():
midx = model.index(ind)
if midx.data(TransformRole):
model.setData(midx, [], TransformRole)
var = midx.data(Qt.EditRole)
self._store_transform(var, [])
modified.append(var)
model.setData(midx, [], TransformRole)
model.setData(midx, None, RestoreWarningRole)
key = model.data(midx, RestoreHintKey)
var = midx.data(Qt.EditRole)
self._domain_change_hints.pop(key, None)
modified.append(var)
if modified:
with disconnected(editor.variable_changed,
self._on_variable_changed):
self._editor.set_data(modified)
self._invalidate()
self._update_restore_warnings()

def reset_all(self):
"""Reset all variables to their original state."""
Expand All @@ -2141,8 +2156,12 @@ def reset_all(self):
for i in range(model.rowCount()):
midx = model.index(i)
model.setData(midx, [], TransformRole)
model.setData(midx, None, RestoreWarningRole)
key = model.data(midx, RestoreHintKey)
self._domain_change_hints.pop(key, None)
self.open_editor()
self._invalidate()
self._update_restore_warnings()

def selected_var_indices(self):
"""Return the current selected variable indices."""
Expand Down Expand Up @@ -2199,7 +2218,6 @@ def _restore(self):
model = self.variables_model
hints = self._domain_change_hints
first_key = None
msgs = []
for i in range(model.rowCount()):
midx = model.index(i, 0)
coldesc = model.data(midx, Qt.EditRole) # type: DataVector
Expand All @@ -2208,9 +2226,10 @@ def _restore(self):
key, tr = res
if tr:
self._store_transform(coldesc.vtype, tr, key)
tr, msgs_ = self._sanitize_transform(coldesc.vtype, tr)
tr, msgs = self._sanitize_transform(coldesc.vtype, tr)
model.setData(midx, tr, TransformRole)
msgs.extend(msgs_)
model.setData(midx, msgs, RestoreWarningRole)
model.setData(midx, key, RestoreHintKey)
if first_key is None:
first_key = key
# Reduce the number of hints to MAX_HINTS, but keep all current hints
Expand All @@ -2219,9 +2238,7 @@ def _restore(self):
(key := next(iter(hints))) != first_key:
del hints[key] # pylint: disable=unsupported-delete-operation

# Show warnings for non-applicable transforms
for msg, names in groupby(msgs, key=itemgetter(0)):
msg(", ".join(map(itemgetter(1), names)))
self._update_restore_warnings()

# Restore the current variable selection
selected_rows = [i for i, vec in enumerate(model)
Expand All @@ -2230,6 +2247,19 @@ def _restore(self):
selected_rows = [0]
itemmodels.select_rows(self.variables_view, selected_rows)

def _update_restore_warnings(self):
def messages(midx):
msgs = model.data(midx, RestoreWarningRole)
return msgs or []
model = self.variables_model
msgs = chain.from_iterable(
messages(model.index(i)) for i in range(model.rowCount())
)
self.Warning.cat_mapping_does_not_apply.clear()
# Show warnings for non-applicable transforms
for msg, names in groupby(sorted(msgs), key=itemgetter(0)):
msg(", ".join(map(itemgetter(1), names)))

def _on_selection_changed(self, _, deselected):
# If the user deselected the last item, select it back with disabled
# signals, so nothing happens
Expand Down Expand Up @@ -2278,19 +2308,22 @@ def _on_variable_changed(self):
*editor.get_data()):
midx = model.index(idx, 0)
model.setData(midx, transform, TransformRole)
model.setData(midx, None, RestoreWarningRole)
self._store_transform(var, transform)
self._invalidate()
self._update_restore_warnings()

def _store_transform(
self, var: Variable, transform: Iterable[Transform], deconvar=None
self, var: Variable, transform: Sequence[Transform] | None, deconvar=None
) -> None:
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]
if transform:
self._domain_change_hints[deconvar] = \
[deconstruct(t) for t in transform]

def _find_stored_transform(
self, var: Variable
Expand Down
21 changes: 20 additions & 1 deletion Orange/widgets/data/tests/test_oweditdomain.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,13 @@
VariableEditDelegate, TransformRole,
RealVector, TimeVector, StringVector, make_dict_mapper,
LookupMappingTransform, as_float_or_nan, column_str_repr,
GroupItemsDialog, VariableListModel, StrpTime, RestoreOriginal, BaseEditor
GroupItemsDialog, VariableListModel, StrpTime, RestoreOriginal, BaseEditor,
RestoreWarningRole
)
from Orange.widgets.data.owcolor import OWColor, ColorRole
from Orange.widgets.tests.base import WidgetTest, GuiTest
from Orange.widgets.tests.utils import contextMenu
from Orange.widgets.utils.itemmodels import select_row
from Orange.widgets.utils import colorpalettes
from Orange.tests import test_filename, assert_array_nanequal

Expand Down Expand Up @@ -382,6 +384,16 @@ def restore(state):
self.assertEqual(output.domain.class_var.values,
("Iris-setosa", "Iris-versicolor", "Iris-virginica"))

restore({viris_1: [("Rename", ("K",),),
("CategoriesMapping", ([("A", "AA")],))]})
self.assertTrue(w.Warning.cat_mapping_does_not_apply.is_shown())
w.reset_all()
self.assertFalse(w.Warning.cat_mapping_does_not_apply.is_shown())

select_row(w.variables_view, 4)
w.reset_selected()
self.assertFalse(w.Warning.cat_mapping_does_not_apply.is_shown())

restore({viris: [("Rename", ("A")), ("NonexistantTransform", ("AA",))]})
tr = model.data(model.index(4), TransformRole)
self.assertEqual(tr, [Rename("A")])
Expand Down Expand Up @@ -1222,6 +1234,13 @@ def get_style_option(row: int) -> QStyleOptionViewItem:
)
p.assert_called_once()

set_item(1, {
TransformRole: Rename("bb"),
RestoreWarningRole: ("bb", "aa"),
})
opt = get_style_option(1)
self.assertEqual(opt.palette.color(QPalette.Text), QColor(Qt.yellow))


class TestTransforms(TestCase):
def _test_common(self, var):
Expand Down
4 changes: 3 additions & 1 deletion Orange/widgets/utils/itemmodels.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,11 @@
__all__ = [
"PyListModel", "VariableListModel", "PyListModelTooltip", "DomainModel",
"AbstractSortTableModel", "PyTableModel", "TableModel",
"ModelActionsWidget", "ListSingleSelectionModel"
"ModelActionsWidget", "ListSingleSelectionModel",
"select_row", "select_rows", "signal_blocking"
]


@contextmanager
def signal_blocking(obj):
blocked = obj.signalsBlocked()
Expand Down
Loading