Opened 4 years ago
Closed 4 years ago
#25053 closed enhancement (fixed)
py3: remove __cmp__ in free monoids
Reported by:  chapoton  Owned by:  

Priority:  major  Milestone:  sage8.2 
Component:  python3  Keywords:  python3, richcmp 
Cc:  embray, jdemeyer, fbissey  Merged in:  
Authors:  Frédéric Chapoton  Reviewers:  Jeroen Demeyer, Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  26de175 (Commits, GitHub, GitLab)  Commit:  26de175f2507e38a68e127d852bfad1ebfecb076 
Dependencies:  Stopgaps: 
Description
part of #16537
Change History (15)
comment:1 Changed 4 years ago by
 Branch set to u/chapoton/23053
 Commit set to 2d164093043f15e86412bf3d9d9bea0a709ebc5e
 Status changed from new to needs_review
comment:2 Changed 4 years ago by
 Status changed from needs_review to needs_work
many failing doctests
comment:3 Changed 4 years ago by
 Commit changed from 2d164093043f15e86412bf3d9d9bea0a709ebc5e to f8d87288fb6d681a57750d188c093b5a578d4143
comment:4 Changed 4 years ago by
 Status changed from needs_work to needs_review
should be better now
comment:6 Changed 4 years ago by
 Status changed from needs_review to needs_work
Can this actually happen?
if not isinstance(right, FreeMonoidElement)
Certainly, this cannot happen in _richcmp_
:
left.parent() != right.parent()):
comment:7 Changed 4 years ago by
Also: please use self, other
instead of left, right
.
comment:8 Changed 4 years ago by
 Commit changed from f8d87288fb6d681a57750d188c093b5a578d4143 to 9405cde60dc9d6b00cdb608eb5334d8d12786939
Branch pushed to git repo; I updated commit sha1. New commits:
9405cde  more details for _richcmp_ in free monoids

comment:10 Changed 4 years ago by
bot is green again
comment:11 followup: ↓ 14 Changed 4 years ago by
Minor point, but it is quite a bit faster to do tuple([gen])
than tuple(gen)
:
sage: %timeit tuple([x for x in range(1000)]) 10000 loops, best of 3: 28.1 µs per loop sage: %timeit tuple(x for x in range(1000)) 10000 loops, best of 3: 40.6 µs per loop
comment:12 Changed 4 years ago by
 Commit changed from 9405cde60dc9d6b00cdb608eb5334d8d12786939 to 26de175f2507e38a68e127d852bfad1ebfecb076
Branch pushed to git repo; I updated commit sha1. New commits:
26de175  trac 25053 some details

comment:13 Changed 4 years ago by
 Reviewers set to Jeroen Demeyer, Travis Scrimshaw
 Status changed from needs_review to positive_review
LGTM.
comment:14 in reply to: ↑ 11 Changed 4 years ago by
Replying to tscrim:
Minor point, but it is quite a bit faster to do
tuple([gen])
thantuple(gen)
:
Weird, why do you think that is? And does that hold for even larger generators? Seems like it's just unnecessary duplication to me.
comment:15 Changed 4 years ago by
 Branch changed from u/chapoton/23053 to 26de175f2507e38a68e127d852bfad1ebfecb076
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
remove __cmp__ in free monoids