Opened 8 years ago

Last modified 8 years ago

#17890 closed enhancement

Remove _(rich)cmp_c_impl — at Version 10

Reported by: Jeroen Demeyer Owned by:
Priority: major Milestone: sage-6.7
Component: cython Keywords:
Cc: Daniel Krenn 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 Jeroen Demeyer)

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 8 years ago by Jeroen Demeyer

Description: modified (diff)
Summary: Remove _cmp_c_implRemove _(rich)cmp_c_impl

comment:2 Changed 8 years ago by Jeroen Demeyer

Description: modified (diff)

comment:3 Changed 8 years ago by Jeroen Demeyer

Description: modified (diff)

comment:4 Changed 8 years ago by Jeroen Demeyer

Branch: u/jdemeyer/ticket/17890

comment:5 Changed 8 years ago by Jeroen Demeyer

Commit: 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 Changed 8 years ago by Vincent Delecroix

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 8 years ago by Jeroen Demeyer

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 8 years ago by Marc Mezzarobba

Related: #10130.

comment:9 Changed 8 years ago by git

Commit: a830cc93c6bb07565db709bfe03f8acc62034dbb25e517fc735d15ba07c919f7a65103cf04acd2cf

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 8 years ago by Jeroen Demeyer

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