Merge pull request #4120 from PrimozGodec/limit-discrete-variable

[FIX] Warning for discrete variable with >100 values in OWFile

Author: janezd

Orange/data/tests/test_io.py

@@ -0,0 +1,70 @@
+import unittest
+import numpy as np
+
+from Orange.data import ContinuousVariable, DiscreteVariable, StringVariable
+from Orange.data.io import guess_data_type
+
+
+class TestTableFilters(unittest.TestCase):
+
+    def test_guess_data_type_continuous(self):
+        # should be ContinuousVariable
+        valuemap, values, coltype = guess_data_type(list(range(1, 100)))
+        self.assertEqual(ContinuousVariable, coltype)
+        self.assertIsNone(valuemap)
+        np.testing.assert_array_equal(np.array(list(range(1, 100))), values)
+
+        valuemap, values, coltype = guess_data_type([1, 2, 3, 1, 2, 3])
+        self.assertEqual(ContinuousVariable, coltype)
+        self.assertIsNone(valuemap)
+        np.testing.assert_array_equal([1, 2, 3, 1, 2, 3], values)
+
+        valuemap, values, coltype = guess_data_type(
+            ["1", "2", "3", "1", "2", "3"])
+        self.assertEqual(ContinuousVariable, coltype)
+        self.assertIsNone(valuemap)
+        np.testing.assert_array_equal([1, 2, 3, 1, 2, 3], values)
+
+    def test_guess_data_type_discrete(self):
+        # should be DiscreteVariable
+        valuemap, values, coltype = guess_data_type([1, 2, 1, 2])
+        self.assertEqual(DiscreteVariable, coltype)
+        self.assertEqual([1, 2], valuemap)
+        np.testing.assert_array_equal([1, 2, 1, 2], values)
+
+        valuemap, values, coltype = guess_data_type(["1", "2", "1", "2", "a"])
+        self.assertEqual(DiscreteVariable, coltype)
+        self.assertEqual(["1", "2", "a"], valuemap)
+        np.testing.assert_array_equal(['1', '2', '1', '2', 'a'], values)
+
+        # just below the threshold for string variable
+        in_values = list(map(lambda x: str(x) + "a", range(24))) + ["a"] * 76
+        valuemap, values, coltype = guess_data_type(in_values)
+        self.assertEqual(DiscreteVariable, coltype)
+        self.assertEqual(sorted(set(in_values)), valuemap)
+        np.testing.assert_array_equal(in_values, values)
+
+    def test_guess_data_type_string(self):
+        # should be StringVariable
+        # too many different values for discrete
+        in_values = list(map(lambda x: str(x) + "a", range(90)))
+        valuemap, values, coltype = guess_data_type(in_values)
+        self.assertEqual(StringVariable, coltype)
+        self.assertIsNone(valuemap)
+        np.testing.assert_array_equal(in_values, values)
+
+        # more than len(values)**0.7
+        in_values = list(map(lambda x: str(x) + "a", range(25))) + ["a"] * 75
+        valuemap, values, coltype = guess_data_type(in_values)
+        self.assertEqual(StringVariable, coltype)
+        self.assertIsNone(valuemap)
+        np.testing.assert_array_equal(in_values, values)
+
+        # more than 100 different values - exactly 101
+        # this is the case when len(values)**0.7 rule would vote for the
+        # DiscreteVariable
+        in_values = list(map(lambda x: str(x) + "a", range(100))) + ["a"] * 999
+        valuemap, values, coltype = guess_data_type(in_values)
+        self.assertEqual(StringVariable, coltype)
+        self.assertIsNone(valuemap)
+        np.testing.assert_array_equal(in_values, values)

Orange/data/variable.py

@@ -26,6 +26,8 @@
 
 DISCRETE_MAX_VALUES = 3  # == 2 + nan
 MAX_NUM_OF_DECIMALS = 5
+# the variable with more than 100 different values should not be StringVariable
+DISCRETE_MAX_ALLOWED_VALUES = 100
 
 
 def make_variable(cls, compute_value, *args):
@@ -63,7 +65,8 @@ def is_discrete_values(values):
     unique = set()
     for i in values:
         unique.add(i)
-        if len(unique) > max_values:
+        if (len(unique) > max_values or
+                len(unique) > DISCRETE_MAX_ALLOWED_VALUES):
             return False
 
     # Strip NaN from unique

Orange/widgets/data/owfile.py

@@ -1,5 +1,6 @@
 import os
 import logging
+from itertools import chain
 from warnings import catch_warnings
 from urllib.parse import urlparse
 from typing import List
@@ -123,6 +124,8 @@ class Warning(widget.OWWidget.Warning):
         file_too_big = widget.Msg("The file is too large to load automatically."
                                   " Press Reload to load.")
         load_warning = widget.Msg("Read warning:\n{}")
+        performance_warning = widget.Msg(
+            "Categorical variables with >100 values may decrease performance.")
 
     class Error(widget.OWWidget.Error):
         file_not_found = widget.Msg("File not found.")
@@ -468,7 +471,13 @@ def reset_domain_edit(self):
         self.domain_editor.reset_domain()
         self.apply_domain_edit()
 
+    def _inspect_discrete_variables(self, domain):
+        for var in chain(domain.variables, domain.metas):
+            if var.is_discrete and len(var.values) > 100:
+                self.Warning.performance_warning()
+
     def apply_domain_edit(self):
+        self.Warning.performance_warning.clear()
         if self.data is None:
             table = None
         else:
@@ -481,6 +490,7 @@ def apply_domain_edit(self):
                 table.name = self.data.name
                 table.ids = np.array(self.data.ids)
                 table.attributes = getattr(self.data, 'attributes', {})
+                self._inspect_discrete_variables(domain)
 
         self.Outputs.data.send(table)
         self.apply_button.setEnabled(False)