Opened 12 years ago
Closed 8 years ago
#8305 closed defect (fixed)
Improve documentation of Monsky-Washnitzer code
Reported by: | kedlaya | Owned by: | kedlaya |
---|---|---|---|
Priority: | major | Milestone: | sage-6.1 |
Component: | elliptic curves | Keywords: | Monsky-Washnitzer, elliptic curves, hyperelliptic curves |
Cc: | jpflori | Merged in: | |
Authors: | Frédéric Chapoton | Reviewers: | Kiran Kedlaya |
Report Upstream: | N/A | Work issues: | |
Branch: | u/chapoton/8305 | Commit: | 18c386a894b4e508ef551e5833e858ea2a5d16dd |
Dependencies: | Stopgaps: |
Description
The code in schemes/elliptic_curves/monsky_washnitzer.py largely dates from a time (early 2007) before Sage documentation and doctesting standards had been codified. As a result, its coverage is terrible (26 of 107).
It may also be worth a mild refactor: since it now applies more generally to hyperelliptic curves, it probably should be under schemes/hyperelliptic_curves.
Attachments (2)
Change History (15)
comment:1 Changed 12 years ago by
comment:2 Changed 12 years ago by
- Owner changed from cremona to kedlaya
OK, so this ticket should stay on ice until someone (e.g., me) has a chance to review #7926. Besides the documentation, there is also the issue of moving the MW code from elliptic to hyperelliptic where it belongs.
Changed 9 years ago by
comment:3 Changed 9 years ago by
first patch, with coverage reaching 67%
comment:4 Changed 8 years ago by
- Milestone changed from sage-5.11 to sage-5.12
comment:5 Changed 8 years ago by
after some more work, coverage is almost 77%
comment:6 Changed 8 years ago by
- Cc jpflori added
comment:7 Changed 8 years ago by
after some more work, coverage is now 83%
Changed 8 years ago by
comment:8 Changed 8 years ago by
after some more work, coverage is now 85%
comment:9 Changed 8 years ago by
- Branch set to u/chapoton/8305
- Commit set to 18c386a894b4e508ef551e5833e858ea2a5d16dd
- Status changed from new to needs_review
comment:10 Changed 8 years ago by
- Status changed from needs_review to positive_review
comment:11 Changed 8 years ago by
comment:13 Changed 8 years ago by
- Resolution set to fixed
- Status changed from positive_review to closed
The patch at #7926 brings coverage up to 50% (though I didn't make it to documenting the really interesting stuff).