Hi,
Does Element.__richcmp__() (via CoercionModel_cache_maps.richcmp())
really need to fall back on comparing by type/id when no common parent
is found? Since comparison operators are part of the coercion
framework, it would seem more sensible to raise an error.
Moreover, having an essentially arbitrary return value is dangerous,
because the user can easily mistake it for a mathematical result. My
immediate reason for asking this question is an example of this kind.
I'm trying to finally remove the coercions from floating-point parents
to interval parents (as discussed here long ago). But doing so would
cause comparisons such as RIF(-1,1) < RR(2) to return nonsensical
results, while they currently give the correct answer at least when the
floating-point operand is a constant (as opposed to the result of a
floating-point computation). (Now, I can perhaps think of ad-hoc fixes
that might be enough in this particular case, but I believe the issue
is more general.)
In contrast, the only reason I can see for this code path to exist in
the first place is that--if I understand right--Python calls
a.__richcmp__(), not a.__cmp__(), when cmp() is applied to objects a
and b whose common superclass does not implement __cmp__. Therefore,
one can end up in Element.__richcmp__() while comparing an Element to a
non-Element via cmp(). And unfortunately, there does not seem to be any
simple way to distinguish between this situation and calls to
__richcmp__() coming from comparison operators. Is this correct? Did I
miss another reason?
If that's really the main reason, I'd argue that having cmp() fail in
some cases is not as bad as having comparison operators return nonsense
on Elements. (Besides, all cases where this could be a problem will
have to be fixed anyway to port Sage to Python3, won't they?) What do
you think?
Note that as a middle ground, we could raise an error when coercion
fails with both operands being Elements, while still returning a result
at all costs when comparing an Element with a non-Element. However,
this wouldn't help in cases like RIF(-1, 1) < float(2).
It would also be safe--though not ideal--to keep returning False for
comparisons using ==, and sort-of-okay to return True when comparing
with !=. And, for example, the following patch, which also whitelists
comparisons with strings (because there are typically safe and seem to
be in wide use in combinat/) only leaves us with about 40 test failures
(many of which trivial).
@@ -1870,8 +1871,6 @@ cdef class
CoercionModel_cache_maps(CoercionModel):
(<Element>x)._parent.get_flag(Parent_richcmp_element_without_coercion))
return PyObject_RichCompare(x, y, op)
- # Comparing with coercion didn't work, try something else.
-
# Try y.__richcmp__(x, revop) where revop is the reversed
# operation (<= becomes >=).
# This only makes sense when y is not a Sage Element, otherwise
@@ -1883,18 +1882,23 @@ cdef class
CoercionModel_cache_maps(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)
+ if op == Py_EQ:
+ return False
+ elif op == Py_NE:
+ return True
+ elif type(y) is str:
+ return rich_to_bool(op, cmp(type(x), type(y)))
else:
- return rich_to_bool(op, -1)
+ raise bin_op_exception(operator_object(op), x, y)
def _coercion_error(self, x, x_map, x_elt, y, y_map, y_elt):
"""
@@ -1924,3 +1928,18 @@ Original elements %r (parent %s) and %r
(parent %s) and maps
%s %r""" % (x_elt, y_elt, parent_c(x_elt), parent_c(y_elt),
x, parent_c(x), y, parent_c(y),
type(x_map), x_map, type(y_map), y_map))
+
+cdef object operator_object(int op):
+ if op == Py_LT:
+ return operator.lt
+ elif op == Py_LE:
+ return operator.le
+ elif op == Py_EQ:
+ return operator.eq
+ elif op == Py_NE:
+ return operator.ne
+ elif op == Py_GE:
+ return operator.ge
+ elif op == Py_GT:
+ return operator.gt
(By the way, does operator_object already exist somewhere?)
Thanks for your input,
--
Marc
--
You received this message because you are subscribed to the Google Groups
"sage-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to sage-devel+unsubscr...@googlegroups.com.
To post to this group, send email to sage-devel@googlegroups.com.
Visit this group at https://groups.google.com/group/sage-devel.
For more options, visit https://groups.google.com/d/optout.
