#30130 closed enhancement (fixed)

get rid of comparison using _cmp_

Reported by: chapoton Owned by:
Priority: major Milestone: sage-9.2
Component: refactoring Keywords:
Cc: tscrim Merged in:
Authors: Frédéric Chapoton Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: e6963fb (Commits, GitHub, GitLab) Commit: e6963fb961493f39d1f7a186e263144b1fb4398f
Dependencies: Stopgaps:

Status badges

Description

and use _richcmp_ instead

So far, this remains used only in complex MPC and Integer classes.

Change History (9)

comment:1 Changed 22 months ago by chapoton

  • Branch set to u/chapoton/30130
  • Commit set to 066b9d98ff5b850bb98c151be4ad9a80e3c32f82
  • Status changed from new to needs_info

This seems to reveal a bug in the pickling of complex numbers. Namely

sage: C = MPComplexField(prec=200, rnd='RNDUU')
sage: b = C(393.39203845902384098234098230948209384028340)
sage: loads(dumps(b))
393.39203845902384098234098230948209384028340000000000000000 + NaN*I

New commits:

066b9d9get rid of _cmp_ comparison

comment:2 Changed 22 months ago by git

  • Commit changed from 066b9d98ff5b850bb98c151be4ad9a80e3c32f82 to 44e743413a43d6f8648844c464edf3d249e365e7

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

44e7434fix the bug in complex_mpc (for string entry without imaginary part)

comment:3 Changed 22 months ago by chapoton

  • Cc tscrim added
  • Status changed from needs_info to needs_review

Should be good to go now. Let us wait for the patchbot report.

comment:4 Changed 22 months ago by tscrim

  • Reviewers set to Travis Scrimshaw

What a glorious day. Green bot => positive review.

comment:5 Changed 22 months ago by git

  • Commit changed from 44e743413a43d6f8648844c464edf3d249e365e7 to e6963fb961493f39d1f7a186e263144b1fb4398f

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

e6963fbfix doc and doctests about _cmp_

comment:6 Changed 22 months ago by chapoton

some trivial doctest failures. Patchbot re-launched

comment:7 Changed 22 months ago by chapoton

One fast patchbot is morally green. Travis, do you agree with my last commit ?

comment:8 Changed 22 months ago by tscrim

  • Status changed from needs_review to positive_review

Yep. LGTM. Thanks.

comment:9 Changed 22 months ago by vbraun

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