Ticket #12517 (closed enhancement: fixed)
EllipticCurve(E.a_invariants()) doesn't work
| Reported by: | was | Owned by: | cremona |
|---|---|---|---|
| Priority: | minor | Milestone: | sage-5.0 |
| Component: | elliptic curves | Keywords: | |
| Cc: | Work issues: | ||
| Report Upstream: | N/A | Reviewers: | Robert Bradshaw |
| Authors: | William Stein | Merged in: | sage-5.0.beta5 |
| Dependencies: | Stopgaps: |
Description
At some point E.a_invariants() was changed to return a 5-tuple instead of a list of 5 numbers. I have no idea why. But this means that:
sage: E = EllipticCurve([1..5]) sage: EllipticCurve(E.a_invariants()) TypeError: invalid input to EllipticCurve constructor
Ugh. Fix this, i.e., make the constructor take a tuple or a list.
Attachments
Change History
comment:1 Changed 15 months ago by was
- Status changed from new to needs_review
- Authors set to William Stein
comment:3 Changed 15 months ago by cremona
See #4262 for the origin of this change, after a report by was (!) and #4264 by the patch which implemented it written mosstly by me. As it says at that ticket, I did (of course) try to get the EllipticCurve? constructor to accept tuples, but for some reason at the time I failed to get it to work. So either you people are cleverer now than you or I were then, or other changes have made that easier.
Of course I approve of the change!
comment:4 follow-up: ↓ 5 Changed 15 months ago by was
All my patch does is change an "isinstance(., list)" to "isinstance(., (list, tuple))" in one line. That's it. You worry me.
comment:5 in reply to: ↑ 4 Changed 15 months ago by cremona
Replying to was:
All my patch does is change an "isinstance(., list)" to "isinstance(., (list, tuple))" in one line. That's it. You worry me.
Don't worry, it was over 3 years ago and needed masses of doctest output changes which is the main thing I remember doing. Looking at my old comments in #4264 it looks as if I was trying to change {{{init}} functions in classes instead of what you did.

