SOM: Fix crash when color is constantly nan

Orange/widgets/unsupervised/owsom.py

@@ -1,4 +1,5 @@
 from collections import defaultdict, namedtuple
+from typing import Optional
 from xml.sax.saxutils import escape
 
 import numpy as np
@@ -26,7 +27,7 @@
 from Orange.widgets.utils.annotated_data import \
     create_annotated_table, create_groups_table, ANNOTATED_DATA_SIGNAL_NAME
 from Orange.widgets.utils.colorpalettes import \
-    BinnedContinuousPalette, LimitedDiscretePalette
+    BinnedContinuousPalette, LimitedDiscretePalette, DiscretePalette
 from Orange.widgets.visualize.utils import CanvasRectangle, CanvasText
 from Orange.widgets.visualize.utils.plotutils import wrap_legend_items
 
@@ -210,6 +211,8 @@ class Warning(OWWidget.Warning):
         ignoring_disc_variables = Msg("SOM ignores categorical variables.")
         missing_colors = \
             Msg("Some data instances have undefined value of '{}'.")
+        no_defined_colors = \
+            Msg("'{}' has node defined values.")
         missing_values = \
             Msg("{} data instance{} with undefined value(s) {} not shown.")
         single_attribute = Msg("Data contains a single numeric column.")
@@ -228,7 +231,12 @@ def __init__(self):
         self.data = self.cont_x = None
         self.cells = self.member_data = None
         self.selection = None
