#8815: Misc elliptic curve typo fixes (easy review)
-------------------------------+--------------------------------------------
Reporter: robertwb | Owner: cremona
Type: defect | Status: needs_work
Priority: major | Milestone: sage-4.4.1
Component: elliptic curves | Keywords:
Author: | Upstream: N/A
Reviewer: | Merged:
Work_issues: |
-------------------------------+--------------------------------------------
Changes (by cremona):
* status: needs_review => needs_work
Comment:
Patch applies to 4.4; tests pass (I checked all
sage/schemes/elliptic_curves).
In the first file patched, was there a reason for not changing the
TypeError in lines 1093/4 to another ValueError?
In the second: there are essentially identical blocks of code in lines
1052-1068 and 1360-1376. But the typo (s to z) was only fixed in one
place. So the other needs fixing too.
In the "logic simplification" bit of the patch -- this is not correct now!
If z's parent is (say) QQ then after the patch, z will have been coerced
into CC but C will be set to QQ. So this now fails:
{{{
sage: E = EllipticCurve('14a1')
sage: L = E.period_lattice()
sage: L.coordinates(1)
}}}
Finally, your description is a little unkind! I spent a long time trying
this out -- though I do admit, of course, that I clearly did not test
every line. Perhaps we need to add some more doctests?
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/8815#comment:2>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en.
