Merge pull request #2087 from jerneju/gh-2054-selectrows-isdefined

[FIX] Select Rows: "is defined" fails

Orange/widgets/data/owselectrows.py

@@ -5,8 +5,8 @@
 
 from AnyQt.QtWidgets import (
     QWidget, QTableWidget, QHeaderView, QComboBox, QLineEdit, QToolButton,
-    QMessageBox, QMenu, QListView, QGridLayout, QPushButton, QSizePolicy
-)
+    QMessageBox, QMenu, QListView, QGridLayout, QPushButton, QSizePolicy,
+    QLabel)
 from AnyQt.QtGui import (
     QDoubleValidator, QRegExpValidator, QStandardItemModel, QStandardItem,
     QFontMetrics, QPalette
@@ -24,6 +24,7 @@
 from Orange.widgets.settings import Setting, ContextSetting, DomainContextHandler
 from Orange.widgets.utils import vartype
 from Orange.canvas import report
+from Orange.widgets.widget import Msg
 
 
 class SelectRowsContextHandler(DomainContextHandler):
@@ -39,7 +40,7 @@ def encode_setting(self, context, setting, value):
             CONTINUOUS = vartype(ContinuousVariable())
             for i, (attr, op, values) in enumerate(value):
                 if context.attributes.get(attr) == CONTINUOUS:
-                    if isinstance(values[0], str):
+                    if values and isinstance(values[0], str):
                         values = [QLocale().toDouble(v)[0] for v in values]
                         value[i] = (attr, op, values)
         return super().encode_setting(context, setting, value)
@@ -120,6 +121,9 @@ class OWSelectRows(widget.OWWidget):
     operator_names = {vtype: [name for _, name in filters]
                       for vtype, filters in Operators.items()}
 
+    class Error(widget.OWWidget.Error):
+        parsing_error = Msg("{}")
+
     def __init__(self):
         super().__init__()
 
@@ -289,7 +293,7 @@ def _get_value_contents(box):
                             names.append(item.text())
                     child.desc_text = ', '.join(names)
                     child.set_text()
-            elif child is None:
+            elif isinstance(child, QLabel) or child is None:
                 pass
             else:
                 raise TypeError('Type %s not supported.' % type(child))
@@ -340,8 +344,10 @@ def add_datetime(contents):
             lc = self._get_lineedit_contents(box) + lc
         oper = oper_combo.currentIndex()
 
-        if oper == oper_combo.count() - 1:
-            self.cond_list.removeCellWidget(oper_combo.row, 2)
+        if oper_combo.currentText() == "is defined":
+            label = QLabel()
+            label.var_type = vartype(var)
+            self.cond_list.setCellWidget(oper_combo.row, 2, label)
         elif var.is_discrete:
             if oper_combo.currentText() == "is one of":
                 if selected_values:
@@ -430,6 +436,8 @@ def conditions_changed(self):
             pass
 
     def _values_to_floats(self, attr, values):
+        if not len(values):
+            return values
         if not all(values):
             return None
         if isinstance(attr, TimeVariable):
@@ -450,7 +458,7 @@ def _values_to_floats(self, attr, values):
     def commit(self):
         matching_output = self.data
         non_matching_output = None
-        self.error()
+        self.Error.clear()
         if self.data:
             domain = self.data.domain
             conditions = []
@@ -463,7 +471,7 @@ def commit(self):
                     try:
                         floats = self._values_to_floats(attr, values)
                     except ValueError as e:
-                        self.error(e.args[0])
+                        self.Error.parsing_error(e.args[0])
                         return
                     if floats is None:
                         continue
@@ -508,7 +516,7 @@ def commit(self):
                 attr_flags = sum([Remove.RemoveConstant * purge_attrs,
                                   Remove.RemoveUnusedValues * purge_attrs])
                 class_flags = sum([Remove.RemoveConstant * purge_classes,
-                                  Remove.RemoveUnusedValues * purge_classes])
+                                   Remove.RemoveUnusedValues * purge_classes])
                 # same settings used for attributes and meta features
                 remover = Remove(attr_flags, class_flags, attr_flags)
 