-        self.colors = self.thresholds = self.bin_labels = None
+
+        # self.colors holds a palette or None when we need to draw same-colored
+        # circles. This happens by user's choice or when the color attribute
+        # is numeric and has no defined values, so we can't construct bins
+        self.colors: Optional[DiscretePalette] = None
+        self.thresholds = self.bin_labels = None
 
         box = gui.vBox(self.controlArea, box="SOM")
         shape = gui.comboBox(
@@ -536,7 +544,7 @@ def _redraw(self):
 
         self.elements = QGraphicsItemGroup()
         self.scene.addItem(self.elements)
-        if self.attr_color is None:
+        if self.colors is None:
             self._draw_same_color(sizes)
         elif self.pie_charts:
             self._draw_pie_charts(sizes)
@@ -562,6 +570,10 @@ def _draw_same_color(self, sizes):
                 self.elements.addToGroup(ellipse)
 
     def _get_color_column(self):
+        # if self.colors is None, we use _draw_same_color and don't call
+        # this function
+        assert self.colors is not None
+
         color_column = \
             self.data.get_column_view(self.attr_color)[0].astype(float,
                                                                  copy=False)
@@ -571,10 +583,7 @@ def _get_color_column(self):
             int_col[np.isnan(color_column)] = len(self.colors)
         else:
             int_col = np.zeros(len(color_column), dtype=int)
-            # The following line is unnecessary because rows with missing
-            # numeric data are excluded. Uncomment it if you change SOM to
-            # tolerate missing values.
-            # int_col[np.isnan(color_column)] = len(self.colors)
+            int_col[np.isnan(color_column)] = len(self.colors)
             for i, thresh in enumerate(self.thresholds, start=1):
                 int_col[color_column >= thresh] = i
         return int_col
@@ -584,6 +593,7 @@ def _tooltip(self, colors, distribution):
             values = self.attr_color.values
         else:
             values = self._bin_names()
+        values = list(values) + ["(N/A)"]
         tot = np.sum(distribution)
         nbhp = "\N{NON-BREAKING HYPHEN}"
         return '<table style="white-space: nowrap">' + "".join(f"""
@@ -600,6 +610,8 @@ def _tooltip(self, colors, distribution):
             + "</table>"
 
     def _draw_pie_charts(self, sizes):
+        assert self.colors is not None  # if it were, we'd call _draw_same_color
+
         fx, fy = self._grid_factors
         color_column = self._get_color_column()
         colors = self.colors.qcolors_w_nan
@@ -619,6 +631,8 @@ def _draw_pie_charts(self, sizes):
                 pie.setPos(x + (y % 2) * fx, y * fy)
 
     def _draw_colored_circles(self, sizes):
+        assert self.colors is not None  # if it were, we'd call _draw_same_color
+
         fx, fy = self._grid_factors
         color_column = self._get_color_column()
         qcolors = self.colors.qcolors_w_nan
@@ -820,39 +834,50 @@ def update_output(self):
         self.Outputs.annotated_data.send(annotated)
 
     def set_color_bins(self):
+        self.Warning.no_defined_colors.clear()
+
         if self.attr_color is None:
             self.thresholds = self.bin_labels = self.colors = None
-        elif self.attr_color.is_discrete:
+            return
+
+        if self.attr_color.is_discrete:
             self.thresholds = self.bin_labels = None
             self.colors = self.attr_color.palette
+            return
+
+        col = self.data.get_column_view(self.attr_color)[0].astype(float)
+        col = col[np.isfinite(col)]
+        if not col.size:
+            self.Warning.no_defined_colors(self.attr_color)
+            self.thresholds = self.bin_labels = self.colors = None
+            return
+
+        if self.attr_color.is_time:
+            binning = time_binnings(col, min_bins=4)[-1]
         else:
-            col = self.data.get_column_view(self.attr_color)[0].astype(float)
-            if self.attr_color.is_time:
-                binning = time_binnings(col, min_bins=4)[-1]
-            else:
-                binning = decimal_binnings(col, min_bins=4)[-1]
-            self.thresholds = binning.thresholds[1:-1]
-            self.bin_labels = (binning.labels[1:-1], binning.short_labels[1:-1])
-            if not self.bin_labels[0] and binning.labels:
-                # Nan's are already filtered out, but it doesn't hurt much
-                # to use nanmax/nanmin
-                if np.nanmin(col) == np.nanmax(col):
-                    # Handle a degenerate case with a single value
-                    # Use the second threshold (because value must be smaller),
-                    # but the first threshold as label (because that's the
-                    # actual value in the data.
-                    self.thresholds = binning.thresholds[1:]
-                    self.bin_labels = (binning.labels[:1],
-                                       binning.short_labels[:1])
-            palette = BinnedContinuousPalette.from_palette(
-                self.attr_color.palette, binning.thresholds)
-            self.colors = palette
+            binning = decimal_binnings(col, min_bins=4)[-1]
+        self.thresholds = binning.thresholds[1:-1]
+        self.bin_labels = (binning.labels[1:-1], binning.short_labels[1:-1])
+        if not self.bin_labels[0] and binning.labels:
+            # Nan's are already filtered out, but it doesn't hurt much
+            # to use nanmax/nanmin
+            if np.nanmin(col) == np.nanmax(col):
+                # Handle a degenerate case with a single value
+                # Use the second threshold (because value must be smaller),
+                # but the first threshold as label (because that's the
+                # actual value in the data.
+                self.thresholds = binning.thresholds[1:]
+                self.bin_labels = (binning.labels[:1],
+                                   binning.short_labels[:1])
+        palette = BinnedContinuousPalette.from_palette(
+            self.attr_color.palette, binning.thresholds)
+        self.colors = palette
 
     def create_legend(self):
         if self.legend is not None:
             self.scene.removeItem(self.legend)
             self.legend = None
-        if self.attr_color is None:
+        if self.colors is None:
             return
 
         if self.attr_color.is_discrete:
@@ -881,7 +906,7 @@ def create_legend(self):
         self.set_legend_pos()
 
     def _bin_names(self):
-        labels, short_labels = self.bin_labels
+        labels, short_labels = self.bin_labels or ([], [])
         if len(labels) <= 1:
             return labels
         return \
@@ -898,7 +923,7 @@ def set_legend_pos(self):
 
     def send_report(self):
         self.report_plot()
-        if self.attr_color:
+        if self.colors:
             self.report_caption(
                 f"Self-organizing map colored by '{self.attr_color.name}'")
 

Orange/widgets/unsupervised/tests/test_owsom.py

@@ -222,12 +222,40 @@ def test_attr_color_change(self):
         widget._redraw.assert_called()
 
     def test_colored_circles_with_constant(self):
+        domain = self.iris.domain
+        self.widget.pie_charts = False
+
         with self.iris.unlocked():
             self.iris.X[:, 0] = 1
         self.send_signal(self.widget.Inputs.data, self.iris)
+        attr0 = domain.attributes[0]
+
         combo = self.widget.controls.attr_color
-        simulate.combobox_activate_index(
-            combo, combo.model().indexOf(self.iris.domain.attributes[0]))
+        simulate.combobox_activate_index(combo, combo.model().indexOf(attr0))
+        self.assertIsNotNone(self.widget.colors)
+        self.assertFalse(self.widget.Warning.no_defined_colors.is_shown())
+
+        dom1 = Domain(domain.attributes[1:], domain.class_var,
+                      domain.attributes[:1])
+        iris = self.iris.transform(dom1).copy()
+        with iris.unlocked(iris.metas):
+            iris.metas[::2, 0] = np.nan
+        self.send_signal(self.widget.Inputs.data, iris)
+        simulate.combobox_activate_index(combo, combo.model().indexOf(attr0))
+        self.assertIsNotNone(self.widget.colors)
+        self.assertFalse(self.widget.Warning.no_defined_colors.is_shown())
+
+        iris = self.iris.transform(dom1).copy()
+        with iris.unlocked(iris.metas):
+            iris.metas[:, 0] = np.nan
+        self.send_signal(self.widget.Inputs.data, iris)
+        simulate.combobox_activate_index(combo, combo.model().indexOf(attr0))
+        self.assertIsNone(self.widget.colors)
+        self.assertTrue(self.widget.Warning.no_defined_colors.is_shown())
+
+        simulate.combobox_activate_index(combo, 0)
+        self.assertIsNone(self.widget.colors)
+        self.assertFalse(self.widget.Warning.no_defined_colors.is_shown())
 
     @_patch_recompute_som
     def test_cell_sizes(self):