Opened 4 years ago

Closed 4 years ago

#22890 closed enhancement (fixed)

py3: change doc for comparison in element classes

Reported by: chapoton Owned by:
Priority: major Milestone: sage-8.0
Component: python3 Keywords:
Cc: tscrim, jdemeyer, aapitzsch Merged in:
Authors: Frédéric Chapoton Reviewers: Jeroen Demeyer
Report Upstream: N/A Work issues:
Branch: df4c150 (Commits, GitHub, GitLab) Commit: df4c1504395131fc1948fedf52934718d0e667fc
Dependencies: Stopgaps:

Status badges

Description (last modified by chapoton)

with suggestion to use _richcmp_

may help to fix one of the doctest failures in #22297

Change History (12)

comment:1 Changed 4 years ago by chapoton

  • Branch set to u/chapoton/22890
  • Cc tscrim jdemeyer aapitzsch added
  • Commit set to 89d245dfb738f78a6be12092e594038250adef7d
  • Status changed from new to needs_review

New commits:

89d245dpy3: change doc for comparison in element classes

comment:2 Changed 4 years ago by jdemeyer

  • Status changed from needs_review to needs_info

Why is _richcmp_ recommended?

comment:3 follow-up: Changed 4 years ago by chapoton

Because most of the time, writing either a _cmp_ or a __cmp__ implies a call to cmp..

By the way, Jeroen, could you have a look at #22881, please ?

and maybe also to #22875, if you feel like..

comment:4 Changed 4 years ago by chapoton

  • Description modified (diff)

comment:5 Changed 4 years ago by chapoton

Jeroen, could you please say what you want here more precisely ?

comment:6 Changed 4 years ago by chapoton

ping ?

comment:7 in reply to: ↑ 3 Changed 4 years ago by jdemeyer

  • Status changed from needs_info to needs_work

Replying to chapoton:

Because most of the time, writing either a _cmp_ or a __cmp__ implies a call to cmp..

I don't really follow this reasoning. It's obviously true that cmp() should be avoided because that doesn't work in Python 3. But just because some (or even most) implementations of _cmp_() call cmp(), that doesn't mean that _cmp_() should be avoided in general.

If you really want to say that cmp() should be avoided, then just say that instead of something else.

comment:8 Changed 4 years ago by git

  • Commit changed from 89d245dfb738f78a6be12092e594038250adef7d to df4c1504395131fc1948fedf52934718d0e667fc

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

da00b47Merge branch 'u/chapoton/22890' in 8.0.b6
df4c150trac 22890 better doc

comment:9 Changed 4 years ago by chapoton

Better like that ?

comment:10 Changed 4 years ago by chapoton

  • Status changed from needs_work to needs_review

comment:11 Changed 4 years ago by jdemeyer

  • Reviewers set to Jeroen Demeyer
  • Status changed from needs_review to positive_review

comment:12 Changed 4 years ago by vbraun

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