Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bring coverage of monsky_washnitzer up to 50% #7926

Closed
robertwb opened this issue Jan 14, 2010 · 24 comments
Closed

Bring coverage of monsky_washnitzer up to 50% #7926

robertwb opened this issue Jan 14, 2010 · 24 comments

Comments

@robertwb
Copy link
Contributor

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

CC: @jbalakrishnan @kedlaya

Component: number theory

Keywords: ecc2011, rd2

Author: Robert Bradshaw, Jennifer Balakrishnan, David Loeffler

Reviewer: Paul Zimmermann, Jeroen Demeyer

Merged: sage-5.0.beta12

Issue created by migration from https://trac.sagemath.org/ticket/7926

@kedlaya
Copy link
Sponsor Contributor

kedlaya commented Feb 20, 2010

comment:1

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.

@zimmermann6
Copy link

Changed keywords from none to ecc2011

@zimmermann6
Copy link

comment:2

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

@jbalakrishnan
Copy link

Changed keywords from ecc2011 to ecc2011, rd2

@jbalakrishnan
Copy link

comment:3

Attachment: trac_7926_new.patch.gz

This is a rebase against 5.0.beta9.

@loefflerd
Copy link
Mannequin

loefflerd mannequin commented Mar 24, 2012

comment:4

Apply trac_7926_new.patch

(for the patchbot)

@zimmermann6
Copy link

comment:5

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

Paul

@jdemeyer
Copy link

Author: Jennifer Balakrishnan

@jdemeyer
Copy link

Reviewer: Paul Zimmermann

@robertwb
Copy link
Contributor Author

Changed author from Jennifer Balakrishnan to Robert Bradshaw, Jennifer Balakrishnan

@robertwb
Copy link
Contributor Author

comment:7

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

@jdemeyer
Copy link

comment:8

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:

@zimmermann6
Copy link

comment:9

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

Paul

@jdemeyer
Copy link

comment:10

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.

@loefflerd
Copy link
Mannequin

loefflerd mannequin commented Mar 28, 2012

comment:11

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.

@zimmermann6
Copy link

comment:12

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

@loefflerd
Copy link
Mannequin

loefflerd mannequin commented Mar 28, 2012

comment:13

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.

@loefflerd
Copy link
Mannequin

loefflerd mannequin commented Mar 28, 2012

Apply over previous patch

@loefflerd
Copy link
Mannequin

loefflerd mannequin commented Mar 28, 2012

comment:14

Attachment: trac_7926-fix.patch.gz

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.

@loefflerd loefflerd mannequin removed the s: needs work label Mar 28, 2012
@loefflerd loefflerd mannequin added the s: needs review label Mar 28, 2012
@jdemeyer
Copy link

comment:15

Not tested yet, but looks good on first sight.

@zimmermann6
Copy link

comment:16

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

@zimmermann6
Copy link

Changed reviewer from Paul Zimmermann to Paul Zimmermann, Jeroen Demeyer

@zimmermann6
Copy link

Changed author from Robert Bradshaw, Jennifer Balakrishnan to Robert Bradshaw, Jennifer Balakrishnan, David Loeffler

@jdemeyer
Copy link

jdemeyer commented Apr 2, 2012

Merged: sage-5.0.beta12

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants