Merge pull request #4818 from PrimozGodec/remove-ordered

Discrete variable: remove ordered attribute

Author: janezd

Orange/data/io_base.py

@@ -288,8 +288,7 @@ def _disc_with_vals_column(data: np.ndarray, col: int,
                                values="", **_) -> _ColumnProperties:
         vals, coltype = _TableBuilder._disc_column(data, col)
         return _ColumnProperties(valuemap=Flags.split(values), values=vals,
-                                 coltype=coltype, orig_values=vals,
-                                 coltype_kwargs={"ordered": True})
+                                 coltype=coltype, orig_values=vals)
 
     @staticmethod
     def _unknown_column(data: np.ndarray, col: int, **_) -> _ColumnProperties:
@@ -607,8 +606,12 @@ def _vartype(var):
             if var.is_continuous or var.is_string:
                 return var.TYPE_HEADERS[0]
             elif var.is_discrete:
-                return Flags.join(var.values) if var.ordered else \
-                    var.TYPE_HEADERS[0]
+                # if number of values is 1 order is not important if more
+                # values write order in file
+                return (
+                    Flags.join(var.values) if len(var.values) >= 2
+                    else var.TYPE_HEADERS[0]
+                )
             raise NotImplementedError
 
         return ['continuous'] * data.has_weights() + \

Orange/data/pandas_compat.py

@@ -97,8 +97,9 @@ def _column_to_series(col, vals):
         result = ()
         if col.is_discrete:
             codes = pd.Series(vals).fillna(-1).astype(int)
-            result = (col.name, pd.Categorical.from_codes(codes=codes, categories=col.values,
-                                                          ordered=col.ordered))
+            result = (col.name, pd.Categorical.from_codes(
+                codes=codes, categories=col.values, ordered=True
+            ))
         elif col.is_time:
             result = (col.name, pd.to_datetime(vals, unit='s').to_series().reset_index()[0])
         elif col.is_continuous:

Orange/data/tests/test_io_base.py

@@ -128,7 +128,7 @@ def test_column_parts_discrete_values(self):
         np.testing.assert_array_equal(column.orig_values,
                                       ["red", "red", "green"])
         self.assertEqual(column.coltype, DiscreteVariable)
-        self.assertDictEqual(column.coltype_kwargs, {'ordered': True})
+        self.assertDictEqual(column.coltype_kwargs, {})
 
     def test_unknown_type_column(self):
         data = np.array(self.header0)

Orange/data/tests/test_variable.py

@@ -279,15 +279,12 @@ def test_repr(self):
         self.assertEqual(
             repr(var),
             "DiscreteVariable(name='a', values=('F', 'M'))")
-        var.ordered = True
-        self.assertEqual(
-            repr(var),
-            "DiscreteVariable(name='a', values=('F', 'M'), ordered=True)")
 
         var = DiscreteVariable.make("a", values="1234567")
         self.assertEqual(
             repr(var),
-            "DiscreteVariable(name='a', values=('1', '2', '3', '4', '5', '6', '7'))")
+            "DiscreteVariable(name='a', values=('1', '2', '3', '4', '5', '6', '7'))"
+        )
 
     def test_no_nonstringvalues(self):
         self.assertRaises(TypeError, DiscreteVariable, "foo", values=("a", 42))
@@ -488,7 +485,6 @@ def varcls_modified(self, name):
         var = super().varcls_modified(name)
         var.add_value("A")
         var.add_value("B")
-        var.ordered = True
         return var
 
     def test_copy_checks_len_values(self):
@@ -508,6 +504,16 @@ def test_copy_checks_len_values(self):
         var2 = var.copy(values=("W", "M"))
         self.assertEqual(var2.values, ("W", "M"))
 
+    def test_remove_ordered(self):
+        """
+        ordered is deprecated when this test starts to fail remove ordered
+        parameter. Remove also this test.
+        Ordered parameter should still be allowed in __init__ for backward
+        compatibilities in data-sets pickled with older versions, I suggest
+        adding **kwargs which is ignored
+        """
+        self.assertLess(Orange.__version__, "3.29.0")
+
 
 @variabletest(ContinuousVariable)
 class TestContinuousVariable(VariableTest):
@@ -697,10 +703,7 @@ def varcls_modified(self, name):
     "PickleDiscreteVariable",
     ("with_name", lambda: DiscreteVariable(name="Feature 0")),
     ("with_str_value", lambda: DiscreteVariable(name="Feature 0",
-                                                values=("F", "M"))),
-    ("ordered", lambda: DiscreteVariable(name="Feature 0",
-                                         values=("F", "M"),
-                                         ordered=True)),
+                                                values=("F", "M")))
 )
 
 
@@ -712,7 +715,7 @@ def varcls_modified(self, name):
 
 class VariableTestMakeProxy(unittest.TestCase):
     def test_make_proxy_disc(self):
-        abc = DiscreteVariable("abc", values="abc", ordered=True)
+        abc = DiscreteVariable("abc", values="abc")
         abc1 = abc.make_proxy()
         abc2 = abc1.make_proxy()
         self.assertEqual(abc, abc1)
@@ -721,7 +724,7 @@ def test_make_proxy_disc(self):
         self.assertEqual(hash(abc), hash(abc1))
         self.assertEqual(hash(abc1), hash(abc2))
 
-        abcx = DiscreteVariable("abc", values="abc", ordered=True)
+        abcx = DiscreteVariable("abc", values="abc")
         self.assertEqual(abc, abcx)
         self.assertIsNot(abc, abcx)
 

Orange/data/variable.py

@@ -629,21 +629,16 @@ class DiscreteVariable(Variable):
     .. attribute:: values
 
         A list of variable's values.
-
-    .. attribute:: ordered
-
-        Some algorithms (and, in particular, visualizations) may
-        sometime reorder the values of the variable, e.g. alphabetically.
-        This flag hints that the given order of values is "natural"
-        (e.g. "small", "middle", "large") and should not be changed.
     """
 
     TYPE_HEADERS = ('discrete', 'd', 'categorical')
 
     presorted_values = []
 
-    def __init__(self, name="", values=(), ordered=False, compute_value=None,
-                 *, sparse=False):
+    def __init__(
+            self, name="", values=(), ordered=None, compute_value=None,
+            *, sparse=False
+    ):
         """ Construct a discrete variable descriptor with the given values. """
         values = TupleList(values)  # some people (including me) pass a generator
         if not all(isinstance(value, str) for value in values):
@@ -652,7 +647,23 @@ def __init__(self, name="", values=(), ordered=False, compute_value=None,
         super().__init__(name, compute_value, sparse=sparse)
         self._values = values
         self._value_index = {value: i for i, value in enumerate(values)}
-        self.ordered = ordered
+
+        if ordered is not None:
+            warnings.warn(
+                "ordered is deprecated and does not have effect. It will be "
+                "removed in future versions.",
+                OrangeDeprecationWarning
+            )
+
+    @property
+    def ordered(self):
+        warnings.warn(
+            "ordered is deprecated. It will be removed in future versions.",
+            # FutureWarning warning is used instead of OrangeDeprecation
+            # warning otherwise tests fail (__repr__ still asks for ordered)
+            FutureWarning
+        )
+        return None
 
     @property
     def values(self):
@@ -819,9 +830,11 @@ def __reduce__(self):
             raise PickleError("Variables without names cannot be pickled")
         __dict__ = dict(self.__dict__)
         __dict__.pop("_values")
-        return make_variable, (self.__class__, self._compute_value, self.name,
-                               self.values, self.ordered), \
+        return (
+            make_variable,
+            (self.__class__, self._compute_value, self.name, self.values),
             __dict__
+        )
 
     def copy(self, compute_value=Variable._CopyComputeValue,
              *, name=None, values=None, **_):
@@ -830,7 +843,7 @@ def copy(self, compute_value=Variable._CopyComputeValue,
             raise ValueError(
                 "number of values must match the number of original values")
         return super().copy(compute_value=compute_value, name=name,
-                            values=values or self.values, ordered=self.ordered)
+                            values=values or self.values)
 
 
 class StringVariable(Variable):

Orange/preprocess/remove.py

@@ -169,7 +169,6 @@ def merge_transforms(exp):
             new_var = DiscreteVariable(
                 exp.var.name,
                 values=exp.var.values,
-                ordered=exp.var.ordered,
                 compute_value=merge_lookup(A, B),
                 sparse=exp.var.sparse,
             )

Orange/statistics/distribution.py

@@ -217,12 +217,10 @@ def sample(self, size=None, replace=True):
         return data.Value(self.variable, value_indices)
 
     def min(self):
-        if self.variable.ordered:
-            return self.variable.values[0]
+        return None
 
     def max(self):
-        if self.variable.ordered:
-            return self.variable.values[-1]
+        return None
 
     def sum(self, *args, **kwargs):
         res = super().sum(*args, **kwargs)

Orange/tests/test_distribution.py

@@ -32,7 +32,7 @@ def setUp(self):
             data.Domain(
                 attributes=[
                     data.DiscreteVariable('rgb', values=('r', 'g', 'b', 'a')),
-                    data.DiscreteVariable('num', values=('1', '2', '3'), ordered=True),
+                    data.DiscreteVariable('num', values=('1', '2', '3')),
                 ]
             ),
             X=np.array([
@@ -201,8 +201,8 @@ def test_min_max(self):
         self.assertEqual(self.rgb.min(), None)
         self.assertEqual(self.rgb.max(), None)
         # Min and max should work for ordinal variables
-        self.assertEqual(self.num.min(), '1')
-        self.assertEqual(self.num.max(), '3')
+        self.assertEqual(self.num.min(), None)
+        self.assertEqual(self.num.max(), None)
 
     def test_array_with_unknowns(self):
         d = data.Table("zoo")

Orange/tests/test_io.py

@@ -7,6 +7,7 @@
 import shutil
 import tempfile
 import unittest
+import warnings
 from unittest.mock import Mock, patch
 
 from Orange import data
@@ -16,6 +17,7 @@
 from Orange.data.table import get_sample_datasets_dir
 from Orange.data import Table, Variable
 from Orange.tests import test_dirname
+from Orange.util import OrangeDeprecationWarning
 
 
 class WildcardReader(FileFormat):
@@ -161,27 +163,34 @@ def test_load_pickle(self):
         This function tests whether pickled files in older Orange loads
         correctly with newer version of Orange.
         """
-        # load pickles created with Orange 3.20
-        # in next version there is a change in variables.py - line 738
-        # which broke back compatibility - tests were introduced after the fix
-        data1 = Table("datasets/sailing-orange-3-20.pkl")
-        data2 = Table("datasets/sailing-orange-3-20.pkl.gz")
-
-        # load pickles created with Orange 3.21
-        data3 = Table("datasets/sailing-orange-3-21.pkl")
-        data4 = Table("datasets/sailing-orange-3-21.pkl.gz")
-
-        examples_count = 20
-        self.assertEqual(examples_count, len(data1))
-        self.assertEqual(examples_count, len(data2))
-        self.assertEqual(examples_count, len(data3))
-        self.assertEqual(examples_count, len(data4))
-
-        attributes_count = 3
-        self.assertEqual(attributes_count, len(data1.domain.attributes))
-        self.assertEqual(attributes_count, len(data2.domain.attributes))
-        self.assertEqual(attributes_count, len(data3.domain.attributes))
-        self.assertEqual(attributes_count, len(data4.domain.attributes))
+        with warnings.catch_warnings():
+            # in unittests on travis/github actions OrangeDeprecationWarning
+            # is raised as an error. With this statement it si disabled only
+            # for this test - when unpickling pickle created with version older
+            # than 3.27 ordered parameter in DiscreteVariable which is
+            # deprecated still appears - which will raise deprecation warning
+            warnings.simplefilter('default', OrangeDeprecationWarning)
+            # load pickles created with Orange 3.20
+            # in next version there is a change in variables.py - line 738
+            # which broke back compatibility - tests introduced after the fix
+            data1 = Table("datasets/sailing-orange-3-20.pkl")
+            data2 = Table("datasets/sailing-orange-3-20.pkl.gz")
+
+            # load pickles created with Orange 3.21
+            data3 = Table("datasets/sailing-orange-3-21.pkl")
+            data4 = Table("datasets/sailing-orange-3-21.pkl.gz")
+
+            examples_count = 20
+            self.assertEqual(examples_count, len(data1))
+            self.assertEqual(examples_count, len(data2))
+            self.assertEqual(examples_count, len(data3))
+            self.assertEqual(examples_count, len(data4))
+
+            attributes_count = 3
+            self.assertEqual(attributes_count, len(data1.domain.attributes))
+            self.assertEqual(attributes_count, len(data2.domain.attributes))
+            self.assertEqual(attributes_count, len(data3.domain.attributes))
+            self.assertEqual(attributes_count, len(data4.domain.attributes))
 
     def test_pickle_version(self):
         """
@@ -196,3 +205,7 @@ def test_pickle_version(self):
         self.assertGreaterEqual(PICKLE_PROTOCOL, pickle.DEFAULT_PROTOCOL)
         # we should not use a version that is not supported
         self.assertLessEqual(PICKLE_PROTOCOL, pickle.HIGHEST_PROTOCOL)
+
+
+if __name__ == "__main__":
+    unittest.main()

Orange/widgets/data/owcsvimport.py

@@ -1543,12 +1543,11 @@ def pandas_to_table(df):
             coldata = series.values  # type: pd.Categorical
             categories = [str(c) for c in coldata.categories]
             var = Orange.data.DiscreteVariable.make(
-                str(header), values=categories, ordered=coldata.ordered
+                str(header), values=categories
             )
             # Remap the coldata into the var.values order/set
             coldata = pd.Categorical(
-                coldata.astype("str"), categories=var.values,
-                ordered=coldata.ordered,
+                coldata.astype(coldata), categories=var.values
             )
             codes = coldata.codes
             assert np.issubdtype(codes.dtype, np.integer)