Improve rich comparison methods, including fix for #13078

[danielwe]

Comparison methods are modified to return NotImplemented when comparing against an unknown type that cannot be sympified. The rationale is described in #13078, and details of how python resolves binary operators to methods are described at https://docs.python.org/3/reference/datamodel.html and http://docs.sympy.org/latest/python-comparisons.html.

All comparisons between sympy/sympifiable types are left unchanged.

The common pattern for defining __ne__,

    def __ne__(self, other):
        return not self.__eq__(other)

should be avoided, since not NotImplemented evaluates to False, thus skipping the call to the reflected method on the other object in such cases. This pull request therefore replaces all such occurrences with

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

and this pattern is extended to most other cases where a rich comparison method used to be called directly (unless it's a super call, an explicit call to the reflected method on the other object, or the special cases in #7951).

The final commit fixes what appears to be a bug in the way reflected methods are called in comparisons involving NumberSymbol instances. While these comparisons are tricky (see #13081), it seems strange for them not to be symmetric wrt. their reflections.

Fixes #13078

[jksuom]

Is this the correct result when self == other returns NotImplemented?

[danielwe]

The point is that self == other never returns NotImplemented. If self.__eq__(other) returns NotImplemented, the comparison is delegated to other.__eq__(self). If it, too, returns NotImplemented, self == other evaluates to False.

This way, a != b always returns the opposite of a == b. If using the not self.__eq__(other)pattern, this short circuits the method resolution such that both a != b and a == b may return False in some circumstances.

For other rich comparison operators (<, > etc.), the method resolution is similar, and if both sides return NotImplemented a TypeError is raised.

The pull request includes tests for all of these details.

[skirpichev]

btw, this method redundant, isn't?

from py3 docs:
"By default, __ne__() delegates to __eq__() and inverts the result unless it is NotImplemented."

[danielwe]

Redundant in python 3, but not in python 2.

[jksuom]

The point is that self == other never returns NotImplemented.

Ah... I see. That explains the change.

[danielwe]

Failing checks are due to python 2 not raising TypeError when both sides of an inequality comparison return NotImplemented. Instead, the comparison evaluates to either True or False in a consistent (a < b implies b > a, not a > b, not b < a etc.), but seemingly arbitrary fashion. I'm currently looking into how to work around this.

[asmeurer]

We may need to leave the inequalities alone. We strictly speaking break against the python 2 behavior of allowing arbitrary comparisons for the more sane python 3 behavior of only allowing mathematically consistent comparisons.

[asmeurer]

Looks good to me.