Python: Can you make this __eq__ understandable?

Question

I have one more question for you.

I have a python class with a metainfo list. This list contains the names of the variables that my class may contain. I wrote a __eq__ method that returns True if both self and other have the same variables from metainfo , and these 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 on how I can make this code easier on the eyes? Be as merciless as you wish.

Answer 1

Use getattr third argument to set various default values:

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

Set the default value to a value that will never be the actual value, such as Ellipsis ^{& dagger;} . Thus, the values ​​will correspond only if both objects contain the same value for a specific attribute or if both of them do not have a specified attribute.

Edit : as Nadia points out NotImplemented might be a more appropriate constant (if you don't save the result of rich comparisons ...).

Edit 2: In fact, as Lac points out, just using hasattr leads to 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) 

---

^{& dagger;} : for additional ambiguity you can write ... instead of Ellipsis , thus getattr(self, a, ...) etc. No, do not do this :)

Answer 2

I would add a docstring which explains that it compares how you did it 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

Transitioning from "Flat is better than nested" I would delete nested try statements. Instead, getattr should return a watch that is only equal to itself. However, unlike Stephan202, I prefer to keep the for loop. I also create a sentinel, and will not reuse any existing Python object. This ensures 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 # (eg 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 

In addition, the method must have a doc string explaining the behavior of eq ; the same applies to a class that must have a docstring explaining the use of the metainfo attribute.

Finally, a unit test must be present for this equality behavior. Some interesting test cases:

• Objects that have the same content for all metainfo attributes, but different content for some other attributes (=> they are equal)
• If necessary, check for the commutativity of peers, i.e. if a == b: b == a
• Objects that do not have a metainfo attribute set

Answer 5

Since this makes 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. EG : >>> 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 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 6

I would break the logic into separate pieces that are easier to understand, each of which checks a different condition (and each of them assumes that the previous thing has been checked). The easiest way is to simply 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 the updated version. I still think this is a clear way to write such logic, but it is uglier than I expected and not at all effective, so in this case I will probably go with a different solution.

Answer 7

With try / excepts, your code becomes more difficult to read. I would use getattr with a default value, which is guaranteed that otherwise it will not. In the code below, I just create a temporary object. Thus, if the object does not have a given value, they both return "NOT_PRESENT" and, therefore, are considered 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 an option that is pretty easy to read IMO without using sentinel objects. It compares first if both have the hasnt attribute or not, and then compare the values.

This can be done on a single line using all () and the generator expression, as Stephen did, but I find it 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 its code does not make the equality conditions clear enough. Here I take it upon myself:

 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)