Merge pull request #4843 from janezd/variable-fix-hash

[FIX] Variable: Fix cases when equal variables had different hashes

Author: lanzagar

Orange/data/tests/test_variable.py

@@ -163,6 +163,7 @@ def test_eq_with_compute_value(self):
         a._compute_value = Identity(a1)
         self.assertEqual(a, a)
         self.assertEqual(a, b)
+        self.assertEqual(hash(a), hash(b))
 
         b._compute_value = a.compute_value
         self.assertEqual(a, b)
@@ -206,6 +207,27 @@ def test_hash(self):
         b._compute_value = Identity(a2)
         self.assertEqual(hash(a), hash(b))
 
+        at = TimeVariable("a")
+        b = ContinuousVariable("b")
+        self.assertEqual(hash(a1), hash(a2))
+        self.assertNotEqual(hash(a1), hash(b))
+        self.assertNotEqual(hash(a1), hash(at))
+
+    def test_hash_eq(self):
+        a = ContinuousVariable("a")
+        b1 = ContinuousVariable("b", compute_value=Identity(a))
+        b2 = ContinuousVariable("b2", compute_value=Identity(b1))
+        b3 = ContinuousVariable("b")
+        self.assertEqual(a, b2)
+        self.assertEqual(b1, b2)
+        self.assertEqual(a, b1)
+        self.assertNotEqual(b1, b3)
+
+        self.assertEqual(hash(a), hash(b2))
+        self.assertEqual(hash(b1), hash(b2))
+        self.assertEqual(hash(a), hash(b1))
+        self.assertNotEqual(hash(b1), hash(b3))
+
 
 def variabletest(varcls):
     def decorate(cls):
@@ -252,7 +274,6 @@ def test_val_from_str_add(self):
         self.assertEqual(var.val_from_str_add("F"), 0)
         self.assertEqual(var.val_from_str_add("N"), 2)
 
-
     def test_repr(self):
         var = DiscreteVariable.make("a", values=("F", "M"))
         self.assertEqual(

Orange/data/variable.py

@@ -347,34 +347,26 @@ def make_proxy(self):
         return var
 
     def __eq__(self, other):
-        # pylint: disable=protected-access,import-outside-toplevel
-
-        def to_match(var):
-            if var._compute_value is None:
-                return var
-            elif isinstance(var._compute_value, Identity):
-                return var._compute_value.variable
-            return None
+        if type(self) is not type(other):
+            return False
 
-        from Orange.preprocess.transformation import Identity
-        return type(self) is type(other) and (
-            self.name == other.name
-            and self._compute_value == other._compute_value
-            or
-            (self.compute_value or other.compute_value)
-            and to_match(self) == to_match(other) != None)
+        var1 = self._get_identical_source(self)
+        var2 = self._get_identical_source(other)
+        # pylint: disable=protected-access
+        return var1.name == var2.name \
+               and var1._compute_value == var2._compute_value
 
     def __hash__(self):
-        # Two variables that are not equal can have the same hash.
-        # This happens if one has compute_value == Identity and the other
-        # doesn't have compute_value, or they have a different Identity.
-        # Having the same hash while not being equal is of course allowed.
-        # pylint: disable=import-outside-toplevel
+        var = self._get_identical_source(self)
+        return hash((var.name, type(self), var._compute_value))
+
+    @staticmethod
+    def _get_identical_source(var):
+        # pylint: disable=protected-access,import-outside-toplevel
         from Orange.preprocess.transformation import Identity
-        compute_value = self._compute_value
-        if isinstance(self._compute_value, Identity):
-            compute_value = None
-        return hash((self.name, type(self), compute_value))
+        while isinstance(var._compute_value, Identity):
+            var = var._compute_value.variable
+        return var
 
     @classmethod
     def make(cls, name, *args, **kwargs):