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:

Status badges

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)

trac_8305_monsky_doc.patch (86.1 KB) - added by chapoton 9 years ago.
trac_8305_monsky_doc_step2.patch (50.5 KB) - added by chapoton 8 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 Changed 12 years ago by robertwb

The patch at #7926 brings coverage up to 50% (though I didn't make it to documenting the really interesting stuff).

comment:2 Changed 12 years ago by kedlaya

  • 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 chapoton

comment:3 Changed 9 years ago by chapoton

first patch, with coverage reaching 67%

comment:4 Changed 8 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:5 Changed 8 years ago by chapoton

after some more work, coverage is almost 77%

comment:6 Changed 8 years ago by jpflori

  • Cc jpflori added

comment:7 Changed 8 years ago by chapoton

after some more work, coverage is now 83%

Changed 8 years ago by chapoton

comment:8 Changed 8 years ago by chapoton

after some more work, coverage is now 85%

comment:9 Changed 8 years ago by chapoton

  • Branch set to u/chapoton/8305
  • Commit set to 18c386a894b4e508ef551e5833e858ea2a5d16dd
  • Status changed from new to needs_review

maybe this can be considered as a good first step towards 100% ?


New commits:

18c386atrac #8305 clean monsky-w doc step 2
71cab69trac 8305 doc of monsky waschnitzer

comment:10 Changed 8 years ago by kedlaya

  • Status changed from needs_review to positive_review

An excellent step, yes! Good work.

I've spun off tickets #15645 and #15646 to track the remaining issues on this ticket (100% coverage and refactoring).

comment:11 Changed 8 years ago by chapoton

  • Authors set to Frédéric Chapoton

comment:12 Changed 8 years ago by vbraun

  • Reviewers set to Kiran Kedlaya

Fill in the reviewer name...

comment:13 Changed 8 years ago by vbraun

  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.