Opened 3 years ago

Closed 3 years ago

Last modified 2 years ago

#22036 closed enhancement (fixed)

py3 remove cmp() in comparison of padics printing modes (pyx)

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

Description

as yet another infinitesimal step to py3

Change History (20)

comment:1 Changed 3 years ago by chapoton

  • Branch set to u/chapoton/22036
  • Commit set to fa5cf71569f3b1ef8c27ffb518e6a20a062f1267
  • Status changed from new to needs_review

Last 10 new commits:

446e2c0trac 15634 using sorting_key for dendriform operad also
66e5b7cMerge branch 'u/chapoton/15634' into 7.3.b6
beb1ffdtrac 15634 correct doctest in C3controlled
68a0edcMerge branch 'u/chapoton/15634' in 7.3.b7
eb28edcMerge branch 'u/chapoton/15634' in 7.4.b6
aee01bbtrac 15634 modern imports
4eb9043Merge branch 'u/chapoton/15634' in 7.5.b2
4a42e29trac 15634 remove xrange, full doc
df0b014Merge branch 'u/chapoton/15634' in 7.5.b5
fa5cf71py3 remove cmp() in comparison of padics printing modes

comment:2 Changed 3 years ago by git

  • Commit changed from fa5cf71569f3b1ef8c27ffb518e6a20a062f1267 to 83e491b90042142a7314ab985adb26e1333d0254

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

83e491bpy3 remove cmp() in comparison of padics printing modes

comment:3 Changed 3 years ago by chapoton

  • Status changed from needs_review to needs_work

failing doctests

comment:4 Changed 3 years ago by git

  • Commit changed from 83e491b90042142a7314ab985adb26e1333d0254 to 6de5e273502deafdfca0736cd1984c890af8b79a

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

6de5e27trac 22036 trying to fix some doctests by switching to richcmp

comment:5 Changed 3 years ago by chapoton

still not ok, for some reason related to symbolic rings. Remains 2 failing doctests.

sage -t src/sage/rings/padics/factory.py # 2 doctests failed

Last edited 3 years ago by chapoton (previous) (diff)

comment:6 Changed 3 years ago by chapoton

Let me summarize the problem:

sage: x = var('x')
sage: X.<a> = Qq(27, modulus = x^3 + 2*x + 1)
sage: P.<x> = ZZ[]
sage: R.<a> = Qq(27, modulus = x^3 + 2*x + 1)
sage: X == R
False
sage: X._cmp_(R)
1
sage: X.__richcmp__(R, 2)
True

So the default behavior for == is apparently to call _cmp_ in Parent which calls __cmp__. Therefore one may have to change the behaviour in Parent to first call __richcmp__ before being able to handle this.

comment:7 Changed 3 years ago by git

  • Commit changed from 6de5e273502deafdfca0736cd1984c890af8b79a to 42d4ef609ece092b9a2daf7bc39f7ead455d6972

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

d24b715Merge branch 'u/chapoton/22036' in 7.5.b6
42d4ef6trac 22036 using __eq__ and __ne__ in padic extensions

comment:8 Changed 3 years ago by chapoton

  • Status changed from needs_work to needs_review

comment:9 Changed 3 years ago by chapoton

  • Cc tscrim jdemeyer aapitzsch added

green bot, please review

comment:10 Changed 3 years ago by chapoton

ping ?

comment:11 Changed 3 years ago by chapoton

PING ?

comment:12 Changed 3 years ago by tscrim

  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_review to positive_review

LGTM.

comment:13 Changed 3 years ago by chapoton

  • Milestone changed from sage-7.5 to sage-7.6

comment:14 Changed 3 years ago by vbraun

  • Status changed from positive_review to needs_work

Merge conflict

comment:15 Changed 3 years ago by git

  • Commit changed from 42d4ef609ece092b9a2daf7bc39f7ead455d6972 to ebc6d42cde90f73601a078ee3dc7ccc2ef237c24

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

ebc6d42Merge branches 'develop' in 7.6.b0

comment:16 Changed 3 years ago by chapoton

  • Status changed from needs_work to positive_review

rebased on the latest beta, but there were no conflict

Last edited 3 years ago by chapoton (previous) (diff)

comment:17 Changed 3 years ago by vbraun

  • Branch changed from u/chapoton/22036 to ebc6d42cde90f73601a078ee3dc7ccc2ef237c24
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:18 Changed 2 years ago by jdemeyer

  • Commit ebc6d42cde90f73601a078ee3dc7ccc2ef237c24 deleted

WTF? You implemented __richcmp__ in a pure Python file? That doesn't do anything...

comment:19 Changed 2 years ago by chapoton

indeed.. :(

comment:20 Changed 2 years ago by tscrim

We put it there as a perfect use-case for #23102. :P

Note: See TracTickets for help on using tickets.