@@ -532,7 +540,9 @@ def sp(s, capitalize=True):
         else:
             lab1.setText(label + "~%s row%s, %s variable%s" %
                          (sp(data.approx_len()) +
-            sp(len(data.domain.variables) + len(data.domain.metas))))
+                          sp(len(data.domain.variables) +
+                             len(data.domain.metas)))
+                        )
 
     def send_report(self):
         if not self.data:

Orange/widgets/data/tests/test_owselectrows.py

@@ -8,7 +8,7 @@
     Table, ContinuousVariable, StringVariable, DiscreteVariable)
 from Orange.widgets.data.owselectrows import (
     OWSelectRows, FilterDiscreteType, SelectRowsContextHandler)
-from Orange.widgets.tests.base import WidgetTest
+from Orange.widgets.tests.base import WidgetTest, datasets
 
 from Orange.data.filter import FilterContinuous, FilterString
 from Orange.widgets.tests.utils import simulate, override_locale
@@ -115,7 +115,7 @@ def test_continuous_filter_with_sl_SI_locale(self):
         self.enterFilter(iris.domain[2], "is below", "5.2")
         self.assertEqual(self.widget.conditions[0][2], ("52",))
 
-    def enterFilter(self, variable, filter, value, value2=None):
+    def enterFilter(self, variable, filter, value=None, value2=None):
         row = self.widget.cond_list.model().rowCount()
         self.widget.add_button.click()
 
@@ -125,15 +125,21 @@ def enterFilter(self, variable, filter, value, value2=None):
         oper_combo = self.widget.cond_list.cellWidget(row, 1)
         simulate.combobox_activate_item(oper_combo, filter, delay=0)
 
-        value_inputs = self.widget.cond_list.cellWidget(row, 2).children()
-        value_inputs = [w for w in value_inputs if isinstance(w, QLineEdit)]
-        QTest.mouseClick(value_inputs[0], Qt.LeftButton)
-        QTest.keyClicks(value_inputs[0], value, delay=0)
-        QTest.keyClick(value_inputs[0], Qt.Key_Enter)
-        if value2 is not None:
-            QTest.mouseClick(value_inputs[1], Qt.LeftButton)
-            QTest.keyClicks(value_inputs[1], value2, delay=0)
-            QTest.keyClick(value_inputs[1], Qt.Key_Enter)
+        value_inputs = self._get_value_line_edits(row)
+        for i, value in enumerate([value, value2]):
+            if value is None:
+                continue
+            QTest.mouseClick(value_inputs[i], Qt.LeftButton)
+            QTest.keyClicks(value_inputs[i], value, delay=0)
+            QTest.keyClick(value_inputs[i], Qt.Key_Enter)
+
+    def _get_value_line_edits(self, row):
+        value_inputs = self.widget.cond_list.cellWidget(row, 2)
+        if value_inputs:
+            value_inputs = [w for w in value_inputs.children()
+                            if isinstance(w, QLineEdit)]
+        return value_inputs
+
 
     @override_locale(QLocale.Slovenian)
     def test_stores_settings_in_invariant_locale(self):
@@ -214,6 +220,19 @@ def test_load_settings(self):
         oper_combo = w2.cond_list.cellWidget(1, 1)
         self.assertEqual(oper_combo.currentText(), "is at most")
 
+    def test_is_defined_on_continuous_variable(self):
+        # gh-2054 regression
+
+        data = Table(datasets.path("testing_dataset_cls"))
+        self.send_signal("Data", data)
+
+        self.enterFilter(data.domain["c2"], "is defined")
+        self.assertFalse(self.widget.Error.parsing_error.is_shown())
+        self.assertEqual(len(self.get_output("Matching Data")), 3)
+        self.assertEqual(len(self.get_output("Unmatched Data")), 1)
+
+        # Test saving of settings
+        self.widget.settingsHandler.pack_data(self.widget)
 
     def widget_with_context(self, domain, conditions):
         ch = SelectRowsContextHandler()