Formatting around "and"

[grantjenks]

I'm not thrilled with this diff:

     def __eq__(self, that):
         if not isinstance(that, type(self)):
             return NotImplemented
-        return (self.__slots__ == that.__slots__
-                and all(item == iota for item, iota in zip(self, that)))
+        return self.__slots__ == that.__slots__ and all(
+            item == iota for item, iota in zip(self, that)
+        )
 
     def __repr__(self):

[grantjenks]

I think the ideal here is:

     def __eq__(self, that):
         if not isinstance(that, type(self)):
             return NotImplemented
         return (
            self.__slots__ == that.__slots__
            and all(item == iota for item, iota in zip(self, that))
        )

[wbolster]

oh hai,

imo, the ‘ideal’ is far from ideal.

• for starters, this code implicitly depends on the type being iterable.
• on top of that, it implicitly assumes that iteration yields the values worth comparing, which may or may not be the same as having equal values in each field defined in __slots__
• the variable ‘that’ would contrast with a variable ‘this’, but this is python, which uses ‘self’. so ‘self’ / ‘other’ would be a better choice.
• a variable named ‘iota’ means nothing
• a variable ‘item’ compared to ‘iota’ doesn't mean much either

that said, here's a quick attempt that would be closer to ‘ideal’, and lo and behold, it doesn't suffer from perceived formatting ‘problems’ either:

class C:
    def __eq__(self, other):
        if not isinstance(other, type(self)):
            return NotImplemented

        if self.__slots__ != other.__slots__:
            return False

        for x in self.__slots__:
            if getattr(self, x) != getattr(other, x):
                return False

        return True

the ‘ideal’ solution would be to not write any code at all, and use attrs instead to declaratively define attributes and whether they should be included in an an equality comparison.

[merwok]

This ticket is not a discussion of that __eq__ method, but of the formatting of specific lines by the black tool.

[wbolster]

which is way more subtle and complex than this ticket seems to imply; see psf/black#2156 (comment)