Opened 14 years ago

Closed 13 years ago

#4822 closed enhancement (fixed)

[with patch, positive review] Tweak to the error message for EllipticCurve

Reported by: L J P Kilford Owned by: David Loeffler
Priority: minor Milestone: sage-4.1.2
Component: elliptic curves Keywords:
Cc: Merged in: Sage 4.1.2.alpha0
Authors: John Cremona Reviewers: Chris Wuthrich
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

I was using SAGE with the small version of the CremonaDatabase?, and tried the following, which does not work because the conductor is too high:

EllipticCurve("10001a1")

I think it would be useful if the error message not only said "this curve is not in the database" (which is indeed true) but also checked to see if one was using the small database of curves, and if so told the user how to access the larger version using the incantation

!sage -i database_cremona_ellcurve-2005.11.03

or otherwise.

Attachments (2)

trac_4822-ecdb_error_msg.patch (1.8 KB) - added by John Cremona 13 years ago.
Applies to 4.1.1
trac_4822-doctest-fix.patch (825 bytes) - added by John Cremona 13 years ago.
Apply after previous

Download all attachments as: .zip

Change History (8)

comment:1 Changed 14 years ago by Robert Miller

Type: defectenhancement

comment:2 Changed 13 years ago by David Loeffler

Component: number theoryelliptic curves
Owner: changed from William Stein to David Loeffler

Changed 13 years ago by John Cremona

Applies to 4.1.1

Changed 13 years ago by John Cremona

Attachment: trac_4822-doctest-fix.patch added

Apply after previous

comment:3 Changed 13 years ago by John Cremona

Summary: Tweak to the error message for EllipticCurve[with patch, needs review] Tweak to the error message for EllipticCurve

The patch deals with this in a more intelligent way. I did not add doctests since it's hard to do that when the output message depends on whether or not the extra database is installed. But I did test it before and after installing the optional database. The message suggests installing the optional database iff the desired conductor is betweem 10000 and 13000 and the optional db is not already installed; I did not actually include the command-line to install it though.

The second patch fixes a doctest which otherwise fails when run after installing the optional database (but is otherwise independent of this ticket, except that I wrote it so was my fault anyway!)

comment:4 Changed 13 years ago by wuthrich

Summary: [with patch, needs review] Tweak to the error message for EllipticCurve[with patch, under review] Tweak to the error message for EllipticCurve

The patch is ok (so far only tested without the optional package). I will test it with it later today.

Maybe while we are at it I could suggest that the same change is done to cremona_label(). This is without the optional package:

sage: E= EllipticCurve([3,10])
sage: E.cremona_label()
---------------------------------------------------------------------------
RuntimeError                              Traceback (most recent call last)

/home/chrigu/.sage/temp/linux_ljo8/12909/_home_chrigu__sage_init_sage_0.py in <module>()

/usr/local/sage/local/lib/python2.6/site-packages/sage/schemes/elliptic_curves/ell_rational_field.pyc in cremona_label(self, space)
   3305                 X = self.database_curve()
   3306             except RuntimeError:
-> 3307                 raise RuntimeError, "Cremona label not known for %s."%self
   3308             self.__cremona_label = X.__cremona_label
   3309             return self.cremona_label(space)

RuntimeError: Cremona label not known for Elliptic Curve defined by y^2 = x^3 + 3*x + 10 over Rational Field.
sage: E.conductor()
44928

Chris.

ps : 'I don't know if there is an official 'under review'.

comment:5 Changed 13 years ago by wuthrich

Summary: [with patch, under review] Tweak to the error message for EllipticCurve[with patch, positive review] Tweak to the error message for EllipticCurve

This patch also works with the optional package. I give this ticket a positive review and I agree that the command-line to install the optional package should better not be included. It is very clear what to do as it is now.

The second [independent] patch obtains also a positive review with this.

I propose to address the issue that I raised on cremona_label() in a new ticket.

Chris.

comment:6 Changed 13 years ago by Minh Van Nguyen

Authors: John Cremona
Merged in: Sage 4.1.2.alpha0
Resolution: fixed
Reviewers: Chris Wuthrich
Status: newclosed

Merged both patches.

Note: See TracTickets for help on using tickets.