diff --git a/Orange/widgets/data/owselectcolumns.py b/Orange/widgets/data/owselectcolumns.py index 3a6198687aa..ed028a3c5e0 100644 --- a/Orange/widgets/data/owselectcolumns.py +++ b/Orange/widgets/data/owselectcolumns.py @@ -1,12 +1,12 @@ from functools import partial from typing import Optional, Dict, Tuple -from AnyQt.QtWidgets import QWidget, QGridLayout -from AnyQt.QtWidgets import QListView from AnyQt.QtCore import ( Qt, QTimer, QSortFilterProxyModel, QItemSelection, QItemSelectionModel, QMimeData, QAbstractItemModel ) +from AnyQt.QtGui import QDrag, QDropEvent +from AnyQt.QtWidgets import QWidget, QGridLayout, QListView from Orange.data import Domain, Variable from Orange.widgets import gui, widget @@ -50,6 +50,10 @@ class VariablesListItemModel(VariableListModel): """ MIME_TYPE = "application/x-Orange-VariableListModelData" + def __init__(self, *args, primitive=False, **kwargs): + super().__init__(*args, **kwargs) + self.primitive = primitive + def flags(self, index): flags = super().flags(index) if index.isValid(): @@ -59,7 +63,7 @@ def flags(self, index): return flags @staticmethod - def supportedDropActions(): + def supportedDropActions(): # pylint: disable=arguments-differ return Qt.MoveAction # pragma: no cover @staticmethod @@ -88,20 +92,95 @@ def dropMimeData(self, mime, action, row, column, parent): Reimplemented. """ if action == Qt.IgnoreAction: - return True # pragma: no cover + return True if not mime.hasFormat(self.MIME_TYPE): - return False # pragma: no cover + return False variables = mime.property("_items") if variables is None: - return False # pragma: no cover + return False if row < 0: row = self.rowCount() + if self.primitive and not all(var.is_primitive() for var in variables): + variables = [var for var in variables if var.is_primitive()] + self[row:row] = variables + mime.setProperty("_moved", variables) + return bool(variables) + self[row:row] = variables + mime.setProperty("_moved", True) return True +class SelectedVarsView(VariablesListItemView): + """ + VariableListItemView that supports partially accepted drags. + + Upon finish, the mime data contains a list of variables accepted by the + destination, and removes only those variables from the model. + """ + def startDrag(self, supported_actions): + indexes = self.selectedIndexes() + if len(indexes) == 0: + return + data = self.model().mimeData(indexes) + if not data: + return + drag = QDrag(self) + drag.setMimeData(data) + res = drag.exec(supported_actions, Qt.DropAction.MoveAction) + + moved = data.property("_moved") + if moved is None: + return + + if moved is True: + # A quicker path if everything is moved. + # When removing rows, private method QAbstractItemView::clearOrRemove + # iterates over ranges and removes them, apparently assuming their + # reverse order. I haven't found any guarantee for this order in + # documentation (nor, actually, in the code that maintains their + # order, so let's sort them. + to_remove = sorted( + ((index.top(), index.bottom() + 1) + for index in self.selectionModel().selection()), + reverse=True) + else: + moved = set(moved) + to_remove = reversed(list(slices( + index.row() for index in self.selectionModel().selectedIndexes() + if index.data(gui.TableVariable) in moved))) + + for start, end in to_remove: + self.model().removeRows(start, end - start) + + self.dragDropActionDidComplete.emit(res) + + +class PrimitivesView(SelectedVarsView): + """ + A SelectedVarsView that accepts drops events if it contains *any* + primitive variables. This overrides the inherited behaviour that accepts + the event only if *all* variables are primitive. + """ + def acceptsDropEvent(self, event: QDropEvent) -> bool: + if event.source() is not None and \ + event.source().window() is not self.window(): + return False # pragma: nocover + + mime = event.mimeData() + items = mime.property('_items') + if items is None or not any(var.is_primitive() for var in items): + return False + + event.accept() + return True + + +# It is, what it is (and should be), pylint: disable=invalid-name class SelectAttributesDomainContextHandler(DomainContextHandler): + # Context handler's methods have variable arguments, + # pylint: disable=arguments-differ,keyword-arg-before-vararg def encode_setting(self, context, setting, value): if setting.name == 'domain_role_hints': value = {(var.name, vartype(var)): role_i @@ -217,7 +296,9 @@ def update_on_change(view): self.available_attrs = VariablesListItemModel() filter_edit, self.available_attrs_view = variables_filter( - parent=self, model=self.available_attrs) + parent=self, model=self.available_attrs, + view_type=SelectedVarsView + ) box.layout().addWidget(filter_edit) self.view_boxes.append((name, box, self.available_attrs_view)) filter_edit.textChanged.connect(self.__var_counts_update_timer.start) @@ -236,11 +317,12 @@ def dropcompleted(action): # 3rd column name = "Features" box = gui.vBox(self.controlArea, name, addToLayout=False) - self.used_attrs = VariablesListItemModel() + self.used_attrs = VariablesListItemModel(primitive=True) filter_edit, self.used_attrs_view = variables_filter( parent=self, model=self.used_attrs, accepted_type=(Orange.data.DiscreteVariable, - Orange.data.ContinuousVariable)) + Orange.data.ContinuousVariable), + view_type=PrimitivesView) self.used_attrs.rowsInserted.connect(self.__used_attrs_changed) self.used_attrs.rowsRemoved.connect(self.__used_attrs_changed) self.used_attrs_view.selectionModel().selectionChanged.connect( @@ -262,10 +344,10 @@ def dropcompleted(action): name = "Target" box = gui.vBox(self.controlArea, name, addToLayout=False) - self.class_attrs = VariablesListItemModel() - self.class_attrs_view = VariablesListItemView( + self.class_attrs = VariablesListItemModel(primitive=True) + self.class_attrs_view = PrimitivesView( acceptedType=(Orange.data.DiscreteVariable, - Orange.data.ContinuousVariable) + Orange.data.ContinuousVariable), ) self.class_attrs_view.setModel(self.class_attrs) self.class_attrs_view.selectionModel().selectionChanged.connect( @@ -279,7 +361,7 @@ def dropcompleted(action): name = "Metas" box = gui.vBox(self.controlArea, name, addToLayout=False) self.meta_attrs = VariablesListItemModel() - self.meta_attrs_view = VariablesListItemView( + self.meta_attrs_view = SelectedVarsView( acceptedType=Orange.data.Variable) self.meta_attrs_view.setModel(self.meta_attrs) self.meta_attrs_view.selectionModel().selectionChanged.connect( @@ -294,7 +376,7 @@ def dropcompleted(action): self.move_attr_button = gui.button( bbox, self, ">", callback=partial(self.move_selected, - self.used_attrs_view) + self.used_attrs_view, primitive=True) ) layout.addWidget(bbox, 0, 1, 1, 1) @@ -302,7 +384,7 @@ def dropcompleted(action): self.move_class_button = gui.button( bbox, self, ">", callback=partial(self.move_selected, - self.class_attrs_view) + self.class_attrs_view, primitive=True) ) layout.addWidget(bbox, 1, 1, 1, 1) @@ -532,14 +614,19 @@ def move_up(self, view: QListView): def move_down(self, view: QListView): self.move_rows(view, 1) - def move_selected(self, view): + def move_selected(self, view, *, primitive=False): if self.selected_rows(view): self.move_selected_from_to(view, self.available_attrs_view) elif self.selected_rows(self.available_attrs_view): - self.move_selected_from_to(self.available_attrs_view, view) + self.move_selected_from_to(self.available_attrs_view, view, + primitive) - def move_selected_from_to(self, src, dst): - self.move_from_to(src, dst, self.selected_rows(src)) + def move_selected_from_to(self, src, dst, primitive=False): + rows = self.selected_rows(src) + if primitive: + model = src.model().sourceModel() + rows = [row for row in rows if model[row].is_primitive()] + self.move_from_to(src, dst, rows) def move_from_to(self, src, dst, rows): src_model = source_model(src) @@ -589,18 +676,18 @@ def selected_vars(view): meta_selected = selected_vars(self.meta_attrs_view) available_types = set(map(type, available_selected)) - all_primitive = all(var.is_primitive() + any_primitive = any(var.is_primitive() for var in available_types) move_attr_enabled = \ - ((available_selected and all_primitive) or attrs_selected) and \ + ((available_selected and any_primitive) or attrs_selected) and \ self.used_attrs_view.isEnabled() self.move_attr_button.setEnabled(bool(move_attr_enabled)) if move_attr_enabled: self.move_attr_button.setText(">" if available_selected else "<") - move_class_enabled = bool(all_primitive and available_selected) or class_selected + move_class_enabled = bool(any_primitive and available_selected) or class_selected self.move_class_button.setEnabled(bool(move_class_enabled)) if move_class_enabled: diff --git a/Orange/widgets/data/tests/test_owselectcolumns.py b/Orange/widgets/data/tests/test_owselectcolumns.py index ab1fbfa1873..20b684bb777 100644 --- a/Orange/widgets/data/tests/test_owselectcolumns.py +++ b/Orange/widgets/data/tests/test_owselectcolumns.py @@ -6,14 +6,18 @@ import numpy as np from AnyQt.QtCore import QMimeData, QPoint, QPointF, Qt from AnyQt.QtGui import QDragEnterEvent, QDropEvent, QDrag +from AnyQt.QtWidgets import QApplication -from Orange.data import Table, ContinuousVariable, DiscreteVariable, Domain +from orangewidget.tests.base import GuiTest + +from Orange.data import Table, Domain, \ + ContinuousVariable, DiscreteVariable, StringVariable from Orange.widgets.settings import ContextSetting from Orange.widgets.utils import vartype from Orange.widgets.tests.base import WidgetTest from Orange.widgets.data.owselectcolumns \ import OWSelectAttributes, VariablesListItemModel, \ - SelectAttributesDomainContextHandler + SelectAttributesDomainContextHandler, SelectedVarsView, PrimitivesView from Orange.widgets.data.owrank import OWRank from Orange.widgets.utils.itemmodels import select_rows from Orange.widgets.widget import AttributeList @@ -22,6 +26,7 @@ Discrete = vartype(DiscreteVariable("d")) +# It is, what it is (and should be), pylint: disable=invalid-name class TestSelectAttributesDomainContextHandler(TestCase): def setUp(self): self.domain = Domain( @@ -41,6 +46,7 @@ def setUp(self): self.handler.read_defaults = lambda: None def test_open_context(self): + # Why not? pylint: disable=use-dict-literal self.handler.bind(SimpleWidget) context = Mock( attributes=self.args[1], metas=self.args[2], values=dict( @@ -68,6 +74,7 @@ def test_open_context(self): domain['c2']: ('class', 0)}) def test_open_context_with_imperfect_match(self): + # Why not? pylint: disable=use-dict-literal self.handler.bind(SimpleWidget) context1 = Mock(values=dict( domain_role_hints=({('d1', Discrete): ('attribute', 0), @@ -97,15 +104,72 @@ def test_open_context_with_imperfect_match(self): class TestModel(TestCase): + def setUp(self): + self.variables = \ + [ContinuousVariable(c) for c in "xyz"] + \ + [StringVariable(s) for s in "spqr"] + \ + [DiscreteVariable(d, values=tuple("def")) for d in "abc"] + + @staticmethod + def _vars(s): + return "".join(var.name for var in s) + def test_drop_mime(self): - iris = Table("iris") - m = VariablesListItemModel(iris.domain.variables) + m = VariablesListItemModel(self.variables) mime = m.mimeData([m.index(1, 0)]) self.assertTrue(mime.hasFormat(VariablesListItemModel.MIME_TYPE)) assert m.dropMimeData(mime, Qt.MoveAction, 5, 0, m.index(-1, -1)) self.assertIs(m[5], m[1]) assert m.dropMimeData(mime, Qt.MoveAction, -1, -1, m.index(-1, -1)) - self.assertIs(m[6], m[1]) + self.assertIs(m[11], m[1]) + + def test_drop_mime_primitive(self): + mime = QMimeData() + # the encoded 'data' is empty, variables are passed by properties + mime.setData(VariablesListItemModel.MIME_TYPE, b'') + mime.setProperty("_items", self.variables[2:]) + + m = VariablesListItemModel(self.variables[:2], primitive=False) + assert m.dropMimeData(mime, Qt.MoveAction, 1, 0, m.index(-1, -1)) + self.assertEqual(self._vars(m), "xzspqrabcy") + self.assertTrue(mime.property("_moved")) + + m = VariablesListItemModel(self.variables[:2], primitive=True) + assert m.dropMimeData(mime, Qt.MoveAction, 1, 0, m.index(-1, -1)) + self.assertEqual(self._vars(m), "xzabcy") + self.assertEqual(self._vars(mime.property("_moved")), "zabc") + + def test_drop_mime_noop(self): + m = VariablesListItemModel(self.variables[:2], primitive=False) + + mime = QMimeData() + # the encoded 'data' is empty, variables are passed by properties + mime.setData(VariablesListItemModel.MIME_TYPE, b'') + + mime.setProperty("_items", self.variables[:2]) + self.assertTrue(m.dropMimeData(mime, Qt.IgnoreAction, 1, 0, m.index(-1, -1))) + self.assertEqual(self._vars(m), "xy") + self.assertIsNone(mime.property("_moved")) + + mime.setProperty("_items", None) + self.assertFalse(m.dropMimeData(mime, Qt.MoveAction, 1, 0, m.index(-1, -1))) + self.assertEqual(self._vars(m), "xy") + self.assertIsNone(mime.property("_moved")) + + mime = QMimeData() + mime.setData("application/x-that-other-format", b'') + mime.setProperty("_items", self.variables[:2]) + + self.assertFalse(m.dropMimeData(mime, Qt.MoveAction, 1, 0, m.index(-1, -1))) + self.assertEqual(self._vars(m), "xy") + self.assertIsNone(mime.property("_moved")) + + def test_mimedata(self): + m = VariablesListItemModel(self.variables) + mime = m.mimeData([m.index(i, 0) for i in (1, 2, 5, 7, 9)]) + # 0123456789 + # xyzspqrabc + self.assertEqual(self._vars(mime.property("_items")), "yzqac") def test_flags(self): m = VariablesListItemModel([ContinuousVariable("X")]) @@ -117,6 +181,96 @@ def test_flags(self): self.assertTrue(flags & Qt.ItemIsDropEnabled) +class TestViews(GuiTest): + def setUp(self): + self.variables = \ + [ContinuousVariable(c) for c in "xyz"] + \ + [StringVariable(s) for s in "spqr"] + \ + [DiscreteVariable(d, values=tuple("def")) for d in "abc"] + self.model = VariablesListItemModel(self.variables) + self.view = SelectedVarsView() + self.view.setModel(self.model) + + @staticmethod + def _vars(s): + return "".join(var.name for var in s) + + @patch("AnyQt.QtGui.QDrag.exec") + def test_noop(self, drag_exec): + with patch.object(self.view, "selectedIndexes", return_value=[]): + assert self.view.startDrag(Qt.MoveAction) is None + drag_exec.assert_not_called() + + with patch.object(self.view, "selectedIndexes", + return_value=[self.model.index(1, 0)]), \ + patch.object(self.model, "mimeData", return_value=None): + assert self.view.startDrag(Qt.MoveAction) is None + drag_exec.assert_not_called() + + def test_move(self): + + def drag_exec(self, *_): + self.mimeData().setProperty("_moved", moved) + return Qt.MoveAction + + # 0123456789 + # xyzspqrabc + # yz p rab + indexes = [self.model.index(i, 0) for i in (1, 2, 4, 6, 7, 8)] + selmodel = self.view.selectionModel() + for index in indexes: + selmodel.select(index, selmodel.Select) + with patch("AnyQt.QtGui.QDrag.exec", drag_exec): + + moved = None + self.view.startDrag(Qt.MoveAction) + self.assertEqual(self.model.rowCount(), 10) + + moved = True + self.view.startDrag(Qt.MoveAction) + self.assertEqual(self._vars(self.model), "xsqc") + + self.model[:] = self.variables + indexes = [self.model.index(i, 0) for i in (1, 2, 4, 6, 7, 8)] + for index in indexes: + selmodel.select(index, selmodel.Select) + moved = [self.model[i] for i in (4, 6)] + self.view.startDrag(Qt.MoveAction) + self.assertEqual(self._vars(self.model), "xyzsqabc") + + @patch("AnyQt.QtGui.QDropEvent.source") + def test_primitives_accepts_drop(self, src): + view = PrimitivesView() + mime = QMimeData() + mime.setData(VariablesListItemModel.MIME_TYPE, b'') + event = QDropEvent(QPointF(20, 20), Qt.MoveAction, mime, + Qt.NoButton, Qt.NoModifier) + + with patch.object(event, "mimeData"): + self.assertFalse(view.acceptsDropEvent(event)) + event.mimeData.assert_not_called() + self.assertFalse(event.isAccepted()) + + src.return_value.window.return_value = view.window() + + mime.setProperty("_items", self.variables) + self.assertTrue(view.acceptsDropEvent(event)) + self.assertTrue(event.isAccepted()) + event.setAccepted(False) + + mime.setProperty("_items", None) + self.assertFalse(view.acceptsDropEvent(event)) + self.assertFalse(event.isAccepted()) + + mime.setProperty("_items", []) + self.assertFalse(view.acceptsDropEvent(event)) + self.assertFalse(event.isAccepted()) + + mime.setProperty("_items", self.variables[3:7]) # string variables + self.assertFalse(view.acceptsDropEvent(event)) + self.assertFalse(event.isAccepted()) + + class SimpleWidget: domain_role_hints = ContextSetting({}) required = ContextSetting("", required=ContextSetting.REQUIRED) @@ -176,6 +330,87 @@ def test_multiple_target_variable(self): self.widget.move_selected(self.widget.class_attrs_view) self.assertVariableCountsEqual(0, 0, 5) + def test_move_to_primitive(self): + app = QApplication.instance() + widget = self.widget + + data = Table("zoo") + self.send_signal(widget.Inputs.data, data) + + # Selecting meta attribute must enable the corresponding button + widget.meta_attrs_view.selectAll() + app.processEvents() + self.assertFalse(widget.move_attr_button.isEnabled()) + self.assertFalse(widget.move_class_button.isEnabled()) + self.assertTrue(widget.move_meta_button.isEnabled()) + + # Moving to available + widget.move_meta_button.click() + self.assertVariableCountsEqual(available=1, used=16, classattrs=1, metas=0) + + # Selecting available attributes must enable only meta button + # because all selected attrs are non-primitive and can't be used for + # features or classes + widget.available_attrs_view.selectAll() + app.processEvents() + self.assertFalse(widget.move_attr_button.isEnabled()) + self.assertFalse(widget.move_class_button.isEnabled()) + self.assertTrue(widget.move_meta_button.isEnabled()) + + # Selecting class attributes must enable the corresponding button + widget.class_attrs_view.selectAll() + app.processEvents() + self.assertFalse(widget.move_attr_button.isEnabled()) + self.assertTrue(widget.move_class_button.isEnabled()) + self.assertFalse(widget.move_meta_button.isEnabled()) + + # Move it to available + widget.move_class_button.click() + self.assertVariableCountsEqual(available=2, used=16, classattrs=0, metas=0) + + # Selecting meta attributes: nothing there, so disable all buttons + widget.meta_attrs_view.selectAll() + app.processEvents() + self.assertFalse(widget.move_attr_button.isEnabled()) + self.assertFalse(widget.move_class_button.isEnabled()) + self.assertFalse(widget.move_meta_button.isEnabled()) + + # Selecting available attributes must now enable all buttons because + # there some of selected attributes are not primitive + widget.available_attrs_view.selectAll() + app.processEvents() + self.assertTrue(widget.move_attr_button.isEnabled()) + self.assertTrue(widget.move_class_button.isEnabled()) + self.assertTrue(widget.move_meta_button.isEnabled()) + + # Move to metas should move both attributes + widget.move_meta_button.click() + self.assertVariableCountsEqual(available=0, used=16, classattrs=0, metas=2) + + # Move them back to available + widget.meta_attrs_view.selectAll() + app.processEvents() + widget.move_meta_button.click() + self.assertVariableCountsEqual(available=2, used=16, classattrs=0, metas=0) + + # Now move them to class: only one should be moved + widget.available_attrs_view.selectAll() + app.processEvents() + widget.move_class_button.click() + self.assertVariableCountsEqual(available=1, used=16, classattrs=1, metas=0) + + # Move them back to available + widget.class_attrs_view.selectAll() + app.processEvents() + widget.move_class_button.click() + self.assertVariableCountsEqual(available=2, used=16, classattrs=0, metas=0) + + # Now move them to attributes: only one should be moved + widget.available_attrs_view.selectAll() + app.processEvents() + widget.move_attr_button.click() + self.assertVariableCountsEqual(available=1, used=17, classattrs=0, metas=0) + def test_input_features(self): data = Table("zoo") in_features = AttributeList(data.domain.attributes)