Sage: Ticket #5346: [with patch, positive review] Some doctests in schemes/elliptic_curves/ell_rational_field.py fail with optional database installed
https://trac.sagemath.org/ticket/5346
<p>
Reported by Jan Groenewald in <a class="ext-link" href="http://groups.google.com/group/sage-devel/browse_thread/thread/d5db5c25fbce1e99"><span class="icon"></span>http://groups.google.com/group/sage-devel/browse_thread/thread/d5db5c25fbce1e99</a>
</p>
<pre class="wiki">sage -t "devel/sage/sage/schemes/elliptic_curves/ell_rational_field.py"
**********************************************************************
File "/usr/local/src/sage-3.3/devel/sage/sage/schemes/elliptic_curves/ell_rational_field.py", line 2675:
sage: E.cremona_label()
Expected:
Traceback (most recent call last):
...
RuntimeError: Cremona label not known for Elliptic Curve defined by y^2 + x*y + 3*y = x^3 + 2*x^2 + 4*x + 5 over Rational Field.
Got:
'10351a1'
**********************************************************************
</pre>en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/5346
Trac 1.1.6mabshoffMon, 23 Feb 2009 08:19:16 GMT
https://trac.sagemath.org/ticket/5346#comment:1
https://trac.sagemath.org/ticket/5346#comment:1
<p>
This problem is ironically introduced by having database_cremona_ellcurve-20071019.spkg installed :)
</p>
<p>
Cheers,
</p>
<p>
Michael
</p>
TicketGeorgSWeberThu, 26 Mar 2009 23:05:25 GMT
https://trac.sagemath.org/ticket/5346#comment:2
https://trac.sagemath.org/ticket/5346#comment:2
<p>
For the record:
</p>
<p>
The optional database covers conductor ranges from 10000 to 130000 AFAIK. So an obviously working patch (to be tested ...) already discussed elsewhere (I don't remember exactly who had this idea first, it wasn't me) would be to exchange the curve with conductor 10351 in the doctest, with a curve having conductor bigger than 130000.
</p>
<p>
Cheers,
gsw
</p>
TicketcremonaFri, 27 Mar 2009 08:11:00 GMT
https://trac.sagemath.org/ticket/5346#comment:3
https://trac.sagemath.org/ticket/5346#comment:3
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/5346#comment:2" title="Comment 2">GeorgSWeber</a>:
</p>
<blockquote class="citation">
<p>
For the record:
</p>
<p>
The optional database covers conductor ranges from 10000 to 130000 AFAIK. So an obviously working patch (to be tested ...) already discussed elsewhere (I don't remember exactly who had this idea first, it wasn't me) would be to exchange the curve with conductor 10351 in the doctest, with a curve having conductor bigger than 130000.
</p>
</blockquote>
<p>
I agree, and suggest
</p>
<pre class="wiki">sage: E = EllipticCurve([1, -1, 0, -79, 289])
sage: E.cremona_label()
---------------------------------------------------------------------------
RuntimeError Traceback (most recent call last)
/home/john/.sage/temp/ubuntu/1126/_home_john__sage_init_sage_0.py in <module>()
/home/john/sage-3.4/local/lib/python2.5/site-packages/sage/schemes/elliptic_curves/ell_rational_field.pyc in cremona_label(self, space)
3034 X = self.database_curve()
3035 except RuntimeError:
-> 3036 raise RuntimeError, "Cremona label not known for %s."%self
3037 self.__cremona_label = X.__cremona_label
3038 return self.cremona_label(space)
RuntimeError: Cremona label not known for Elliptic Curve defined by y^2 + x*y = x^3 - x^2 - 79*x + 289 over Rational Field.
</pre><p>
as that curve will one day have label 234446a1 (or b1 or c1, I forget).
</p>
<blockquote class="citation">
<p>
Cheers,
gsw
</p>
</blockquote>
TicketcremonaThu, 23 Apr 2009 09:19:22 GMTattachment set
https://trac.sagemath.org/ticket/5346
https://trac.sagemath.org/ticket/5346
<ul>
<li><strong>attachment</strong>
set to <em>trac_5346.patch</em>
</li>
</ul>
<p>
Apply to 3.4.1
</p>
TicketcremonaThu, 23 Apr 2009 09:22:06 GMTsummary changed
https://trac.sagemath.org/ticket/5346#comment:4
https://trac.sagemath.org/ticket/5346#comment:4
<ul>
<li><strong>summary</strong>
changed from <em>Sage 3.3: schemes/elliptic_curves/ell_rational_field.py fails to doctest with optional database installed</em> to <em>[with patch, needs review] Some doctests in schemes/elliptic_curves/ell_rational_field.py fail with optional database installed</em>
</li>
</ul>
<p>
The patch cahnges taht example to the one with condictor 234446 as I suggested.
</p>
<p>
It also make another doctest work ok with & without the database (at line 4941) by hard-wiring in some points instead of getting the gens.
</p>
<p>
I tested this on sage-3.4.1 with & without the database installed.
</p>
TicketmabshoffFri, 24 Apr 2009 08:26:50 GMTsummary changed
https://trac.sagemath.org/ticket/5346#comment:5
https://trac.sagemath.org/ticket/5346#comment:5
<ul>
<li><strong>summary</strong>
changed from <em>[with patch, needs review] Some doctests in schemes/elliptic_curves/ell_rational_field.py fail with optional database installed</em> to <em>[with patch, positive review] Some doctests in schemes/elliptic_curves/ell_rational_field.py fail with optional database installed</em>
</li>
</ul>
<p>
Thanks for taking care of this John. Doctests pass, pass reads good and fixes the problem. Positve review.
</p>
<p>
Cheers,
</p>
<p>
Michael
</p>
TicketmabshoffFri, 24 Apr 2009 08:27:50 GMTstatus, milestone changed; resolution set
https://trac.sagemath.org/ticket/5346#comment:6
https://trac.sagemath.org/ticket/5346#comment:6
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>fixed</em>
</li>
<li><strong>milestone</strong>
changed from <em>sage-4.0</em> to <em>sage-3.4.2</em>
</li>
</ul>
<p>
Merged in Sage 3.4.2.alpha0.
</p>
<p>
Cheers,
</p>
<p>
Michael
</p>
Ticket