Opened 4 years ago

Closed 4 years ago

#23261 closed enhancement (fixed)

py3: some care for cmp in modular/overconvergent

Reported by: chapoton Owned by:
Priority: major Milestone: sage-8.0
Component: python3 Keywords:
Cc: tscrim, jdemeyer, jhpalmieri Merged in:
Authors: Frédéric Chapoton Reviewers: John Palmieri
Report Upstream: N/A Work issues:
Branch: 400cdcc (Commits, GitHub, GitLab) Commit: 400cdccf08f87b38ec5eca98efd12a6a23c3f77b
Dependencies: Stopgaps:

Status badges

Description

as another tiny step to py3

Change History (11)

comment:1 Changed 4 years ago by chapoton

  • Branch set to u/chapoton/23261
  • Commit set to 61fa91ff0b346bf8afefaf1eaeed5e321de55de8

comment:2 Changed 4 years ago by git

  • Commit changed from 61fa91ff0b346bf8afefaf1eaeed5e321de55de8 to dbec83113e162d153c49c298cffda30b8696de57

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

dbec831work on cmp in overconvergent

comment:3 Changed 4 years ago by git

  • Commit changed from dbec83113e162d153c49c298cffda30b8696de57 to 400cdccf08f87b38ec5eca98efd12a6a23c3f77b

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

400cdcctrac 23261 wrong imports

comment:4 Changed 4 years ago by chapoton

  • Cc tscrim jdemeyer jhpalmieri added
  • Status changed from new to needs_review

bot is morally green, please review

comment:5 Changed 4 years ago by jhpalmieri

In genus0.py, why convert one __cmp__ to __eq__ and __ne__, while converting the other to _richcmp_?

comment:6 Changed 4 years ago by chapoton

_richcmp_ (and the former _cmp_) can be used only for Element classes, and will be used by the coercion framework.

On the other hand, if often makes no sense to define a total order on a set of parents. That's why __cmp__ is replaced by just __eq__ and __ne__.

EDIT When comparison is really wanted for parents, one soon will be able to use __richcmp__, which is for the moment restricted to .pyx files.

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

comment:7 Changed 4 years ago by jhpalmieri

For these elements, it seems that we only care about __eq__ and __ne__, not a total ordering. Or at least the doctests don't show any interest in a total ordering. Is _richcmp_ preferable anyway because it uses coercion, or for some other reason?

(I am not objecting to the changes, just curious.)

comment:8 Changed 4 years ago by chapoton

Well, I would see at least two reasons for using _richcmp_, namely

# this is easy to use, as a quick replacement for __cmp__ or _cmp_

Given the quantity of work remaining to be done on cmp, this is a big plus.

# it uses coercion

this allows in particular to remove all checks of the kind "do they have the same parent"

comment:9 Changed 4 years ago by jhpalmieri

  • Reviewers set to John Palmieri
  • Status changed from needs_review to positive_review

comment:10 Changed 4 years ago by chapoton

Thanks !

comment:11 Changed 4 years ago by vbraun

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