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: |
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
Authors: | → Frédéric Chapoton |
---|---|
Branch: | → public/21145 |
Commit: | → 6406454101de9ef8444789a2911f08f1db226c2e |
Status: | new → needs_info |
comment:2 Changed 7 years ago by
Commit: | 6406454101de9ef8444789a2911f08f1db226c2e → 3b727230c132f3e6525614a87628ac6cd13d8395 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
3b72723 | trac 21145 more work on deprecating cmp in factorisations
|
comment:3 Changed 7 years ago by
Commit: | 3b727230c132f3e6525614a87628ac6cd13d8395 → d23c60af6bc24cc8a218d3259732d1132028ff88 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
d23c60a | no change to ordered tree (moved to another ticket)
|
comment:4 Changed 7 years ago by
Commit: | d23c60af6bc24cc8a218d3259732d1132028ff88 → 24f03209d7e8cc03bab9ddc2eeba3032863d9523 |
---|
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
24f0320 | trac 21145 work on cmp in factorisations
|
comment:5 Changed 7 years ago by
Commit: | 24f03209d7e8cc03bab9ddc2eeba3032863d9523 → d2531cff9ef52d2534b9f7a867b47cd3f3cf0dd6 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
d2531cf | trac #21145 fixing one failing doctest
|
comment:6 Changed 7 years ago by
Cc: | tscrim jmantysalo jdemeyer added |
---|---|
Status: | needs_info → needs_review |
comment:8 Changed 7 years ago by
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
Reviewers: | → Travis Scrimshaw |
---|---|
Status: | needs_review → positive_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
Status: | positive_review → needs_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
Commit: | d2531cff9ef52d2534b9f7a867b47cd3f3cf0dd6 → 930414a09e73fb7429418cbeb9bf3a7634e2fd3d |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
930414a | trac #21145 using cmp_to_key in deprecated code
|
comment:12 Changed 7 years ago by
Status: | needs_work → needs_review |
---|
comment:13 Changed 7 years ago by
Status: | needs_review → positive_review |
---|
Thanks, and sorry I didn't understand it earlier.
comment:14 Changed 7 years ago by
Branch: | public/21145 → 930414a09e73fb7429418cbeb9bf3a7634e2fd3d |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
New commits:
trac 21145 first tentative