Merge pull request #4741 from janezd/featurestatistics-var-settings

[FIX] Feature Statistics: Fix wrong or even crashing selections

Author: markotoplak

Orange/widgets/data/owfeaturestatistics.py

@@ -8,6 +8,7 @@
 
 import datetime
 from enum import IntEnum
+from itertools import chain
 from typing import Any, Optional, Tuple, List
 
 import numpy as np
@@ -24,7 +25,8 @@
     ContinuousVariable, TimeVariable, Domain, Variable
 from Orange.widgets import widget, gui
 from Orange.widgets.data.utils.histogram import Histogram
-from Orange.widgets.settings import ContextSetting, DomainContextHandler
+from Orange.widgets.settings import Setting, ContextSetting, \
+    DomainContextHandler
 from Orange.widgets.utils.itemmodels import DomainModel, AbstractSortTableModel
 from Orange.widgets.utils.signals import Input, Output
 from Orange.widgets.utils.widgetpreview import WidgetPreview
@@ -685,13 +687,14 @@ class Outputs:
     buttons_area_orientation = Qt.Vertical
 
     settingsHandler = DomainContextHandler()
+    settings_version = 2
 
-    auto_commit = ContextSetting(True)
+    auto_commit = Setting(True)
     color_var = ContextSetting(None)  # type: Optional[Variable]
     # filter_string = ContextSetting('')
 
-    sorting = ContextSetting((0, Qt.DescendingOrder))
-    selected_rows = ContextSetting([])
+    sorting = Setting((0, Qt.DescendingOrder))
+    selected_vars = ContextSetting([], schema_only=True)
 
     def __init__(self):
         super().__init__()
@@ -753,7 +756,7 @@ def _filter_table_variables(self):
     def set_data(self, data):
         # Clear outputs and reset widget state
         self.closeContext()
-        self.selected_rows = []
+        self.selected_vars = []
         self.model.resetSorting()
         self.Outputs.reduced_data.send(None)
         self.Outputs.statistics.send(None)
@@ -786,10 +789,10 @@ def __restore_selection(self):
         """Restore the selection on the table view from saved settings."""
         selection_model = self.table_view.selectionModel()
         selection = QItemSelection()
-        # self.selected_rows can be list or numpy.array, thus
-        # pylint: disable=len-as-condition
-        if len(self.selected_rows):
-            for row in self.model.mapFromSourceRows(self.selected_rows):
+        if self.selected_vars:
+            var_indices = {var: i for i, var in enumerate(self.model.variables)}
+            selected_indices = [var_indices[var] for var in self.selected_vars]
+            for row in self.model.mapFromSourceRows(selected_indices):
                 selection.append(QItemSelectionRange(
                     self.model.index(row, 0),
                     self.model.index(row, self.model.columnCount() - 1)
@@ -816,22 +819,21 @@ def __color_var_changed(self, *_):
             self.model.set_target_var(self.color_var)
 
     def on_select(self):
-        self.selected_rows = list(self.model.mapToSourceRows([
+        selection_indices = list(self.model.mapToSourceRows([
             i.row() for i in self.table_view.selectionModel().selectedRows()
         ]))
+        self.selected_vars = list(self.model.variables[selection_indices])
         self.commit()
 
     def commit(self):
-        # self.selected_rows can be list or numpy.array, thus
-        # pylint: disable=len-as-condition
-        if not len(self.selected_rows):
+        if not self.selected_vars:
             self.info.set_output_summary(self.info.NoOutput)
             self.Outputs.reduced_data.send(None)
             self.Outputs.statistics.send(None)
             return
 
         # Send a table with only selected columns to output
-        variables = self.model.variables[self.selected_rows]
+        variables = self.selected_vars
         self.info.set_output_summary(len(self.data[:, variables]),
                                      format_summary_details(self.data[:, variables]))
         self.Outputs.reduced_data.send(self.data[:, variables])
@@ -850,6 +852,26 @@ def commit(self):
     def send_report(self):
         pass
 
+    @classmethod
+    def migrate_context(cls, context, version):
+        if not version or version < 2:
+            selected_rows = context.values.pop("selected_rows", None)
+            if not selected_rows:
+                selected_vars = []
+            else:
+                # This assumes that dict was saved by Python >= 3.6 so dict is
+                # ordered; if not, context hasn't had worked anyway.
+                all_vars = [
+                    (var, tpe)
+                    for (var, tpe) in chain(context.attributes.items(),
+                                            context.metas.items())
+                    # it would be nicer to use cls.HIDDEN_VAR_TYPES, but there
+                    # is no suitable conversion function, and StringVariable (3)
+                    # was the only hidden var when settings_version < 2, so:
+                    if tpe != 3]
+                selected_vars = [all_vars[i] for i in selected_rows]
+            context.values["selected_vars"] = selected_vars, -3
+
 
 if __name__ == '__main__':  # pragma: no cover
     WidgetPreview(OWFeatureStatistics).run(Table("iris"))

Orange/widgets/data/tests/test_owfeaturestatistics.py

@@ -18,6 +18,7 @@
 from Orange.widgets.data.owfeaturestatistics import \
     OWFeatureStatistics
 from Orange.widgets.utils.state_summary import format_summary_details
+from orangewidget.settings import Context
 
 VarDataPair = namedtuple('VarDataPair', ['variable', 'data'])
 
@@ -454,17 +455,44 @@ def setUp(self):
     def test_restores_previous_selection(self):
         """Widget should remember selection with domain context handler."""
         # Send data and select rows
+        domain1 = self.data1.domain
         self.send_signal(self.widget.Inputs.data, self.data1)
         self.select_rows([0, 2])
-        self.assertEqual(len(self.widget.selected_rows), 2)
+        self.assertEqual(set(self.widget.selected_vars),
+                         {domain1[0], domain1[2]})
 
         # Sending new data clears selection
         self.send_signal(self.widget.Inputs.data, self.data2)
-        self.assertEqual(len(self.widget.selected_rows), 0)
+        self.assertEqual(len(self.widget.selected_vars), 0)
 
         # Sending back the old data restores the selection
-        self.send_signal(self.widget.Inputs.data, self.data1)
-        self.assertEqual(len(self.widget.selected_rows), 2)
+        iris3 = self.data1.transform(
+            Domain([domain1[2], domain1[0], domain1[1]], domain1.class_var))
+        self.send_signal(self.widget.Inputs.data, iris3)
+        self.assertEqual(set(self.widget.selected_vars),
+                         {domain1[0], domain1[2]})
+
+    def test_settings_migration_to_ver21(self):
+        settings = {
+            'controlAreaVisible': True, 'savedWidgetGeometry': '',
+            '__version__': 1,
+            'context_settings': [
+                Context(
+                    values={'auto_commit': (True, -2),
+                            'color_var': ('iris', 101),
+                            'selected_rows': [1, 4],
+                            'sorting': ((1, 0), -2), '__version__': 1},
+                    attributes={'petal length': 2, 'petal width': 2,
+                                'sepal length': 2, 'sepal width': 2},
+                    metas={'iris': 1})]
+        }
+        widget = self.create_widget(OWFeatureStatistics,
+                                    stored_settings=settings)
+        self.send_signal(widget.Inputs.data, self.data1)
+        domain = self.data1.domain
+        self.assertEqual(widget.selected_vars, [domain["petal width"],
+                                                domain["iris"]])
+
 
 class TestSummary(WidgetTest):
     def setUp(self):