Lombokython - Automatic __eq__, __hash__, __repr__

Question

I recently decided to code some in Python after coding in Java using lombok for quite some time. However, I got bit real hard when I forgot to implement __eq__, since Lombok normally does it for you.

I decided to try to implement something similar to Lombok's @EqualsAndHashCode, which I called eqhash. Then, I added a similar way to generate a __repr__ method.

How well did I follow python style? I developed this using TDD, so I would like to know how my tests are as well.

genmethods.py

def eqhash(cls: type):
    """Adds __eq__, __ne__, and __hash__ methods to the class"""
    class NewCls(cls):
        def __eq__(self, other):
            if self is other: return True
            if not isinstance(other, NewCls): return False
            svars = vars(self)
            ovars = vars(other)
            return svars == ovars

        def __ne__(self, other):
            return not (self == other)

        def __hash__(self):
            return hash(vars(self).values())

    return _pass_attrs(NewCls, cls)


def repr_(cls: type):
    """Adds a __repr__ method to the class"""
    class NewCls(cls):
        def __repr__(self):
            return '{clsname}({variables})'.format(
                clsname=cls.__name__,
                variables=', '.join(repr(o) for o in vars(self).values())
            )

    return _pass_attrs(NewCls, cls)


def _pass_attrs(new_cls, cls):
    new_cls.__name__ = cls.__name__
    new_cls.__qualname__ = cls.__qualname__
    for attr, value in vars(cls).items():
        try:
            setattr(new_cls, attr, value)
        except AttributeError:
            pass

    return new_cls

test_genmethods.py

import unittest
import genmethods


class SData:
    def __init__(self, value):
        self._value = value

    def test(self):
        return self._value


def ensure_not_mangled(self: unittest.TestCase, cls: type, name: str):
    self.assertEqual((1, 2), cls((1, 2)).test())
    self.assertEqual(name, cls.__name__)


class EqHashTest(unittest.TestCase):
    @genmethods.eqhash
    class Data(SData):
        pass

    def test_eq_genned(self):
        self.assertEqual(EqHashTest.Data((1, 2)), EqHashTest.Data((1, 2)))
        self.assertNotEqual(EqHashTest.Data((1, 2)), EqHashTest.Data((1, 4)))

    def test_hash_genned(self):
        v1 = EqHashTest.Data(1)
        v2 = EqHashTest.Data(1)
        self.assertEqual(hash(v1), hash(v2))

    def test_not_mangled(self):
        ensure_not_mangled(self, EqHashTest.Data, "Data")


class ReprTest(unittest.TestCase):
    @genmethods.repr_
    class Data(SData):
        pass

    def test_repr_genned(self):
        self.assertEqual('Data(1)', repr(ReprTest.Data(1)))

    def test_not_mangled(self):
        ensure_not_mangled(self, EqHashTest.Data, "Data")


class ReprEqHashTest(unittest.TestCase):
    @genmethods.repr_
    @genmethods.eqhash
    class Data(SData):
        pass

    def test_eq_genned(self):
        self.assertEqual(ReprEqHashTest.Data((1, 2)), ReprEqHashTest.Data((1, 2)))
        self.assertNotEqual(ReprEqHashTest.Data((1, 2)), ReprEqHashTest.Data((1, 4)))

    def test_hash_genned(self):
        v1 = ReprEqHashTest.Data(1)
        v2 = ReprEqHashTest.Data(1)
        self.assertEqual(hash(v1), hash(v2))

    def test_repr_genned(self):
        self.assertEqual('Data(1)', repr(ReprEqHashTest.Data(1)))

    def test_not_mangled(self):
        ensure_not_mangled(self, EqHashTest.Data, "Data")


class EqHashReprTest(unittest.TestCase):
    @genmethods.eqhash
    @genmethods.repr_
    class Data(SData):
        pass

    def test_eq_genned(self):
        self.assertEqual(ReprEqHashTest.Data((1, 2)), ReprEqHashTest.Data((1, 2)))
        self.assertNotEqual(ReprEqHashTest.Data((1, 2)), ReprEqHashTest.Data((1, 4)))

    def test_hash_genned(self):
        v1 = ReprEqHashTest.Data(1)
        v2 = ReprEqHashTest.Data(1)
        self.assertEqual(hash(v1), hash(v2))

    def test_repr_genned(self):
        self.assertEqual('Data(1)', repr(ReprEqHashTest.Data(1)))

    def test_not_mangled(self):
        ensure_not_mangled(self, EqHashTest.Data, "Data")

Answer 1

1. Review

1. It would be substantially simpler to use a mixin instead of a decorator.

2. The test for equality is not symmetric: you have a == b if b belongs to a subclass of a's class, but not vice versa. So it's possible to have a == b but b != a, which makes no sense.

   There are two sensible things to do here: either (i) ignore the class relationship, so that two objects compare equal if their vars are equal, regardless of which class they belong to; or (ii) insist that classes match, so that two objects compare equal only if they belong to the same class.

3. The intention seems to be for eqhash objects to hash based on their instance attributes, so that instances with different attributes get different hashes. But the code doesn't work! Here are two objects with different instance attributes, but whose hashes are the same:

   >>> @eqhash
   ... class Data(SData):
   ...     pass
   >>> d = Data(1)
   >>> hash(d)
   -9223372036573986785
   >>> e = Data(2)
   >>> hash(e)
   -9223372036573986785
   

   And here is one object that has different hashes at different times:

   >>> d = Data(1)
   >>> hash(d)
   -9223372036573986785
   >>> vars(d).values()
   dict_values([1])
   >>> hash(d)
   -9223372036574014153
   

   The problem is that you are constructing a dict_values object and then taking its hash. But dict_values objects don't hash based on their contents, only on their id (see Python issue 22192), so the hash doesn't tell you anything about the values, only about the location in memory of the dict_values object.

2. Revised code

class EqHash:
    """Mixin adding __eq__, __ne__, and __hash__ methods."""
    def __eq__(self, other):
        return (self is other
                or (type(self) == type(other)
                    and vars(self) == vars(other)))

    def __ne__(self, other):
        return not (self == other)

    def __hash__(self):
        return hash(tuple(sorted(vars(self).items())))

class Repr:
    """Mixin adding a __repr__ method."""
    def __repr__(self):
        return '{name}({values})'.format(
            name=type(self).__name__,
            values=', '.join(map(repr, vars(self).values())))

3. Answers to questions

1. functools.total_ordering is a decorator becuase it looks at the class in order to determine which ordering method was provided. See the implementation, which calls getattr(cls, op, None). It would be much harder to do this in a mixin.

2. I spotted the problems by thinking carefully!

   First, equality is a symmetric relation, but isinstance is asymmetric, so it was suspicious that one was implemented using the other.

   Second, it only makes sense to implement __hash__ on immutable objects, but the values of a dictionary are mutable, so this was also suspicious. See the documentation for __hash__, where it says:

   If a class defines mutable objects and implements an __eq__ method, it should not implement __hash__, since the implementation of hashable collections requires that a key’s hash value is immutable (if the object’s hash value changes, it will be in the wrong hash bucket).