Merge pull request #2915 from pavlin-policar/kmeans-fixes

[FIX] KMeans clear results on k change, do not recluster

Author: lanzagar

Orange/widgets/unsupervised/owkmeans.py

@@ -136,8 +136,8 @@ def migrate_settings(cls, settings, version):
         # type: (Dict, int) -> None
         if version < 2:
             if 'auto_apply' in settings:
-                settings['auto_commit'] = settings['auto_apply']
-                del settings['auto_apply']
+                settings['auto_commit'] = settings.get('auto_apply', True)
+                settings.pop('auto_apply', None)
 
     def __init__(self):
         super().__init__()
@@ -151,11 +151,7 @@ def __init__(self):
         layout = QGridLayout()
         bg = gui.radioButtonsInBox(
             self.controlArea, self, "optimize_k", orientation=layout,
-            box="Number of Clusters",
-            # Because commit is only wrapped when creating the auto_commit
-            # buttons, we can't pass it as the callback here, so we can add
-            # this hacky lambda to call the wrapped commit when necessary
-            callback=lambda: self.commit(),
+            box="Number of Clusters", callback=self.update_method,
         )
 
         layout.addWidget(
@@ -229,18 +225,25 @@ def adjustSize(self):
         s = self.sizeHint()
         self.resize(s)
 
+    def update_method(self):
+        self.table_model.clear_scores()
+        self.commit()
+
     def update_k(self):
         self.optimize_k = False
+        self.table_model.clear_scores()
         self.commit()
 
     def update_from(self):
         self.k_to = max(self.k_from + 1, self.k_to)
         self.optimize_k = True
+        self.table_model.clear_scores()
         self.commit()
 
     def update_to(self):
         self.k_from = min(self.k_from, self.k_to - 1)
         self.optimize_k = True
+        self.table_model.clear_scores()
         self.commit()
 
     def enough_data_instances(self, k):
@@ -395,17 +398,14 @@ def commit(self):
 
         QTimer.singleShot(100, self.adjustSize)
 
-    def invalidate(self, force=False):
+    def invalidate(self):
         self.cancel()
         self.Error.clear()
         self.Warning.clear()
         self.clusterings = {}
         self.table_model.clear_scores()
 
-        if force:
-            self.unconditional_commit()
-        else:
-            self.commit()
+        self.commit()
 
     def update_results(self):
         scores = [
@@ -437,7 +437,7 @@ def send_data(self):
             k = self.k
 
         km = self.clusterings.get(k)
-        if not self.data or km is None or isinstance(km, str):
+        if self.data is None or km is None or isinstance(km, str):
             self.Outputs.annotated_data.send(None)
             self.Outputs.centroids.send(None)
             return
@@ -469,8 +469,14 @@ def send_data(self):
     @Inputs.data
     @check_sql_input
     def set_data(self, data):
-        self.data = data
-        self.invalidate()
+        self.data, old_data = data, self.data
+
+        # Do not needlessly recluster the data if X hasn't changed
+        if old_data and self.data and np.array_equal(self.data.X, old_data.X):
+            if self.auto_commit:
+                self.send_data()
+        else:
+            self.invalidate()
 
     def send_report(self):
         # False positives (Setting is not recognized as int)

Orange/widgets/unsupervised/tests/test_owkmeans.py

@@ -8,7 +8,7 @@
 from AnyQt.QtWidgets import QRadioButton
 
 import Orange.clustering
-from Orange.data import Table
+from Orange.data import Table, Domain
 from Orange.widgets import gui
 from Orange.widgets.tests.base import WidgetTest
 from Orange.widgets.unsupervised.owkmeans import OWKMeans, ClusterTableModel
@@ -377,6 +377,42 @@ def test_invalidate_clusterings_cancels_jobs(self):
 
         self.assertEqual(widget.clusterings, {})
 
+    def test_do_not_recluster_on_same_data(self):
+        """Do not recluster data points when targets or metas change."""
+
+        # Prepare some dummy data
+        x = np.eye(5)
+        y1, y2 = np.ones((5, 1)), np.ones((5, 2))
+        meta1, meta2 = np.ones((5, 1)), np.ones((5, 2))
+
+        table1 = Table.from_numpy(
+            domain=Domain.from_numpy(X=x, Y=y1, metas=meta1),
+            X=x, Y=y1, metas=meta1,
+        )
+        # X is same, should not cause update
+        table2 = Table.from_numpy(
+            domain=Domain.from_numpy(X=x, Y=y2, metas=meta2),
+            X=x, Y=y2, metas=meta2,
+        )
+        # X is different, should cause update
+        table3 = table1.copy()
+        table3.X[:, 0] = 1
+
+        with patch.object(self.widget, 'commit') as commit:
+            self.send_signal(self.widget.Inputs.data, table1)
+            self.commit_and_wait()
+            call_count = commit.call_count
+
+            # Sending data with same X should not recompute the clustering
+            self.send_signal(self.widget.Inputs.data, table2)
+            self.commit_and_wait()
+            self.assertEqual(call_count, commit.call_count)
+
+            # Sending data with different X should recompute the clustering
+            self.send_signal(self.widget.Inputs.data, table3)
+            self.commit_and_wait()
+            self.assertEqual(call_count + 1, commit.call_count)
+
 
 if __name__ == "__main__":
     unittest.main()