Merge pull request #5732 from ales-erjavec/edit-domain-indicate-duplicated-errors

[ENH] oweditdomain: Indicate variables in error state

Author: janezd

Orange/widgets/data/oweditdomain.py

@@ -24,9 +24,12 @@
     QStyledItemDelegate, QStyleOptionViewItem, QStyle, QSizePolicy,
     QDialogButtonBox, QPushButton, QCheckBox, QComboBox, QStackedLayout,
     QDialog, QRadioButton, QGridLayout, QLabel, QSpinBox, QDoubleSpinBox,
-    QAbstractItemView, QMenu
+    QAbstractItemView, QMenu, QToolTip
+)
+from AnyQt.QtGui import (
+    QStandardItemModel, QStandardItem, QKeySequence, QIcon, QBrush, QPalette,
+    QHelpEvent
 )
-from AnyQt.QtGui import QStandardItemModel, QStandardItem, QKeySequence, QIcon
 from AnyQt.QtCore import (
     Qt, QSize, QModelIndex, QAbstractItemModel, QPersistentModelIndex, QRect,
     QPoint,
@@ -1588,11 +1591,30 @@ def initStyleOption(self, option, index):
             # mark as changed (maybe also change color, add text, ...)
             option.font.setItalic(True)
 
+        multiplicity = index.data(MultiplicityRole)
+        if isinstance(multiplicity, int) and multiplicity > 1:
+            option.palette.setBrush(QPalette.Text, QBrush(Qt.red))
+            option.palette.setBrush(QPalette.HighlightedText, QBrush(Qt.red))
+
+    def helpEvent(self, event: QHelpEvent, view: QAbstractItemView,
+                  option: QStyleOptionViewItem, index: QModelIndex) -> bool:
+        multiplicity = index.data(MultiplicityRole)
+        name = VariableListModel.effective_name(index)
+        if isinstance(multiplicity, int) and multiplicity > 1 \
+                and name is not None:
+            QToolTip.showText(
+                event.globalPos(), f"Name `{name}` is duplicated",
+                view.viewport()
+            )
+            return True
+        else:  # pragma: no cover
+            return super().helpEvent(event, view, option, index)
+
 
 # Item model for edited variables (Variable). Define a display role to be the
 # source variable name. This is used only in keyboard search. The display is
 # otherwise completely handled by a delegate.
-class VariableListModel(itemmodels.PyListModel):
+class VariableListModel(CountedListModel):
     def data(self, index, role=Qt.DisplayRole):
         # type: (QModelIndex, Qt.ItemDataRole) -> Any
         row = index.row()
@@ -1606,6 +1628,32 @@ def data(self, index, role=Qt.DisplayRole):
                 return item.vtype.name
         return super().data(index, role)
 
+    def key(self, index):
+        return VariableListModel.effective_name(index)
+
+    def keyRoles(self):  # type: () -> FrozenSet[int]
+        return frozenset((Qt.DisplayRole, Qt.EditRole, TransformRole))
+
+    @staticmethod
+    def effective_name(index) -> Optional[str]:
+        item = index.data(Qt.EditRole)
+        if isinstance(item, DataVectorTypes):
+            var = item.vtype
+        elif isinstance(item, VariableTypes):
+            var = item
+        else:
+            return None
+        tr = index.data(TransformRole)
+        return effective_name(var, tr or [])
+
+
+def effective_name(var: Variable, tr: Sequence[Transform]) -> str:
+    name = var.name
+    for t in tr:
+        if isinstance(t, Rename):
+            name = t.name
+    return name
+
 
 class ReinterpretVariableEditor(VariableEditor):
     """

Orange/widgets/data/tests/test_oweditdomain.py

@@ -10,14 +10,13 @@
 from numpy.testing import assert_array_equal
 import pandas as pd
 
-from AnyQt.QtCore import QItemSelectionModel, Qt, QItemSelection
+from AnyQt.QtCore import QItemSelectionModel, Qt, QItemSelection, QPoint
+from AnyQt.QtGui import QPalette, QColor, QHelpEvent
 from AnyQt.QtWidgets import QAction, QComboBox, QLineEdit, \
-    QStyleOptionViewItem, QDialog, QMenu
+    QStyleOptionViewItem, QDialog, QMenu, QToolTip, QListView
 from AnyQt.QtTest import QTest, QSignalSpy
 
-from Orange.widgets.utils import colorpalettes
 from orangewidget.tests.utils import simulate
-from orangewidget.utils.itemmodels import PyListModel
 
 from Orange.data import (
     ContinuousVariable, DiscreteVariable, StringVariable, TimeVariable,
@@ -35,10 +34,12 @@
     VariableEditDelegate, TransformRole,
     RealVector, TimeVector, StringVector, make_dict_mapper, DictMissingConst,
     LookupMappingTransform, as_float_or_nan, column_str_repr, time_parse,
-    GroupItemsDialog)
+    GroupItemsDialog, VariableListModel
+)
 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 import colorpalettes
 from Orange.tests import test_filename, assert_array_nanequal
 
 MArray = np.ma.MaskedArray
@@ -665,32 +666,72 @@ def test_unlink(self):
         self.assertFalse(cbox.isChecked())
 
 
+class TestModels(GuiTest):
+    def test_variable_model(self):
+        model = VariableListModel()
+        self.assertEqual(model.effective_name(model.index(-1, -1)), None)
+
+        def data(row, role):
+            return model.data(model.index(row,), role)
+
+        def set_data(row, data, role):
+            model.setData(model.index(row), data, role)
+
+        model[:] = [
+            RealVector(Real("A", (3, "g"), (), False), lambda: MArray([])),
+            RealVector(Real("B", (3, "g"), (), False), lambda: MArray([])),
+        ]
+        self.assertEqual(data(0, Qt.DisplayRole), "A")
+        self.assertEqual(data(1, Qt.DisplayRole), "B")
+        self.assertEqual(model.effective_name(model.index(1)), "B")
+        set_data(1, [Rename("A")], TransformRole)
+        self.assertEqual(model.effective_name(model.index(1)), "A")
+        self.assertEqual(data(0, MultiplicityRole), 2)
+        self.assertEqual(data(1, MultiplicityRole), 2)
+        set_data(1, [], TransformRole)
+        self.assertEqual(data(0, MultiplicityRole), 1)
+        self.assertEqual(data(1, MultiplicityRole), 1)
+
+
 class TestDelegates(GuiTest):
     def test_delegate(self):
-        model = PyListModel([None])
+        model = VariableListModel([None, None])
 
-        def set_item(v: dict):
-            model.setItemData(model.index(0),  v)
+        def set_item(row: int, v: dict):
+            model.setItemData(model.index(row),  v)
 
-        def get_style_option() -> QStyleOptionViewItem:
+        def get_style_option(row: int) -> QStyleOptionViewItem:
             opt = QStyleOptionViewItem()
-            delegate.initStyleOption(opt, model.index(0))
+            delegate.initStyleOption(opt, model.index(row))
             return opt
 
-        set_item({Qt.EditRole: Categorical("a", (), (), False)})
+        set_item(0, {Qt.EditRole: Categorical("a", (), (), False)})
         delegate = VariableEditDelegate()
-        opt = get_style_option()
+        opt = get_style_option(0)
         self.assertEqual(opt.text, "a")
         self.assertFalse(opt.font.italic())
-        set_item({TransformRole: [Rename("b")]})
-        opt = get_style_option()
+        set_item(0, {TransformRole: [Rename("b")]})
+        opt = get_style_option(0)
         self.assertEqual(opt.text, "a \N{RIGHTWARDS ARROW} b")
         self.assertTrue(opt.font.italic())
 
-        set_item({TransformRole: [AsString()]})
-        opt = get_style_option()
+        set_item(0, {TransformRole: [AsString()]})
+        opt = get_style_option(0)
         self.assertIn("reinterpreted", opt.text)
         self.assertTrue(opt.font.italic())
+        set_item(1, {
+            Qt.EditRole: String("b", (), False),
+            TransformRole: [Rename("a")]
+        })
+        opt = get_style_option(1)
+        self.assertEqual(opt.palette.color(QPalette.Text), QColor(Qt.red))
+        view = QListView()
+        with patch.object(QToolTip, "showText") as p:
+            delegate.helpEvent(
+                QHelpEvent(QHelpEvent.ToolTip, QPoint(0, 0), QPoint(0, 0)),
+                view, opt, model.index(1),
+            )
+            p.assert_called_once()
 
 
 class TestTransforms(TestCase):