Opened 2 years ago

Closed 23 months ago

Last modified 23 months ago

#27896 closed defect (fixed)

Fix hiding of TESTS in documentation

Reported by: gh-mwageringel Owned by:
Priority: major Milestone: sage-8.9
Component: documentation Keywords:
Cc: Merged in:
Authors: Markus Wageringel Reviewers: John Palmieri
Report Upstream: N/A Work issues:
Branch: 4d0badd (Commits, GitHub, GitLab) Commit: 4d0badd20470a1a8b5143da51fc16fbc7e98393c
Dependencies: Stopgaps:

Status badges

Description

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.

Change History (7)

comment:1 Changed 2 years ago by gh-mwageringel

  • Authors set to Markus Wageringel
  • Branch set to u/gh-mwageringel/docs/tests_block
  • Commit set to 4d0badd20470a1a8b5143da51fc16fbc7e98393c
  • Status changed from new to needs_review

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

comment:2 Changed 2 years ago by gh-mwageringel

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,}.

comment:3 Changed 2 years ago by jhpalmieri

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.

comment:4 Changed 2 years ago by gh-mwageringel

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.

comment:5 Changed 2 years ago by jhpalmieri

  • Reviewers set to John Palmieri
  • Status changed from needs_review to positive_review

Fine.

comment:6 Changed 23 months ago by vbraun

  • Branch changed from u/gh-mwageringel/docs/tests_block to 4d0badd20470a1a8b5143da51fc16fbc7e98393c
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:7 Changed 23 months ago by embray

  • Milestone changed from sage-8.8 to sage-8.9

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.

Note: See TracTickets for help on using tickets.