Opened 11 years ago
Closed 11 years ago
#11822 closed enhancement (fixed)
Wraps E.reduction(p)(P) so you can call P.reduction(p)
Reported by: | Alyson Deines | Owned by: | John Cremona |
---|---|---|---|
Priority: | minor | Milestone: | sage-5.0 |
Component: | elliptic curves | Keywords: | elliptic curves, reduction sd35.5 |
Cc: | Kate Stange | Merged in: | sage-5.0.beta1 |
Authors: | Aly Deines | Reviewers: | William Stein, Cassie Williams, Jeroen Demeyer |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
Given an elliptic curve E with a point P and good reduction at p, this wraps E.reduction(p)(P) to P.reduction(p).
Attachments (2)
Change History (18)
comment:1 Changed 11 years ago by
Cc: | Kate Stange added |
---|
comment:2 Changed 11 years ago by
Status: | new → needs_review |
---|
Changed 11 years ago by
Attachment: | trac_11822.patch added |
---|
comment:3 Changed 11 years ago by
Status: | needs_review → positive_review |
---|
The docstring was misformated (see http://wstein.org/home/wstein/tmp/trac11822.png). I've refreshed the patch and uploaded a fixed one. Positive review.
comment:4 Changed 11 years ago by
Reviewers: | → William Stein |
---|
comment:5 Changed 11 years ago by
It might be nice to have a doc example of the type
E = EllipticCurve(...) P = E(...) E.reduction(p).is_on_curve(P.reduction(p)) True
comment:6 Changed 11 years ago by
Status: | positive_review → needs_work |
---|
I added one line doctest to check that the reduction of the point is on the reduction of the curve. It probably needs_review again now, but I can't seem to set that flag.
comment:7 Changed 11 years ago by
Status: | needs_work → needs_review |
---|
comment:9 Changed 11 years ago by
Keywords: | sd35.5 added |
---|---|
Milestone: | → sage-5.0 |
Reviewers: | William Stein → William Stein, Cassie Williams |
comment:10 Changed 11 years ago by
Status: | needs_review → positive_review |
---|
Checked functionality in Sage, seems to be working as advertised. Positive review.
comment:11 Changed 11 years ago by
Sorry for the extra comment. Forgot to mention that the doctest passed 100% and the documentation looks good.
comment:12 Changed 11 years ago by
Description: | modified (diff) |
---|
comment:13 Changed 11 years ago by
Status: | positive_review → needs_work |
---|
Please fix the formatting of the documentation string to be consistent with the developer manual: there should be an empty line after INPUT
and no indent (currently, there is a 1-space indent).
Changed 11 years ago by
Attachment: | trac_11822_elliptic_curve_point_reduction.patch added |
---|
Fixed doc formatting, apply only this
comment:14 Changed 11 years ago by
Status: | needs_work → needs_review |
---|
comment:15 Changed 11 years ago by
Reviewers: | William Stein, Cassie Williams → William Stein, Cassie Williams, Jeroen Demeyer |
---|---|
Status: | needs_review → positive_review |
comment:16 Changed 11 years ago by
Merged in: | → sage-5.0.beta1 |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
fixes and adds documentation