Merge pull request #5822 from PrimozGodec/remove-odl-verisons-support

Remove old settings support in Select Rows

Author: janezd

Orange/widgets/data/owselectrows.py

@@ -7,7 +7,7 @@
 from AnyQt.QtWidgets import (
     QWidget, QTableWidget, QHeaderView, QComboBox, QLineEdit, QToolButton,
     QMessageBox, QMenu, QListView, QGridLayout, QPushButton, QSizePolicy,
-    QLabel, QHBoxLayout, QDateTimeEdit)
+    QLabel, QDateTimeEdit)
 from AnyQt.QtGui import (QDoubleValidator, QStandardItemModel, QStandardItem,
                          QFontMetrics, QPalette)
 from AnyQt.QtCore import Qt, QPoint, QPersistentModelIndex, QLocale, \
@@ -74,31 +74,19 @@ def decode_setting(self, setting, value, domain=None, *_args):
         value = super().decode_setting(setting, value, domain)
         if setting.name == 'conditions':
             CONTINUOUS = vartype(ContinuousVariable("x"))
-            # Use this after 2022/2/2:
-            # for i, (attr, tpe, op, values) in enumerate(value):
-            #     if tpe is not None:
-            for i, (attr, *tpe, op, values) in enumerate(value):
-                if tpe != [None] \
-                        or not tpe and attr not in OWSelectRows.AllTypes:
+            for i, (attr, tpe, op, values) in enumerate(value):
+                if tpe is not None:
                     attr = domain[attr]
                 # check for exact match, pylint: disable=unidiomatic-typecheck
                 if type(attr) is ContinuousVariable \
                         or OWSelectRows.AllTypes.get(attr) == CONTINUOUS:
                     values = [QLocale().toString(float(i), 'f') for i in values]
                 elif isinstance(attr, DiscreteVariable):
-                    # After 2022/2/2, use just the expression in else clause
-                    if values and isinstance(values[0], int):
-                        # Backwards compatibility. Reset setting if we detect
-                        # that the number of values decreased. Still broken if
-                        # they're reordered or we don't detect the decrease.
-                        #
-                        # indices start with 1, thus >, not >=
-                        if max(values) > len(attr.values):
-                            values = (0, )
-                    else:
-                        values = tuple(attr.to_val(val) + 1 if val else 0
-                                       for val in values if val in attr.values) \
-                                 or (0, )
+                    values = tuple(
+                        attr.to_val(val) + 1 if val else 0
+                        for val in values
+                        if val in attr.values
+                    ) or (0,)
                 value[i] = (attr, op, values)
         return value
 
@@ -109,10 +97,10 @@ def match(self, context, domain, attrs, metas):
         conditions = context.values["conditions"]
         all_vars = attrs.copy()
         all_vars.update(metas)
-        matched = [all_vars.get(name) == tpe  # also matches "all (...)" strings
-                   # After 2022/2/2 remove this line:
-                   if len(rest) == 2 else name in all_vars
-                   for name, tpe, *rest in conditions]
+        matched = [
+            all_vars.get(name) == tpe  # also matches "all (...)" strings
+            for name, tpe, *rest in conditions
+        ]
         if any(matched):
             return 0.5 * sum(matched) / len(matched)
         return self.NO_MATCH
@@ -125,15 +113,11 @@ def filter_value(self, setting, data, domain, attrs, metas):
         all_vars = attrs.copy()
         all_vars.update(metas)
         conditions = data["conditions"]
-        # Use this after 2022/2/2: if any(all_vars.get(name) == tpe:
-        # conditions[:] = [(name, tpe, *rest) for name, tpe, *rest in conditions
-        #                  if all_vars.get(name) == tpe]
         conditions[:] = [
-            (name, tpe, *rest) for name, tpe, *rest in conditions
-            # all_vars.get(name) == tpe also matches "all (...)" which are
-            # encoded with type `None`
-            if (all_vars.get(name) == tpe if len(rest) == 2
-                else name in all_vars)]
+            (name, tpe, *rest)
+            for name, tpe, *rest in conditions
+            if all_vars.get(name) == tpe
+        ]
 
 
 class FilterDiscreteType(enum.Enum):
@@ -779,13 +763,11 @@ def send_report(self):
                  ("Non-matching data",
                   nonmatch_inst > 0 and "{} instances".format(nonmatch_inst))))
 
-    # Uncomment this on 2022/2/2
-    #
-    # @classmethod
-    # def migrate_context(cls, context, version):
-    #     if not version or version < 2:
-    #         # Just remove; can't migrate because variables types are unknown
-    #         context.values["conditions"] = []
+    @classmethod
+    def migrate_context(cls, context, version):
+        if not version or version < 2:
+            # Just remove; can't migrate because variables types are unknown
+            context.values["conditions"] = []
 
 
 class CheckBoxPopup(QWidget):

