Python: Can you make this __eq__ easy to understand?

Question

I have another question for you.

I have a python class with a list 'metainfo'. This list contains variable names that my class might contain. I wrote a __eq__ method that returns True if the both self and other have the same variables from metainfo and those variables have the same value.

Here is my implementation:

 def __eq__(self, other):
    for attr in self.metainfo:
      try:
        ours = getattr(self, attr) 
        try:
          theirs = getattr(other, attr)
          if ours != theirs:
            return False
        except AttributeError:
          return False
      except AttributeError:
        try:
          theirs = getattr(other, attr)
          return False
        except AttributeError:
          pass
    return True

Does anyone have any suggestions as to how I can make this code easier on the eye? Be as ruthless as you please.

Answer 1

Use getattr's third argument to set distinct default values:

def __eq__(self, other):
    return all(getattr(self, a, Ellipsis) == getattr(other, a, Ellipsis)
               for a in self.metainfo)

As the default value, set something that will never be an actual value, such as Ellipsis^†. Thus the values will match only if both objects contain the same value for a certain attribute or if both do not have said attribute.

Edit: as Nadia points out, NotImplemented may be a more appropriate constant (unless you're storing the result of rich comparisons...).

Edit 2: Indeed, as Lac points out, just using hasattr results in a more readable solution:

def __eq__(self, other):
    return all(hasattr(self, a) == hasattr(other, a) and
               getattr(self, a) == getattr(other, a) for a in self.metainfo)

---

  ^†: for extra obscurity you could write ... instead of Ellipsis, thus getattr(self, a, ...) etc. No, don't do it :)

Answer 2

I would add a docstring which explains what it compares, as you did in your question.

Answer 3

def __eq__(self, other):
    """Returns True if both instances have the same variables from metainfo
    and they have the same values."""
    for attr in self.metainfo:
        if attr in self.__dict__:
            if attr not in other.__dict__:
                return False
            if getattr(self, attr) != getattr(other, attr):
                return False
            continue
        else:
            if attr in other.__dict__:
                return False
    return True

Answer 4

Since it's about to make it easy to understand, not short or very fast :

class Test(object):

    def __init__(self):
        self.metainfo = ["foo", "bar"]

    # adding a docstring helps a lot
    # adding a doctest even more : you have an example and a unit test
    # at the same time ! (so I know this snippet works :-))
    def __eq__(self, other):
        """
            This method check instances equality and returns True if both of
            the instances have the same attributs with the same values.
            However, the check is performed only on the attributs whose name
            are listed in self.metainfo.

            E.G :

            >>> t1 = Test()
            >>> t2 = Test()
            >>> print t1 == t2
            True
            >>> t1.foo = True
            >>> print t1 == t2
            False
            >>> t2.foo = True
            >>> t2.bar = 1
            >>> print t1 == t2
            False
            >>> t1.bar = 1
            >>> print t1 == t2
            True
            >>> t1.new_value = "test"
            >>> print t1 == t2
            True
            >>> t1.metainfo.append("new_value")
            >>> print t1 == t2
            False

        """

        # Then, let's keep the code simple. After all, you are just
        # comparing lists :

        self_metainfo_val = [getattr(self, info, Ellipsis)
                             for info in self.metainfo]
        other_metainfo_val = [getattr(other, info, Ellipsis)
                              for info in self.metainfo]
        return self_metainfo_val == other_metainfo_val

Answer 5

Going with "Flat is better than nested" I would remove the nested try statements. Instead, getattr should return a sentinel that only equals itself. Unlike Stephan202, however, I prefer to keep the for loop. I also would create a sentinel by myself, and not re-use some existing Python object. This guarantees that there are no false positives, even in the most exotic situations.

def __eq__(self, other):
    if set(metainfo) != set(other.metainfo):
        # if the meta info differs, then assume the items differ.
        # alternatively, define how differences should be handled
        # (e.g. by using the intersection or the union of both metainfos)
        # and use that to iterate over
        return False
    sentinel = object() # sentinel == sentinel <=> sentinel is sentinel
    for attr in self.metainfo:
        if getattr(self, attr, sentinel) != getattr(other, attr, sentinel):
            return False
    return True

Also, the method should have a doc-string explaining it's eq behavior; same goes for the class which should have a docstring explaining the use of the metainfo attribute.

Finally, a unit-test for this equality-behavior should be present as well. Some interesting test cases would be:

1. Objects that have the same content for all metainfo-attributes, but different content for some other attributes (=> they are equal)
2. If required, checking for commutativity of equals, i.e. if a == b: b == a
3. Objects that don't have any of the metainfo-attributes set

Answer 6

I would break the logic up into separate chunks that are easier to understand, each one checking a different condition (and each one assuming the previous thing was checked). Easiest just to show the code:

# First, check if we have the same list of variables.
my_vars = [var for var in self.metainf if hasattr(self, var)]
other_vars = [var for var in other.metainf if hasattr(other, var)]

if my_vars.sorted() != other_vars.sorted():
  return False # Don't even have the same variables.

# Now, check each variable:
for var in my_vars:
   if self.var != other.var:
      return False # We found a variable with a different value.

# We're here, which means we haven't found any problems!
return True

Edit: I misunderstood the question, here is an updated version. I still think this is a clear way to write this kind of logic, but it's uglier than I intended and not at all efficient, so in this case I'd probably go with a different solution.

Answer 7

The try/excepts make your code harder to read. I'd use getattr with a default value that is guaranteed not to otherwise be there. In the code below I just make a temp object. That way if object do not have a given value they'll both return "NOT_PRESENT" and thus count as being equal.

def __eq__(self, other):
    NOT_PRESENT = object()
    for attr in self.metainfo:
        ours = getattr(self, attr, NOT_PRESENT) 
        theirs = getattr(other, attr, NOT_PRESENT)
        if ours != theirs:
            return False
    return True

Answer 8

Here is a variant that is pretty easy to read IMO, without using sentinel objects. It will first compare if both has or hasnt the attribute, then compare the values.

It could be done in one line using all() and a generator expression as Stephen did, but I feel this is more readable.

def __eq__(self, other):
    for a in self.metainfo:
        if hasattr(self, a) != hasattr(other, a):
             return False
        if getattr(self, a, None) != getattr(other, a, None):
             return False
    return True

Answer 9

I like Stephan202's answer, but I think that his code doesn't make equality conditions clear enough. Here's my take on it:

def __eq__(self, other):
    wehave = [attr for attr in self.metainfo if hasattr(self, attr)]
    theyhave = [attr for attr in self.metainfo if hasattr(other, attr)]
    if wehave != theyhave:
        return False
    return all(getattr(self, attr) == getattr(other, attr) for attr in wehave)