In the Python3 spirit, it would be nice to get a type error instead of ordering by id:
$ python3 >>> 1 < 'a' Traceback (most recent call last): File "<stdin>", line 1, in <module> TypeError: unorderable types: int() < str() The current behavior is certainly not great, and was just defaulted because thats what Python2 does. On Sunday, December 4, 2016 at 7:39:59 PM UTC+1, Marc Mezzarobba wrote: > > 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.