Orange/widgets/data/tests/test_owselectrows.py

@@ -310,37 +310,6 @@ def test_partial_match_values(self):
         self.assertEqual(condition[1], 2)
         self.assertEqual(condition[2], (2, ))  # index of value + 1
 
-    def test_backward_compat_match_values(self):
-        iris = Table("iris")
-        domain = iris.domain
-        class_var = domain.class_var
-        self.widget = self.widget_with_context(
-            domain, [[class_var.name, 1, 2, (1, 2)]])
-
-        new_class_var = DiscreteVariable(class_var.name, class_var.values[1:])
-        new_domain = Domain(domain.attributes, new_class_var)
-        non0 = iris.Y != 0
-        iris2 = Table.from_numpy(new_domain, iris.X[non0], iris.Y[non0] - 1)
-        self.send_signal(self.widget.Inputs.data, iris2)
-        condition = self.widget.conditions[0]
-        self.assertIs(condition[0], new_class_var)
-        self.assertEqual(condition[1], 2)
-        self.assertEqual(condition[2], (1, 2))  # index of value + 1
-
-        # reset to [0] if out of range
-        self.widget = self.widget_with_context(
-            domain, [[class_var.name, 1, 2, (1, 3)]])
-
-        new_class_var = DiscreteVariable(class_var.name, class_var.values[1:])
-        new_domain = Domain(domain.attributes, new_class_var)
-        non0 = iris.Y != 0
-        iris2 = Table.from_numpy(new_domain, iris.X[non0], iris.Y[non0] - 1)
-        self.send_signal(self.widget.Inputs.data, iris2)
-        condition = self.widget.conditions[0]
-        self.assertIs(condition[0], new_class_var)
-        self.assertEqual(condition[1], 2)
-        self.assertEqual(condition[2], (0, ))  # index of value + 1
-
     @override_locale(QLocale.C)
     def test_partial_matches_with_missing_vars(self):
         iris = Table("iris")
@@ -532,43 +501,15 @@ def test_report(self, _):
         self.enterFilter(zoo.domain[1], "is one of")
         self.widget.send_report()  # don't crash
 
-    # Uncomment this on 2022/2/2
-    #
-    # def test_migration_to_version_1(self):
-    #     iris = Table("iris")
-    #
-    #     ch = SelectRowsContextHandler()
-    #     context = ch.new_context(iris.domain, *ch.encode_domain(iris.domain))
-    #     context.values = dict(conditions=[["petal length", 2, (5.2,)]])
-    #     settings = dict(context_settings=[context])
-    #     widget = self.create_widget(OWSelectRows, settings)
-    #     self.assertEqual(widget.conditions, [])
-
-    @override_locale(QLocale.C)
-    def test_support_old_settings(self):
+    def test_migration_to_version_1(self):
         iris = Table("iris")
-        self.widget = self.widget_with_context(
-            iris.domain, [["sepal length", 2, ("5.2",)]])
-        self.send_signal(self.widget.Inputs.data, iris)
-        condition = self.widget.conditions[0]
-        self.assertEqual(condition[0], iris.domain["sepal length"])
-        self.assertEqual(condition[1], 2)
-        self.assertTrue(condition[2][0].startswith("5.2"))
-
-    def test_end_support_for_version_1(self):
-        if time.gmtime() >= (2022, 2, 2):
-            self.fail("""
-Happy 22/2/2!
-
-Now remove support for version==None settings in
-SelectRowsContextHandler.decode_setting and SelectRowsContextHandler.match,
-and uncomment OWSelectRows.migrate.
-
-In tests, uncomment test_migration_to_version_1,
-and remove test_support_old_settings and this test.
 
-Basically, revert this commit.
-""")
+        ch = SelectRowsContextHandler()
+        context = ch.new_context(iris.domain, *ch.encode_domain(iris.domain))
+        context.values = dict(conditions=[["petal length", 2, (5.2,)]])
+        settings = dict(context_settings=[context])
+        widget = self.create_widget(OWSelectRows, settings)
+        self.assertEqual(widget.conditions, [])
 
     def test_purge_discretized(self):
         housing = Table("housing")
@@ -577,7 +518,9 @@ def test_purge_discretized(self):
             discretize_class=True, method=method)
         domain = discretizer(housing)
         data = housing.transform(domain)
-        widget = self.widget_with_context(domain, [["MEDV", 101, 2, (2, 3)]])
+        widget = self.widget_with_context(
+            domain, [["MEDV", 101, 2, domain.class_var.values[1:]]]
+        )
         widget.purge_classes = True
         self.send_signal(widget.Inputs.data, data)
         out = self.get_output(widget.Outputs.matching_data)