Opened 7 years ago

Last modified 7 years ago

#17890 closed enhancement

Remove _(rich)cmp_c_impl — at Version 10

Reported by: jdemeyer Owned by:
Priority: major Milestone: sage-6.7
Component: cython Keywords:
Cc: dkrenn Merged in:
Authors: Jeroen Demeyer Reviewers:
Report Upstream: N/A Work issues:
Branch: u/jdemeyer/ticket/17890 (Commits, GitHub, GitLab) Commit: 25e517fc735d15ba07c919f7a65103cf04acd2cf
Dependencies: Stopgaps:

Status badges

Description (last modified by jdemeyer)

Instead of

    def _cmp_(left, right)
    cdef int _cmp_c_impl(left, right) except -2

we should have

    cpdef int _cmp_(left, right) except -2

Analogously for _richcmp_.

There is one important functional change: If a Python class defines both __cmp__ and _cmp_, then formerly the coercion framework would use __cmp__ by default to implement relational operators like >=. After this ticket, _cmp_ will be tried first, which defaults to __cmp__.

Change History (10)

comment:1 Changed 7 years ago by jdemeyer

  • Description modified (diff)
  • Summary changed from Remove _cmp_c_impl to Remove _(rich)cmp_c_impl

comment:2 Changed 7 years ago by jdemeyer

  • Description modified (diff)

comment:3 Changed 7 years ago by jdemeyer

  • Description modified (diff)

comment:4 Changed 7 years ago by jdemeyer

  • Branch set to u/jdemeyer/ticket/17890

comment:5 Changed 7 years ago by jdemeyer

  • Commit set to a830cc93c6bb07565db709bfe03f8acc62034dbb
  • Description modified (diff)

New commits:

7083f19Remove use of PY_IS_NUMERIC
7eb0e93Move py_scalar_to_element to coerce.pyx
8300f59trac #17862 (review): doc typo
a3ea57fMerge tag '6.6.beta2' into HEAD
a830cc9Remove _cmp_c_impl and _richcmp_c_impl

comment:6 follow-up: Changed 7 years ago by vdelecroix

If it is slower this is very bad. Do you know why? Isn't a cdef function call equivalent to cpdef function call?

Vincent

comment:7 in reply to: ↑ 6 Changed 7 years ago by jdemeyer

Replying to vdelecroix:

If it is slower this is very bad. Do you know why?

Yes, see https://groups.google.com/forum/#!topic/sage-devel/AXYk1uEfuPE

comment:8 Changed 7 years ago by mmezzarobba

Related: #10130.

comment:9 Changed 7 years ago by git

  • Commit changed from a830cc93c6bb07565db709bfe03f8acc62034dbb to 25e517fc735d15ba07c919f7a65103cf04acd2cf

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

25e517fRemove _cmp_c_impl and _richcmp_c_impl

comment:10 Changed 7 years ago by jdemeyer

  • Description modified (diff)
Note: See TracTickets for help on using tickets.