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: sage-7.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:

Status badges

Description

a small step to py3

Change History (10)

comment:1 Changed 4 years ago by chapoton

  • Branch set to u/chapoton/22446
  • Commit set to 220c687e9a77ad7ea4fefa964bae38cf479d4b9d
  • Status changed from new to needs_review

New commits:

​220c687py3 turn a few `__cmp__` to `__eq__` in some py files, remove cmp() in doc

comment:2 Changed 4 years ago by chapoton

  • Cc tscrim jmantysalo jdemeyer aapitzsch added

green bot, please review

comment:3 Changed 4 years ago by tscrim

  • 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 git

  • Commit changed from 220c687e9a77ad7ea4fefa964bae38cf479d4b9d to 01d2fc015258a0ede5ffeafc1237a701df023349

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

​4276954Merge branch 'u/chapoton/22446' in 7.6.b5
​01d2fc0trac 22446 adding __ne__

comment:5 Changed 4 years ago by chapoton

  • 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 git

  • Commit changed from 01d2fc015258a0ede5ffeafc1237a701df023349 to 77a8187f9f0aae97b17b315b81305f969cc703a7

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

​77a8187trac 22446 adding some doc to pari_group

comment:7 Changed 4 years ago by tscrim

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 git

  • Commit changed from 77a8187f9f0aae97b17b315b81305f969cc703a7 to fd787c628de38d902e19475cd460a3dd194a8072

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

​fd787c6trac 22446 reviewer's suggestion

comment:9 Changed 4 years ago by tscrim

  • Status changed from needs_review to positive_review

Thank you.

comment:10 Changed 4 years ago by vbraun

  • Branch changed from u/chapoton/22446 to fd787c628de38d902e19475cd460a3dd194a8072
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.