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

Fix hiding of TESTS in documentation #27896

Closed
mwageringel opened this issue May 30, 2019 · 11 comments
Closed

Fix hiding of TESTS in documentation #27896

mwageringel opened this issue May 30, 2019 · 11 comments

Comments

@mwageringel
Copy link

The function sage.misc.sagedoc.skip_TESTS_block is responsible for hiding TESTS blocks from the documentation, but the case below is not correctly handled. As a result, all the tests below the line :: are not hidden from the documentation.

    TESTS:

    Some description::

        sage: 2 * 3
        6

    ::

        sage: 2 * 4
        8

This seems to be a common pattern for writing tests. For an actual example, check the end of the output of ideal? for instance, also available here.

Component: documentation

Author: Markus Wageringel

Branch/Commit: 4d0badd

Reviewer: John Palmieri

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

@mwageringel
Copy link
Author

Commit: 4d0badd

@mwageringel
Copy link
Author

comment:1

Apparently, :: gets interpreted as ReST header which can take the form

Header
::::::

This commit adds special handling of :: to avoid this.


New commits:

4d0baddhandle :: when hiding TESTS from documentation

@mwageringel
Copy link
Author

Branch: u/gh-mwageringel/docs/tests_block

@mwageringel
Copy link
Author

Author: Markus Wageringel

@mwageringel
Copy link
Author

comment:2

There is one pyflakes warning about an unused import of sage.all. However, that import is needed for the line:

x = eval('sage.all.%s'%obj, locals())

Removing the import makes the doctest of the corresponding function fail.

There is also a patchbot warning about insertion of triple colons. I am not sure what the intent of that warning is, but since I am using them in a regular expression, I guess that the warning is irrelevant. Otherwise, :::+ can be replaced by :{3,}.

@jhpalmieri
Copy link
Member

comment:3

Is a single colon valid reST? Can we change the end of header from |:|:::+) to |:::+)

I suppose ideally, we would match a string of colons with matching text on the previous line, to make sure it really was a header, but we don't want to write a reST parser to solve this problem.

@mwageringel
Copy link
Author

comment:4

A reST header can have length 1 and can even look like this:

:
e
:

The Euler number.

This is indeed interpreted as header by Sphinx. If we disallow single colons nevertheless, this should also apply to the other characters.

I agree that ideally the line containing the header text should be taken into account, but it seems like a solution covering all the cases would add quite a bit of complexity for something that has not been a problem so far.

@jhpalmieri
Copy link
Member

Reviewer: John Palmieri

@jhpalmieri
Copy link
Member

comment:5

Fine.

@vbraun
Copy link
Member

vbraun commented Jun 27, 2019

Changed branch from u/gh-mwageringel/docs/tests_block to 4d0badd

@embray
Copy link
Contributor

embray commented Jul 3, 2019

comment:7

Not in Sage 8.8. Let's please to try keep tickets' milestones related to the release in which we actually intend to include them, and in particular the release in which they were actually included, especially when closing tickets.

@embray embray modified the milestones: sage-8.8, sage-8.9 Jul 3, 2019
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

4 participants