Skip to content

Commit

Permalink
Trac #22029: Element richcmp: never use id()
Browse files Browse the repository at this point in the history
As discussed in the sage-devel thread starting with
`<o21nte$6jp$1@blaine.gmane.org>` (https://groups.google.com/d/msg/sage-
devel/YVFdxPH6avk/4OZUmzLHBgAJ),
`coercion_model.richcmp()` should not fall back on comparing by type/id.

This branch implements comparisons for `Element` the same way as in
Python 3: a `TypeError` is raised for uncomparable objects (instead of
comparing by `id`).

Dependencies: #22297, #22344, #22346, #22369, #22370, #22371, #22372,
#22373, #22376, #22382, #24968, #26917, #26931, #26933, #26934, #26935,
#26936, #26937, #26938, #26947, #27003, #27009, #27010, #27026, #27027,
#27029, #27123, #27241

URL: https://trac.sagemath.org/22029
Reported by: mmezzarobba
Ticket author(s): Marc Mezzarobba, Jeroen Demeyer
Reviewer(s): Julian Rüth, Marc Mezzarobba
  • Loading branch information
Release Manager authored and vbraun committed Feb 10, 2019
2 parents d4add0d + e311ab1 commit ded99bf
Showing 1 changed file with 23 additions and 9 deletions.
32 changes: 23 additions & 9 deletions src/sage/structure/coerce.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ see the documentation for :class:`Parent`.
from __future__ import print_function, absolute_import

from cpython.object cimport (PyObject, PyTypeObject,
PyObject_CallObject, PyObject_RichCompare, Py_TYPE)
PyObject_CallObject, PyObject_RichCompare, Py_TYPE,
Py_EQ, Py_NE, Py_LT, Py_LE, Py_GT, Py_GE)
from cpython.weakref cimport PyWeakref_GET_OBJECT, PyWeakref_NewRef
from libc.string cimport strncmp

Expand Down Expand Up @@ -1893,14 +1894,17 @@ cdef class CoercionModel:
sage: richcmp(int(1), float(2), op_GE)
False
If there is no coercion, only comparisons for equality make
sense::
If there is no coercion, we only support ``==`` and ``!=``::
sage: x = QQ.one(); y = GF(2).one()
sage: richcmp(x, y, op_EQ)
False
sage: richcmp(x, y, op_NE)
True
sage: richcmp(x, y, op_GT)
Traceback (most recent call last):
...
TypeError: unsupported operand parent(s) for >: 'Rational Field' and 'Finite Field of size 2'
We support non-Sage types with the usual Python convention::
Expand Down Expand Up @@ -1950,13 +1954,23 @@ cdef class CoercionModel:
if res is not NotImplemented:
return res

# Final attempt: compare by id()
if (<unsigned long><PyObject*>x) >= (<unsigned long><PyObject*>y):
# It cannot happen that x is y, since they don't
# have the same parent.
return rich_to_bool(op, 1)
# At this point, we have 2 objects which cannot be coerced to
# a common parent. So we assume that they are not equal.
if op == Py_EQ:
return False
if op == Py_NE:
return True

# It does not make sense to compare x and y with an inequality,
# so we raise an exception.
if op == Py_LT:
raise bin_op_exception('<', x, y)
elif op == Py_LE:
raise bin_op_exception('<=', x, y)
elif op == Py_GT:
raise bin_op_exception('>', x, y)
else:
return rich_to_bool(op, -1)
raise bin_op_exception('>=', x, y)

def _coercion_error(self, x, x_map, x_elt, y, y_map, y_elt):
"""
Expand Down

0 comments on commit ded99bf

Please sign in to comment.