Opened 7 years ago

Closed 7 years ago

#21145 closed enhancement (fixed)

deprecate some cmp keyword in favor of key in factorizations

Reported by: chapoton Owned by:
Priority: major Milestone: sage-7.3
Component: python3 Keywords:
Cc: tscrim, jmantysalo, jdemeyer Merged in:
Authors: Frédéric Chapoton Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 930414a (Commits, GitHub, GitLab) Commit: 930414a09e73fb7429418cbeb9bf3a7634e2fd3d
Dependencies: Stopgaps:

Status badges

Description

as a step towards python3

let us try to get rid of cmp= in factorizations

Change History (14)

comment:1 Changed 7 years ago by chapoton

Authors: Frédéric Chapoton
Branch: public/21145
Commit: 6406454101de9ef8444789a2911f08f1db226c2e
Status: newneeds_info

New commits:

6406454trac 21145 first tentative

comment:2 Changed 7 years ago by git

Commit: 6406454101de9ef8444789a2911f08f1db226c2e3b727230c132f3e6525614a87628ac6cd13d8395

Branch pushed to git repo; I updated commit sha1. New commits:

3b72723trac 21145 more work on deprecating cmp in factorisations

comment:3 Changed 7 years ago by git

Commit: 3b727230c132f3e6525614a87628ac6cd13d8395d23c60af6bc24cc8a218d3259732d1132028ff88

Branch pushed to git repo; I updated commit sha1. New commits:

d23c60ano change to ordered tree (moved to another ticket)

comment:4 Changed 7 years ago by git

Commit: d23c60af6bc24cc8a218d3259732d1132028ff8824f03209d7e8cc03bab9ddc2eeba3032863d9523

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

24f0320trac 21145 work on cmp in factorisations

comment:5 Changed 7 years ago by git

Commit: 24f03209d7e8cc03bab9ddc2eeba3032863d9523d2531cff9ef52d2534b9f7a867b47cd3f3cf0dd6

Branch pushed to git repo; I updated commit sha1. New commits:

d2531cftrac #21145 fixing one failing doctest

comment:6 Changed 7 years ago by chapoton

Cc: tscrim jmantysalo jdemeyer added
Status: needs_infoneeds_review

comment:7 Changed 7 years ago by chapoton

bot is green !

comment:8 Changed 7 years ago by aapitzsch

Calls like self.__x.sort(cmp=_cmp) will crash in python 3. I suggest you use cmp_to_key() from functools: self.__x.sort(key=cmp_to_key(_cmp))

comment:9 Changed 7 years ago by tscrim

Reviewers: Travis Scrimshaw
Status: needs_reviewpositive_review

LGTM.

@aapitzsch Those should go away by the time we switch to Python3 (hopefully we will be there before EOL for Python2).

comment:10 Changed 7 years ago by tscrim

Status: positive_reviewneeds_work

Actually, I now understand Andre's comment, we should change

-self.__x.sort(cmp=_cmp)
+self.__x.sort(key=cmp_to_key(_cmp))

in the deprecated code to be python2/3 compatible now (I interpreted it as a long-term, sorry!).

comment:11 Changed 7 years ago by git

Commit: d2531cff9ef52d2534b9f7a867b47cd3f3cf0dd6930414a09e73fb7429418cbeb9bf3a7634e2fd3d

Branch pushed to git repo; I updated commit sha1. New commits:

930414atrac #21145 using cmp_to_key in deprecated code

comment:12 Changed 7 years ago by chapoton

Status: needs_workneeds_review

comment:13 Changed 7 years ago by tscrim

Status: needs_reviewpositive_review

Thanks, and sorry I didn't understand it earlier.

comment:14 Changed 7 years ago by vbraun

Branch: public/21145930414a09e73fb7429418cbeb9bf3a7634e2fd3d
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.