Opened 4 years ago
Closed 4 years ago
#22446 closed enhancement (fixed)
py3: small cmp step (cmp() in doc, __cmp__ in code)
Reported by:  chapoton  Owned by:  

Priority:  major  Milestone:  sage7.6 
Component:  python3  Keywords:  
Cc:  tscrim, jmantysalo, jdemeyer, aapitzsch  Merged in:  
Authors:  Frédéric Chapoton  Reviewers:  Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  fd787c6 (Commits, GitHub, GitLab)  Commit:  fd787c628de38d902e19475cd460a3dd194a8072 
Dependencies:  Stopgaps: 
Description
a small step to py3
Change History (10)
comment:1 Changed 4 years ago by
 Branch set to u/chapoton/22446
 Commit set to 220c687e9a77ad7ea4fefa964bae38cf479d4b9d
 Status changed from new to needs_review
comment:2 Changed 4 years ago by
 Cc tscrim jmantysalo jdemeyer aapitzsch added
green bot, please review
comment:3 Changed 4 years ago by
 Reviewers set to Travis Scrimshaw
 Status changed from needs_review to needs_work
You run into subtle problems by only defining __eq__
and not __ne__
:
sage: R.<x> = PolynomialRing(QQ) sage: f = x^4  17*x^3  2*x + 1 sage: A = f.galois_group(pari_group=True) sage: B = f.galois_group(pari_group=True) sage: A is B False sage: A == B True sage: A != B True
This comes from by Python falling back to __ne__
by id
.
comment:4 Changed 4 years ago by
 Commit changed from 220c687e9a77ad7ea4fefa964bae38cf479d4b9d to 01d2fc015258a0ede5ffeafc1237a701df023349
comment:5 Changed 4 years ago by
 Status changed from needs_work to needs_review
ok, thanks. Done. Let us wait for the bots again (my personal bot seems to be ill).
comment:6 Changed 4 years ago by
 Commit changed from 01d2fc015258a0ede5ffeafc1237a701df023349 to 77a8187f9f0aae97b17b315b81305f969cc703a7
Branch pushed to git repo; I updated commit sha1. New commits:
77a8187  trac 22446 adding some doc to pari_group

comment:7 Changed 4 years ago by
I've been told in the past (I think by Jeroen) that instead of return not self.__eq__(other)
, you should do return not (self == other)
. I think this is the right way because it allows Python to switch the order of the two objects in case self.__eq__(other)
fails. Otherwise LGTM.
comment:8 Changed 4 years ago by
 Commit changed from 77a8187f9f0aae97b17b315b81305f969cc703a7 to fd787c628de38d902e19475cd460a3dd194a8072
Branch pushed to git repo; I updated commit sha1. New commits:
fd787c6  trac 22446 reviewer's suggestion

comment:10 Changed 4 years ago by
 Branch changed from u/chapoton/22446 to fd787c628de38d902e19475cd460a3dd194a8072
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
py3 turn a few `__cmp__` to `__eq__` in some py files, remove cmp() in doc