Opened 6 years ago

Closed 6 years ago

#21142 closed enhancement (fixed)

sort elliptic curves over QQ using a key (for py3)

Reported by: Frédéric Chapoton Owned by:
Priority: major Milestone: sage-7.3
Component: elliptic curves Keywords:
Cc: John Cremona, wuthrich, David Roe, Jeroen Demeyer Merged in:
Authors: Frédéric Chapoton Reviewers: John Cremona, Jeroen Demeyer
Report Upstream: N/A Work issues:
Branch: 4abf3d2 (Commits, GitHub, GitLab) Commit: 4abf3d2d19cbce8de8dcd9f95a52d1fac42af633
Dependencies: Stopgaps:

Status badges

Description

in order to avoid using cmp (not py3 compatible) let us use a key function to sort elliptic curves over QQ.

Change History (12)

comment:1 Changed 6 years ago by Frédéric Chapoton

Branch: public/21142
Cc: John Cremona wutrich added
Commit: 323b4501113ba9543ff326e7a60b87639de93c8c
Status: newneeds_review

New commits:

323b450sorting using a key instead of cmp for elliptic curves over QQ

comment:2 Changed 6 years ago by Frédéric Chapoton

Cc: wuthrich David Roe Jeroen Demeyer added; wutrich removed

comment:3 Changed 6 years ago by John Cremona

Thanks for doing this. Instead of using the string (the middle part of the label) it would be better to use the numerical version as produced by applying the function sage.databases.cremona.class_to_int() since the string represents a number in base 26 and string sorting will get the order wrong.

comment:4 Changed 6 years ago by git

Commit: 323b4501113ba9543ff326e7a60b87639de93c8c666090a008cfaedbd6cce942c023a2fa3ddb8d2e

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

666090atrac #21142 use class_to_int for the label

comment:5 Changed 6 years ago by Frédéric Chapoton

done

comment:6 Changed 6 years ago by Jeroen Demeyer

Shouldn't curve_cmp be deprecated?

comment:7 Changed 6 years ago by git

Commit: 666090a008cfaedbd6cce942c023a2fa3ddb8d2e4abf3d2d19cbce8de8dcd9f95a52d1fac42af633

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

4abf3d2trac 21142 deprecate curve_cmp

comment:8 Changed 6 years ago by Frédéric Chapoton

done

comment:9 Changed 6 years ago by John Cremona

Thanks! I am happy with this if the tests pass.

comment:10 Changed 6 years ago by Frédéric Chapoton

Reviewers: John Cremona, Jeroen Demeyer

ok, bot is green. I am setting to positive review.

comment:11 Changed 6 years ago by Frédéric Chapoton

Status: needs_reviewpositive_review

comment:12 Changed 6 years ago by Volker Braun

Branch: public/211424abf3d2d19cbce8de8dcd9f95a52d1fac42af633
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.