Merge pull request #1854 from ales-erjavec/fixes/select-rows-enum-use

[FIX] Select Rows filter enum

Author: lanzagar

.ci_tools/appveyor/step-test.ps1

@@ -57,7 +57,7 @@ try {
 
     # Widget tests
     python -m pip install `
-        --extra-index-url "$Env:STAGING_INDEX" `
+        --index-url "$Env:STAGING_INDEX" `
         PyQt5
 
     echo "Running widget tests with PyQt5"

Orange/widgets/data/owselectrows.py

@@ -1,3 +1,5 @@
+import enum
+
 from collections import OrderedDict
 from itertools import chain
 
@@ -14,6 +16,7 @@
 from Orange.data import (ContinuousVariable, DiscreteVariable, StringVariable,
                          Table, TimeVariable)
 import Orange.data.filter as data_filter
+from Orange.data.filter import FilterContinuous, FilterString
 from Orange.data.domain import filter_visible
 from Orange.data.sql.table import SqlTable
 from Orange.preprocess import Remove
@@ -32,6 +35,13 @@ def is_valid_item(self, setting, condition, attrs, metas):
         return varname in attrs or varname in metas
 
 
+class FilterDiscreteType(enum.Enum):
+    Equal = "Equal"
+    NotEqual = "NotEqual"
+    In = "In"
+    IsDefined = "IsDefined"
+
+
 class OWSelectRows(widget.OWWidget):
     name = "Select Rows"
     id = "Orange.widgets.data.file"
@@ -51,20 +61,43 @@ class OWSelectRows(widget.OWWidget):
     purge_classes = Setting(True)
     auto_commit = Setting(True)
 
-    operator_names = {
-        ContinuousVariable: ["equals", "is not",
-                             "is below", "is at most",
-                             "is greater than", "is at least",
-                             "is between", "is outside",
-                             "is defined"],
-        DiscreteVariable: ["is", "is not", "is one of", "is defined"],
-        StringVariable: ["equals", "is not",
-                         "is before", "is equal or before",
-                         "is after", "is equal or after",
-                         "is between", "is outside", "contains",
-                         "begins with", "ends with",
-                         "is defined"]}
-    operator_names[TimeVariable] = operator_names[ContinuousVariable]
+    Operators = {
+        ContinuousVariable: [
+            (FilterContinuous.Equal, "equals"),
+            (FilterContinuous.NotEqual, "is not"),
+            (FilterContinuous.Less, "is below"),
+            (FilterContinuous.LessEqual, "is at most"),
+            (FilterContinuous.Greater,"is greater than"),
+            (FilterContinuous.GreaterEqual, "is at least"),
+            (FilterContinuous.Between, "is between"),
+            (FilterContinuous.Outside, "is outside"),
+            (FilterContinuous.IsDefined, "is defined"),
+        ],
+        DiscreteVariable: [
+            (FilterDiscreteType.Equal, "is"),
+            (FilterDiscreteType.NotEqual, "is not"),
+            (FilterDiscreteType.In, "is one of"),
+            (FilterDiscreteType.IsDefined, "is defined")
+        ],
+        StringVariable: [
+            (FilterString.Equal, "equals"),
+            (FilterString.NotEqual, "is not"),
+            (FilterString.Less, "is before"),
+            (FilterString.LessEqual, "is equal or before"),
+            (FilterString.Greater, "is after"),
+            (FilterString.GreaterEqual, "is equal or after"),
+            (FilterString.Between, "is between"),
+            (FilterString.Outside, "is outside"),
+            (FilterString.Contains, "contains"),
+            (FilterString.StartsWith, "begins with"),
+            (FilterString.EndsWith, "ends with"),
+            (FilterString.IsDefined, "is defined"),
+        ]
+    }
+    Operators[TimeVariable] = Operators[ContinuousVariable]
+
+    operator_names = {vtype: [name for _, name in filters]
+                      for vtype, filters in Operators.items()}
 
     def __init__(self):
         super().__init__()
@@ -199,8 +232,8 @@ def set_new_operators(self, attr_combo, adding_all,
         var = self.data.domain[attr_combo.currentText()]
         oper_combo.addItems(self.operator_names[type(var)])
         oper_combo.setCurrentIndex(selected_index or 0)
-        self.set_new_values(oper_combo, adding_all, selected_values)
         self.cond_list.setCellWidget(oper_combo.row, 1, oper_combo)
+        self.set_new_values(oper_combo, adding_all, selected_values)
         oper_combo.currentIndexChanged.connect(
             lambda _: self.set_new_values(oper_combo, False))
 
@@ -338,14 +371,17 @@ def set_data(self, data):
         except Exception:
             pass
 
-        if not self.conditions and len(data.domain.variables):
+        variables = list(filter_visible(chain(data.domain.variables,
+                                              data.domain.metas)))
+        varnames = [v.name for v in variables]
+        if self.conditions:
+            for attr, cond_type, cond_value in self.conditions:
+                if attr in varnames:
+                    self.add_row(varnames.index(attr), cond_type, cond_value)
+        elif variables:
             self.add_row()
+
         self.update_info(data, self.data_in_variables, "In: ")
-        for attr, cond_type, cond_value in self.conditions:
-            attrs = [a.name for a in
-                     filter_visible(chain(data.domain.variables, data.domain.metas))]
-            if attr in attrs:
-                self.add_row(attrs.index(attr), cond_type, cond_value)
         self.unconditional_commit()
 
     def conditions_changed(self):
@@ -372,10 +408,11 @@ def commit(self):
         if self.data:
             domain = self.data.domain
             conditions = []
-            for attr_name, oper, values in self.conditions:
+            for attr_name, oper_idx, values in self.conditions:
                 attr_index = domain.index(attr_name)
                 attr = domain[attr_index]
-
+                operators = self.Operators[type(attr)]
+                opertype, _ = operators[oper_idx]
                 if attr.is_continuous:
                     if any(not v for v in values):
                         continue
@@ -389,23 +426,23 @@ def commit(self):
                             return
 
                     filter = data_filter.FilterContinuous(
-                        attr_index, oper, *[float(v) for v in values])
+                        attr_index, opertype, *[float(v) for v in values])
                 elif attr.is_string:
                     filter = data_filter.FilterString(
-                        attr_index, oper, *[str(v) for v in values])
+                        attr_index, opertype, *[str(v) for v in values])
                 else:
-                    if oper == 3:
+                    if opertype == FilterDiscreteType.IsDefined:
                         f_values = None
                     else:
                         if not values or not values[0]:
                             continue
                         values = [attr.values[i-1] for i in values]
-                        if oper == 0:
+                        if opertype == FilterDiscreteType.Equal:
                             f_values = {values[0]}
-                        elif oper == 1:
+                        elif opertype == FilterDiscreteType.NotEqual:
                             f_values = set(attr.values)
                             f_values.remove(values[0])
-                        elif oper == 2:
+                        elif opertype == FilterDiscreteType.In:
                             f_values = set(values)
                         else:
                             raise ValueError("invalid operand")

Orange/widgets/data/tests/test_owselectrows.py

@@ -0,0 +1,81 @@
+# Test methods with long descriptive names can omit docstrings
+# pylint: disable=missing-docstring
+from Orange.data import (
+    Table, ContinuousVariable, StringVariable, DiscreteVariable)
+from Orange.widgets.data.owselectrows import OWSelectRows, FilterDiscreteType
+from Orange.widgets.tests.base import WidgetTest
+
+from Orange.data.filter import FilterContinuous, FilterString
+
+CFValues = {
+    FilterContinuous.Equal: ["5.4"],
+    FilterContinuous.NotEqual: ["5.4"],
+    FilterContinuous.Less: ["5.4"],
+    FilterContinuous.LessEqual: ["5.4"],
+    FilterContinuous.Greater: ["5.4"],
+    FilterContinuous.GreaterEqual: ["5.4"],
+    FilterContinuous.Between: ["5.4", "6.0"],
+    FilterContinuous.Outside: ["5.4", "6.0"],
+    FilterContinuous.IsDefined: [],
+}
+
+
+SFValues = {
+    FilterString.Equal: ["aardwark"],
+    FilterString.NotEqual: ["aardwark"],
+    FilterString.Less: ["aardwark"],
+    FilterString.LessEqual: ["aardwark"],
+    FilterString.Greater: ["aardwark"],
+    FilterString.GreaterEqual: ["aardwark"],
+    FilterString.Between: ["aardwark", "cat"],
+    FilterString.Outside: ["aardwark"],
+    FilterString.Contains: ["aa"],
+    FilterString.StartsWith: ["aa"],
+    FilterString.EndsWith: ["ark"],
+    FilterString.IsDefined: []
+}
+
+DFValues = {
+    FilterDiscreteType.Equal: [0],
+    FilterDiscreteType.NotEqual: [0],
+    FilterDiscreteType.In: [0, 1],
+    FilterDiscreteType.IsDefined: [],
+}
+
+
+class TestOWSelectRows(WidgetTest):
+    def setUp(self):
+        self.widget = self.create_widget(OWSelectRows)  # type: OWSelectRows
+
+    def test_filter_cont(self):
+        iris = Table("iris")[::5]
+        self.widget.auto_commit = True
+        self.widget.set_data(iris)
+
+        for i, (op, _) in enumerate(OWSelectRows.Operators[ContinuousVariable]):
+            self.widget.remove_all()
+            self.widget.add_row(0, i, CFValues[op])
+            self.widget.conditions_changed()
+            self.widget.unconditional_commit()
+
+    def test_filter_str(self):
+        zoo = Table("zoo")[::5]
+        self.widget.auto_commit = False
+        self.widget.set_data(zoo)
+        var_idx = len(zoo.domain)
+        for i, (op, _) in enumerate(OWSelectRows.Operators[StringVariable]):
+            self.widget.remove_all()
+            self.widget.add_row(var_idx, i, SFValues[op])
+            self.widget.conditions_changed()
+            self.widget.unconditional_commit()
+
+    def test_filter_disc(self):
+        lenses = Table("lenses")
+        self.widget.auto_commit = False
+        self.widget.set_data(lenses)
+
+        for i, (op, _) in enumerate(OWSelectRows.Operators[DiscreteVariable]):
+            self.widget.remove_all()
+            self.widget.add_row(0, i, DFValues[op])
+            self.widget.conditions_changed()
+            self.widget.unconditional_commit()