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

tutorial: put double colon on its own line #6572

Closed
sagetrac-mvngu mannequin opened this issue Jul 20, 2009 · 17 comments
Closed

tutorial: put double colon on its own line #6572

sagetrac-mvngu mannequin opened this issue Jul 20, 2009 · 17 comments

Comments

@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Jul 20, 2009

See this sage-devel thread for some background information. John Palmieri suggests that all doctests in the Sage tutorial be preceded with a double colon on its own line. That way, the doctest script would pick up all doctests in the tutorial.

Depends on #6642 and #7149.

CC: @jhpalmieri @nathanncohen

Component: documentation

Keywords: tutorial

Author: John Palmieri

Reviewer: Mike Hansen

Merged: sage-4.2.alpha0

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

@sagetrac-mvngu sagetrac-mvngu mannequin added this to the sage-4.2 milestone Jul 20, 2009
@jhpalmieri
Copy link
Member

comment:2

Here are two patches, one for the main repository and one for the scripts repository.

The scripts patch does the following: first, previously, doctests had to be preceded by a double colon at the beginning of a line, and now they only need to be preceded by a line which starts with white space then a double colon -- the double colon no longer has to be at the left margin. This is important for some files (like constructions/algebraic_geometry.rst), in which indentations might break if we had to move the double colons to the left. The second change is figuring out when the doctest block ends: it looks for text indented no farther than the double colons were. (Previously, it looked for text which wasn't indented at all, so if a paragraph was indented, then some doctests were indented further, then a paragraph unindented one level, that second paragaph was treated as part of the doctest block.)

As a result of all of this, some more doctests are found by sage -t. The other patch does two things: it makes the change advertised in the summary for the ticket, moving double colons onto lines of their own where necessary. It also tries to fix the newly discovered broken doctests. I don't know how to fix some of these, and so they are being skipped. These include the one which triggered this whole episode:

     sage: theta = var('theta')
     sage: solve(cos(theta)==sin(theta))
     [sin(theta) == cos(theta)]

as well as some doctests involving starting and stopping the Singular console, for example.

For me, this passes all tests on Mac OS X (both 32-bit and 64-bit), and also on a 32-bit linux box (an old Ubuntu machine) and a 64-bit linux box (sage.math). It would be best to fix the skipped tests, but that can go in another ticket if no one knows how to do it right now.

@jhpalmieri
Copy link
Member

comment:3

Attachment: trac_6572_scripts.patch.gz

See #6642 for the problem with the 'solve' doctest.

@kcrisman
Copy link
Member

comment:4

Replying to @jhpalmieri:

See #6642 for the problem with the 'solve' doctest.

This may be fixed now - see #6642. If so, I guess one could put that back in the tutorial.

@jhpalmieri
Copy link
Member

comment:5

Here is a patch against 4.1.2.alpha1; this depends on #6642.

@mwhansen
Copy link
Contributor

mwhansen commented Oct 5, 2009

comment:6

Doesn't it make more sense to detect double colons at the end of a line?

In restructured text, the two should be equivalent. It should be the same for the doctests as well.

@jhpalmieri
Copy link
Member

Attachment: trac_6572-scripts-part2.patch.gz

apply on top of other scripts patch

@jhpalmieri
Copy link
Member

comment:7

Replying to @mwhansen:

Doesn't it make more sense to detect double colons at the end of a line?

In restructured text, the two should be equivalent. It should be the same for the doctests as well.

Okay, that makes sense. The new scripts patch changes the regular expression to fix this, I think.

Now the problem is that the file (added since this ticket was opened) tour_graphtheory.rst is a complete disaster, with 15 doctest failures. I can fix lots of the failed doctests by adding "..." and ".. link" in various places, but I don't know what to do about failures like these:

    sage: g.plot(edge_colors=g.edge_coloring(hex_colors=True))
    AttributeError: 'Graph' object has no attribute 'edge_coloring'

    sage: g.vertex_coloring()
    AttributeError: 'Graph' object has no attribute 'vertex_coloring'

    sage: print g.max_matching()
    AttributeError: 'Graph' object has no attribute 'max_matching'

I don't know what was intended and therefore I don't know how to fix it. I'll post a patch that fixes what I can.

@jhpalmieri
Copy link
Member

comment:8

I'll post a replacement for "trac_6572-double-colon.patch" soon. That patch shouldn't have to be as large, if double colons at the end of lines work right.

@jhpalmieri
Copy link
Member

Attachment: trac_6572-double-colon.patch.gz

depends on #6642

@jhpalmieri
Copy link
Member

comment:9

Here's a new version of "trac_6572-double-colon.patch". This fixes a few more doctest failures uncovered by the script patches. Apply both script patches, this patch, and the graph theory patch. Together with the patch at #6642, this means that most doctests pass in the documentation. tutorial/tour_graphtheory.rst is the only problem now.

@jhpalmieri

This comment has been minimized.

@jhpalmieri
Copy link
Member

comment:10

With the patch at #7149, this is now ready for review. Ignore the graph theory patch, since that is now taken care of by #7149.

@jhpalmieri
Copy link
Member

ignore this patch!

@mwhansen
Copy link
Contributor

comment:11

Attachment: trac_6572-graphtheory.patch.gz

Looks good to me.

@mwhansen
Copy link
Contributor

Author: John Palmieri

@mwhansen
Copy link
Contributor

Reviewer: Mike Hansen

@mwhansen
Copy link
Contributor

Merged: sage-4.2.alpha0

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

3 participants