For comparison functions, use subclass check, or identity check?

[ericvsmith]

From the Abstract in the PEP, the comparison functions are given as:

def __eq__(self, other):
    if other.__class__ is self.__class__:
        return (self.name, self.unit_price, self.quantity_on_hand) == (other.name, other.unit_price, other.quantity_on_hand)
    return NotImplemented

There's been discussion on whether this should be a subclass check, or an exact match. I plan on looking in to this and addressing it before the next PEP version. This is a placeholder to remind me, and for discussion.

[ericvsmith]

Note that attrs does an isinstance check. I couldn't find any discussion of why they chose this instead of an exact type check.

    def lt(self, other):
        """
        Automatically created by attrs.
        """
        if isinstance(other, self.__class__):
            return attrs_to_tuple(self) < attrs_to_tuple(other)
        else:
            return NotImplemented

[ericvsmith]

Actually, that's not the whole story. In attrs, __eq__ and __ne__ use an exact type check:

    def eq(self, other):
        """
        Automatically created by attrs.
        """
        if other.__class__ is self.__class__:
            return attrs_to_tuple(self) == attrs_to_tuple(other)
        else:
            return NotImplemented

[ericvsmith]

In https://mail.python.org/pipermail/python-dev/2017-November/150883.html, @gvanrossum suggested matching the fields as well as their values. This means that a class and a subclass that doesn't add any fields could be compared, but if a subclass adds fields it will not be comparable. This comes down to:

isinstance(other, self.__class__) and len(fields(other)) == len(fields(self)) and <all individual fields match>

You can just compare len(fields(obj)) because a subclass will always start with the same number of fields as the base class, by definition. So if the len's don't match, the subclass must have added some fields.

I think this is a good solution, and provides the functionality that we're looking for. I'll come up with a PR shortly.

[ericvsmith]

@hynek: Can you comment on why attrs uses this code for eq,ne vs. lt,le,gt,ge?

    def eq(self, other):
        if other.__class__ is self.__class__:
            return attrs_to_tuple(self) == attrs_to_tuple(other)
        else:
            return NotImplemented

    def lt(self, other):
        if isinstance(other, self.__class__):
            return attrs_to_tuple(self) < attrs_to_tuple(other)
        else:
            return NotImplemented

That is, why the exact test for type matching on eq and ne, but the isinstance test on the others?

Thanks.

[hynek]

I’m gonna say it’s an oversight when I made eq stricter. I seem to remember, that someone told me, that it should be strict, but I don’t really remember because it’s 2+ years ago. 🤔

[ericvsmith]

That's helpful, thanks.

Given that, I propose that we leave the code with a strict other.__class__ is self.__class__ check for all 6 methods.