Opened 11 years ago

Closed 9 years ago

#7926 closed defect (fixed)

Bring coverage of monsky_washnitzer up to 50%

Reported by: robertwb Owned by: was
Priority: major Milestone: sage-5.0
Component: number theory Keywords: ecc2011, rd2
Cc: jen, kedlaya Merged in: sage-5.0.beta12
Authors: Robert Bradshaw, Jennifer Balakrishnan, David Loeffler Reviewers: Paul Zimmermann, Jeroen Demeyer
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

There's still lots to do here, but I started plowing through the file.

Attachments (2)

trac_7926_new.patch (55.8 KB) - added by jen 9 years ago.
trac_7926-fix.patch (5.0 KB) - added by davidloeffler 9 years ago.
Apply over previous patch

Download all attachments as: .zip

Change History (19)

comment:1 Changed 11 years ago by kedlaya

  • Cc kedlaya added

I haven't looked closely enough yet to be sure, but there's a chance this might need to be rebased after #7927 and #8304 get merged in.

comment:2 Changed 10 years ago by zimmerma

  • Keywords ecc2011 added

I confirm, this patch fails to apply to Sage 4.7.1 and thus should be rebased:

sage: hg_sage.import_patch("/tmp/7926-mw-docs.patch")
cd "/usr/local/sage-4.7.1/sage/devel/sage" && hg status
cd "/usr/local/sage-4.7.1/sage/devel/sage" && hg status
cd "/usr/local/sage-4.7.1/sage/devel/sage" && hg import   "/tmp/7926-mw-docs.patch"
applying /tmp/7926-mw-docs.patch
patching file sage/schemes/elliptic_curves/monsky_washnitzer.py
Hunk #3 FAILED at 2228
Hunk #6 FAILED at 2391
2 out of 9 hunks FAILED -- saving rejects to file sage/schemes/elliptic_curves/monsky_washnitzer.py.rej
patching file sage/schemes/hyperelliptic_curves/hyperelliptic_padic_field.py
Hunk #1 FAILED at 174
1 out of 1 hunks FAILED -- saving rejects to file sage/schemes/hyperelliptic_curves/hyperelliptic_padic_field.py.rej
abort: patch failed to apply

Paul

Changed 9 years ago by jen

comment:3 Changed 9 years ago by jen

  • Keywords rd2 added
  • Status changed from new to needs_review

This is a rebase against 5.0.beta9.

comment:4 Changed 9 years ago by davidloeffler

Apply trac_7926_new.patch

(for the patchbot)

comment:5 Changed 9 years ago by zimmerma

  • Status changed from needs_review to positive_review

positive review. The coverage increased to 53%, which is above the 50% goal of this ticket.

Paul

comment:6 Changed 9 years ago by jdemeyer

  • Authors set to Jennifer Balakrishnan
  • Reviewers set to Paul Zimmermann

comment:7 Changed 9 years ago by robertwb

  • Authors changed from Jennifer Balakrishnan to Robert Bradshaw, Jennifer Balakrishnan

Sorry I never got to 100%, but getting this in now is better than letting it bitrot again.

comment:8 Changed 9 years ago by jdemeyer

  • Status changed from positive_review to needs_work

The documentation doesn't even build properly:

dochtml.log:/padic/scratch/jdemeyer/merger/sage-5.0.beta12/local/lib/python2.7/site-packages/sage/schemes/elliptic_curves/monsky_washnitzer.py:docstring of sage.schemes.elliptic_curves.monsky_washnitzer:7: WARNING: Block quote ends without a blank line; unexpected unindent.
dochtml.log:/padic/scratch/jdemeyer/merger/sage-5.0.beta12/local/lib/python2.7/site-packages/sage/schemes/elliptic_curves/monsky_washnitzer.py:docstring of sage.schemes.elliptic_curves.monsky_washnitzer:15: WARNING: Block quote ends without a blank line; unexpected unindent.
dochtml.log:WARNING: inline latex u'\\phi(x) = x^p\n\n\\phi(y) = y^p \\sqrt{1 ': latex exited with error:
dochtml.log:WARNING: inline latex u'\\phi^*(dx/2y) = px^{p-1} y(\\phi(y))^{-1} dx/2y\n              = px^{p-1} y^{1-p} \\sqrt{1 ': latex exited with error:

comment:9 Changed 9 years ago by zimmerma

sorry Jeroen I did a bad reviewer job. But how can one check the documentation builds properly?

Paul

comment:10 Changed 9 years ago by jdemeyer

The easiest way is

make doc

from $SAGE_ROOT, but that will build more than you need.

You could also do (from $SAGE_ROOT):

./sage --docbuild reference html

Note that the documentation will actually build, there aren only WARNINGs. So you have to look for warnings in the on-screen output.

comment:11 Changed 9 years ago by davidloeffler

You can also look in the output of the patchbot (click on the swirly round blob by the ticket title and go to "plugins.docbuild"). The patchbot builds the reference manual with jsmath, which means it misses the third error (because it doesn't attempt to process latex formulae at build time), but it spots the other two.

comment:12 Changed 9 years ago by zimmerma

thank you Jeroen and David, but how can I identify the corrupted lines? The numbers 7 and 15 do not seem to correspond to bad block quotes.

Paul

comment:13 Changed 9 years ago by davidloeffler

The problem is that the docstring of matrix_of_frobenius is a plain string, not a raw string (r""" ... """) and hence it interprets the \f in \frac as a form feed character! This completely mangles Sphinx's parsing of the Latex formulae, unsurprisingly.

Changed 9 years ago by davidloeffler

Apply over previous patch

comment:14 Changed 9 years ago by davidloeffler

  • Status changed from needs_work to needs_review

Here's a patch which makes the reference manual build without errors, and corrects a few other minor formatting problems which I spotted while I was fixing this.

comment:15 Changed 9 years ago by jdemeyer

Not tested yet, but looks good on first sight.

comment:16 Changed 9 years ago by zimmerma

  • Authors changed from Robert Bradshaw, Jennifer Balakrishnan to Robert Bradshaw, Jennifer Balakrishnan, David Loeffler
  • Reviewers changed from Paul Zimmermann to Paul Zimmermann, Jeroen Demeyer
  • Status changed from needs_review to positive_review

I've done make doc and there is no warning any more (more precisely the only warnings I get are because the dvipng command is not installed on my computer).

Paul

comment:17 Changed 9 years ago by jdemeyer

  • Merged in set to sage-5.0.beta12
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.