Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve rich comparison methods, including fix for #13078 #13091

Merged
merged 3 commits into from Aug 8, 2017
Merged

Improve rich comparison methods, including fix for #13078 #13091

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4edfd3c
Improve rich comparison with unknown types
danielwe Aug 5, 2017
7a5b5ca
Let NumberSymbol comparisons equal reflections
danielwe Aug 5, 2017
bfdaec2
Revert 4edfd3c as far as inequalities go
danielwe Aug 7, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions sympy/core/basic.py
View file
Open in desktop
Expand Up @@ -313,7 +313,7 @@ def __eq__(self, other):
try:
other = _sympify(other)
except SympifyError:
return False # sympy != other
return NotImplemented

if type(self) != type(other):
return False
Expand All @@ -329,7 +329,7 @@ def __ne__(self, other):

but faster
"""
return not self.__eq__(other)
return not self == other
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@skirpichev skirpichev Aug 6, 2017 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw, this method redundant, isn't?

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant in python 3, but not in python 2.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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


def dummy_eq(self, other, symbol=None):
"""
Expand Down Expand Up @@ -1180,7 +1180,7 @@ def _has(self, pattern):

def _has_matcher(self):
"""Helper for .has()"""
return self.__eq__
return lambda other: self == other

def replace(self, query, value, map=False, simultaneous=True, exact=False):
"""
Expand Down
8 changes: 4 additions & 4 deletions sympy/core/expr.py
View file
Open in desktop
Expand Up @@ -248,7 +248,7 @@ def __ge__(self, other):
try:
other = _sympify(other)
except SympifyError:
raise TypeError("Invalid comparison %s >= %s" % (self, other))
return NotImplemented
for me in (self, other):
if (me.is_complex and me.is_real is False) or \
me.has(S.ComplexInfinity):
Expand All @@ -270,7 +270,7 @@ def __le__(self, other):
try:
other = _sympify(other)
except SympifyError:
raise TypeError("Invalid comparison %s <= %s" % (self, other))
return NotImplemented
for me in (self, other):
if (me.is_complex and me.is_real is False) or \
me.has(S.ComplexInfinity):
Expand All @@ -292,7 +292,7 @@ def __gt__(self, other):
try:
other = _sympify(other)
except SympifyError:
raise TypeError("Invalid comparison %s > %s" % (self, other))
return NotImplemented
for me in (self, other):
if (me.is_complex and me.is_real is False) or \
me.has(S.ComplexInfinity):
Expand All @@ -314,7 +314,7 @@ def __lt__(self, other):
try:
other = _sympify(other)
except SympifyError:
raise TypeError("Invalid comparison %s < %s" % (self, other))
return NotImplemented
for me in (self, other):
if (me.is_complex and me.is_real is False) or \
me.has(S.ComplexInfinity):
Expand Down
4 changes: 2 additions & 2 deletions sympy/core/exprtools.py
View file
Open in desktop
Expand Up @@ -797,7 +797,7 @@ def __eq__(self, other): # Factors
return self.factors == other.factors

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


class Term(object):
Expand Down Expand Up @@ -909,7 +909,7 @@ def __eq__(self, other): # Term
self.denom == other.denom)

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


def _gcd_terms(terms, isprimitive=False, fraction=True):
Expand Down