Docbuilding without TESTS: blocks fails

[f29946bc-ee7b-48cd-9abc-3445948c551d]

./sage --docbuild --no-tests all html

gives

[libs     ] docstring of sage.libs.pari.pari_instance:162: WARNING: Definition list ends without a blank line; unexpected unindent.
Error building the documentation.

Component: documentation

Keywords: --no-tests --include-tests-blocks SAGE_SKIP_TESTS_BLOCKS

Author: John Palmieri

Branch/Commit: f4c3edb

Reviewer: Jori Mäntysalo

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

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

Changed keywords from none to --no-tests

[jdemeyer]

comment:2

I thought that docbuilding without TESTS was the default? But apparently not...

[f29946bc-ee7b-48cd-9abc-3445948c551d]

comment:3

Replying to @jdemeyer:

I thought that docbuilding without TESTS was the default? But apparently not...

I would like it to be default for most users. But then for developers it might be easier if it is not.

Anyways, it seems that fix for this problem is removing two empty lines. Someone with more time should check what is the real problem.

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:4

Replying to @jm58660:

Replying to @jdemeyer:

I thought that docbuilding without TESTS was the default? But apparently not...

I would like it to be default for most users. But then for developers it might be easier if it is not.

Yes. It would be good anyway for different purposes to have something like a "Sage developer mode" which is not enabled by default (or in stable releases, say).

Anyways, it seems that fix for this problem is removing two empty lines. Someone with more time should check what is the real problem.

Is it something that should go into the Sage Developer Guide (in order to avoid it in the future)?

[f29946bc-ee7b-48cd-9abc-3445948c551d]

comment:5

Replying to @nexttime:

Yes. It would be good anyway for different purposes to have something like a "Sage developer mode" which is not enabled by default (or in stable releases, say).

Hmm... yes and no. Would be good, but OTOH dangerous as something might work in developer mode only. Actually this one is just a case of that! I compiled docs for several betas of 7.3 in a desktop, but for final version I installed it with --no-tests to our server.

Anyways, it seems that fix for this problem is removing two empty lines. Someone with more time should check what is the real problem.

Is it something that should go into the Sage Developer Guide (in order to avoid it in the future)?

Some patchbot should compile with the flag and another without flag.

I doubt how many developer or reviewer would compile documentation twise just to be sure.

[jdemeyer]

comment:6

Replying to @jm58660:

I would like it to be default for most users. But then for developers it might be easier if it is not.

And how many developers actually compile their own documentation and view that? I guess not many. I do it rarely in case I'm working on a documentation ticket and want to see the output. And even if I do that, I don't care about the TESTS blocks. Those TESTS blocks are mostly useful when reading/writing the code.

I don't see the point of "optionally" removing TESTS blocks. That creates an interesting paradox: the people who know how to remove the TESTS blocks are precisely the people who would be most interesting in seeing the TESTS blocks.

Even the online documentation still has the TESTS blocks and that's the single place where they should not be shown.

(I know this is all orthogonal to this ticket, but I just don't understand why somebody would add an option to remove TESTS blocks but then not make it the default).

[jdemeyer]

comment:7

Replying to @jm58660:

Anyways, it seems that fix for this problem is removing two empty lines.

If you know the fix, can you put it on this ticket?

[jhpalmieri]

comment:8

I think this problem occurs because lines like the following (from Sage output in doctests around line 162 in sage.libs.pari.pari_instance)

    IFAC: cracking composite

match the regular expression which is supposed to signify the end of a TESTS block: a word all in caps followed by a colon (to match things like "ALGORITHM:" or "REFERENCES:" which we want to include in the documentation). As I think someone said, probably on the original ticket for skipping TESTS blocks, rather than regular expression matching, we should have a more sophisticated parser.

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:9

Replying to @jdemeyer:

Replying to @jm58660:

I would like it to be default for most users. But then for developers it might be easier if it is not.

And how many developers actually compile their own documentation and view that? I guess not many. I do it rarely in case I'm working on a documentation ticket and want to see the output. And even if I do that, I don't care about the TESTS blocks. Those TESTS blocks are mostly useful when reading/writing the code.

I don't see the point of "optionally" removing TESTS blocks. That creates an interesting paradox: the people who know how to remove the TESTS blocks are precisely the people who would be most interesting in seeing the TESTS blocks.

Even the online documentation still has the TESTS blocks and that's the single place where they should not be shown.

(I know this is all orthogonal to this ticket, but I just don't understand why somebody would add an option to remove TESTS blocks but then not make it the default).

I fully agree (and by the way think this belongs to this ticket).

Add --include-test-blocks for docbuilding and make --no-tests the default, probably completely removing that option.

IMHO TESTS: should never end up in PDFs for example; it might be convenient for some to review code / docstrings using HTML output, but those could explicitly ask for including them when building their docs. (Just wondering whether that's a "global" option that eventually triggers complete rebuild when changed.)

The problem I see (OTOH) is that EXAMPLES: and TESTS: are often rather randomly used; many doctests that no user would ever be interested in are in EXAMPLES:, while some TESTS: sections on the other hand contain examples that would be valuable to users as well. But that's just my uneducated impression I got over the last years, and I cannot give any recent examples, right now at least.

[f29946bc-ee7b-48cd-9abc-3445948c551d]

comment:10

Replying to @jdemeyer:

I don't see the point of "optionally" removing TESTS blocks. That creates an interesting paradox: the people who know how to remove the TESTS blocks are precisely the people who would be most interesting in seeing the TESTS blocks.

I think that it was discussed when add the switch. But I don't remember what was the reason to not make it default.

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:11

For the record:

The online (terminal) help for sage.libs.pari.pari_instance gives the following:

...
Elliptic curves and precision: If you are working with elliptic
curves, you should set the precision for each method:

   sage: e = pari([0,0,0,-82,0]).ellinit()
   sage: eta1 = e.elleta(precision=100)[0]
   sage: eta1.sage()
   3.6054636014326520859158205642077267748
   sage: eta1 = e.elleta(precision=180)[0]
   sage: eta1.sage()
   3.60546360143265208591582056420772677481026899659802474544

Number fields and precision: TODO

   IFAC: cracking composite
      49191317529892137643

   IFAC: factor 6713103182899
      is prime

   IFAC: factor 7327657
      is prime

   IFAC: prime 7327657
      appears with exponent = 1

   IFAC: prime 6713103182899
      appears with exponent = 1

   IFAC: found 2 large prime (power) factors. [3, 1; 7327657, 1;
   6713103182899, 1] sage: pari.default("debug", 0) sage:
   pari(2^67+1).factor() [3, 1; 7327657, 1; 6713103182899, 1]

Class docstring:
module(name[, doc])

Create a module object. The name must be a string; the optional doc
argument can have any type.
Init docstring:  x.__init__(...) initializes x; see help(type(x)) for signature
(END) 

Note that everything below TODO (only up to Class docstring: of course) belongs to the TESTS: block. :-)

[f29946bc-ee7b-48cd-9abc-3445948c551d]

comment:12

Replying to @nexttime:

The problem I see (OTOH) is that EXAMPLES: and TESTS: are often rather randomly used; many doctests that no user would ever be interested in are in EXAMPLES:, while some TESTS: sections on the other hand contain examples that would be valuable to users as well.

I agree. Most common problem is to not have a TESTS block at all. I have changed those in my Grand Poset Documentaion Polishing Project. :=)

I guess this is partly because --no-tests is new and not the default. It makes much less sense to differentiate between examples and tests when both are shown to the user.

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:13

Well, I do not know whether this was introduced at the same time, but the online help (foo?) meanwhile suppresses the TESTS: section by default, or at least tries to, as you can see above. ;-)

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:14

Couldn't resist:

diff --git a/src/doc/en/installation/source.rst b/src/doc/en/installation/source.rst
index 799b546..fa4a42e 100644
--- a/src/doc/en/installation/source.rst
+++ b/src/doc/en/installation/source.rst
@@ -999,7 +999,7 @@ Here are some of the more commonly used variables affecting the build process:
   you run ``make``, ``make doc``, or ``make doc-pdf``.
   For example, you can add ``--no-plot`` to this variable to avoid building
   the graphics coming from the ``.. PLOT`` directive within the documentation,
-  or you can add ``--no-tests`` to omit all "TESTS" blocks in the
+  or you can add ``--include-tests-blocks`` to include all "TESTS" blocks in the
   reference manual. Run ``sage --docbuild help`` to see the full list
   of options.

diff --git a/src/sage_setup/docbuild/__init__.py b/src/sage_setup/docbuild/__init__.py
index 1fc5ccb..e7363e7 100644
--- a/src/sage_setup/docbuild/__init__.py
+++ b/src/sage_setup/docbuild/__init__.py
@@ -1445,9 +1445,9 @@ def setup_parser():
     standard.add_option("--no-plot", dest="no_plot",
                         action="store_true",
                         help="do not include graphics auto-generated using the '.. plot' markup")
-    standard.add_option("--no-tests", dest="skip_tests", default=False,
-                        action="store_true",
-                        help="do not include TESTS blocks in the reference manual")
+    standard.add_option("--include-tests-blocks", dest="skip_tests", default=True,
+                        action="store_false",
+                        help="include TESTS blocks in the reference manual")
     standard.add_option("--no-pdf-links", dest="no_pdf_links",
                         action="store_true",
                         help="do not include PDF links in DOCUMENT 'website'; FORMATs: html, json, pickle, web")

[jhpalmieri]

comment:15

Replying to @nexttime:

For the record:

The online (terminal) help for sage.libs.pari.pari_instance gives the following:

[snip]

Yes, and this is why I made comment above (plus looking at the documentation and source code in misc/sagedoc.py).

The original ticket #19503 was precisely about skipping TESTS blocks in introspection; the focus was not on the reference manual, and that's one reason the default is the way it is. Plus there was some discussion of the concern Leif expressed above, that some TESTS blocks have useful information, so if we remove them from the reference manual, we lose helpful information.

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:16

So here's the culprit (src/sage/misc/sagedoc.py, line 298 ff.):

    # end_of_block: match a line starting with whitespace, then Sphinx
    # directives of the form ".. foo:". This will match directive
    # names "foo" containing letters of either case, hyphens,
    # underscores.
    # Also match uppercase text followed by a colon, like
    # "REFERENCES:" or "ALGORITHM:".
    end_of_block = re.compile('[ ]*(\.\.[ ]+[-_A-Za-z]+|[A-Z]+):')
    # header: match a string of hyphens, or other characters which are
    # valid markers for ReST headers: - = ` : ' " ~ _ ^ * + # < >

Currently, only another TEST[S]:[:] doesn't terminate the block to skip:

...
        if not skip:
            m = tests_block.match(l)
            if m:
                skip = True
                indentation = m.group(1)
            else:
                s += "\n"
                s += l
        else:
            if end_of_block.match(l) and not tests_block.match(l):
                skip = False
                s += "\n"
                s += l
...

So we'd either have to whitelist PARI's IFAC: to not end a TESTS block, or (more seriously) have to explicitly look for all possible / "valid" section names (like AUTHORS, REFERENCES, etc.).

Not sure whether there is at all a complete list of such. (AFAIK, it's just informal convention to use this or that.)

[jhpalmieri]

comment:17

Or check the indentation level: to terminate the block, a string like AUTHORS: should be indented no more than the original TESTS:, whereas IFAC (and anything in the output of a Sage test) is indented more.

[jhpalmieri]

comment:18

Maybe something like

diff --git a/src/sage/misc/sagedoc.py b/src/sage/misc/sagedoc.py
index 968a44c..77d6984 100644
--- a/src/sage/misc/sagedoc.py
+++ b/src/sage/misc/sagedoc.py
@@ -322,6 +322,8 @@ def skip_TESTS_block(docstring):
                 s += l
         else:
             if end_of_block.match(l) and not tests_block.match(l):
+                if l.find(indentation) == 0:
+                    continue
                 skip = False
                 s += "\n"
                 s += l

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:19

I've tried another way:

diff --git a/src/sage/misc/sagedoc.py b/src/sage/misc/sagedoc.py
index a6c5568..dc5eb13 100644
--- a/src/sage/misc/sagedoc.py
+++ b/src/sage/misc/sagedoc.py
@@ -302,6 +302,13 @@ def skip_TESTS_block(docstring):
     # Also match uppercase text followed by a colon, like
     # "REFERENCES:" or "ALGORITHM:".
     end_of_block = re.compile('[ ]*(\.\.[ ]+[-_A-Za-z]+|[A-Z]+):')
+    # Only recognize commonly used section names, not e.g. output
+    # from doctests incidentally matching '[A-Z]+:':
+    SECTION_NAMES = set([
+        "AUTHOR", "AUTHORS",
+        "ALGORITH", "ALGORITHMS",
+        "REFERENCE", "REFERENCES"
+    ])
     # header: match a string of hyphens, or other characters which are
     # valid markers for ReST headers: - = ` : ' " ~ _ ^ * + # < >
     header = re.compile(r'^[ ]*([-=`:\'"~_^*+#><])\1+[ ]*$')
@@ -320,7 +327,9 @@ def skip_TESTS_block(docstring):
                 s += "\n"
                 s += l
         else:
-            if end_of_block.match(l) and not tests_block.match(l):
+            m = end_of_block.match(l)
+            if m and not tests_block.match(l) \
+                 and m.group(1) in SECTION_NAMES:
                 skip = False
                 s += "\n"
                 s += l

Works for me modulo adding more section names.

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:20

... and modulo spelling of course. ;-)

[jhpalmieri]

comment:21

Speling is an issue, though ;) The likelihood of someone misspelling "ALGORITM", for example, seems pretty high, and it's only going to make a difference when they do it right after a TESTS block. Troubleshooting that will be difficult. Isn't indentation safer than whitelisting?

[jhpalmieri]

comment:22

With your proposal, we at least need to add "EXAMPLE" and "EXAMPLES", by the way.

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:23

Replying to @jhpalmieri:

Maybe something like

diff --git a/src/sage/misc/sagedoc.py b/src/sage/misc/sagedoc.py
index 968a44c..77d6984 100644
--- a/src/sage/misc/sagedoc.py
+++ b/src/sage/misc/sagedoc.py
@@ -322,6 +322,8 @@ def skip_TESTS_block(docstring):
                 s += l
         else:
             if end_of_block.match(l) and not tests_block.match(l):
+                if l.find(indentation) == 0:
+                    continue
                 skip = False
                 s += "\n"
                 s += l

I was about to write that l.find(indentation) is generally wrong (and should be l.startswith(indentation)), but then suddenly realized that .find() is not C's strstr() and even works for the empty string... m)

I'm not entirely sure yet your approach works though. Deeeeeep thinking required...

But anyway, I'll (ab)use my approach to find all kinds of section names (and misspellings) later...

(We could eventually also correct then.)

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:24

Replying to @jhpalmieri:

With your proposal, we at least need to add "EXAMPLE" and "EXAMPLES", by the way.

Yes and no. Shouldn't^TM EXAMPLES always precede TESTS?

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:25

FWIW, l.startswith(indentation) also works for the empty string and will be faster (not sure how much in practice though).

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:26

Replying to @nexttime:

I'm not entirely sure yet your approach works though. Deeeeeep thinking required...

It won't work for e.g.

TESTS:

    ...

FOO:
    bar

will it?

(FOO: matches end_block and has the same indentation as TESTS: and doesn't match tests_block either, so you'd continue skipping.)

[f29946bc-ee7b-48cd-9abc-3445948c551d]

comment:27

So Sphinx has no mechanism to restrict possible blocks of docstring? IMO the system should always fail with input like EXXAMPLES::.

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:28

So here's my preliminary word (section name) list:

ABS:
ACKNOWLEDGEMENT:
ACKNOWLEDGEMENTS:
ACKNOWLEDGMENT:
ADD:
ALGORITHM:
ALGORITHMS:
ALIAS:
ALIASES:
APPLICATIONS:
ARTEFACT:
ASSUMPTION:
ASSUMPTIONS:
AUTHOR:
AUTHORS:
B:
BENCHMARK:
BUGS:
CACHING:
CHECKME:
CODE:
COERCIONS:
COMMENTS:
COMPARISONS:
COMPLEXITY:
CONDITIONS:
CONSTRUCTION:
CRUCIAL:
DEBUG:
DEFINITION:
DETAILS:
DISCUSSION:
DIV:
DUP:
EFFECT:
EOS:
EQ:
ERROR:
EXAMPLE:
EXAMPLES:
EXMAPLES:
F:
FACTS:
FIXME:
FLOAT:
FORMALLY:
FORMATS:
FUNCTIONS:
G:
GAME:
GE:
GRIN:
GT:
H:
HISTORY:
IFAC:
IML:
IMPLEMENTATION:
IMPORTANT:
IN:
INFO:
INPUT:
INPUTS:
INT:
INVERT:
ISBN:
ISGCI:
J:
JOKE:
KEYWORDS:
KIND:
LE:
LICENSE:
LIMITATIONS:
LOCALS:
LOG:
LT:
M:
MAPLE:
MATHEMATICA:
MATRIX:
METHOD:
MH:
MODULE:
MODULUS:
MUL:
MUTABILITY:
NAME:
NB:
NE:
NEG:
NOTATION:
NOTE:
NOTES:
OPTIONAL:
OPTIONS:
OR:
OUPUT:
OUT:
OUTPUT:
OUTPUTS:
OUTUT:
P:
PARALLELE:
PARI:
PATH:
PERFORMANCE:
PLOTTING:
POP:
POW:
PRECONDITIONS:
PREMIER:
PRINTING:
PROBLEM:
PROFILE:
PROOF:
R:
RAISES:
RATIONALE:
REFERENCE:
REFERENCES:
REFERENCS:
REFRENCES:
REMARK:
REMARKS:
REQUIRED:
REQUIRES:
RESTRICTIONS:
RETURN:
RI:
RINGS:
SAGE:
SEE:
SEEALSO:
SERIE:
SUB:
T:
TEST:
TESTS:
THEOREM:
THEORY:
TIMINGS:
TODO:
TUTORIAL:
U:
UNKN:
USAGE:
USE:
V:
VALUES:
VERSION:
W:
WARNING:
X:
Y:

And no, there's not a single instance of misspelled ALLGORERHYTHM, but other interesting EXMAPLES [sic]. :-)

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:29

P.S.: Yet produced with grep and sed, so there are of course still false positives (including PARI's IFAC:).

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:30

John, is "acknowledgment" really (the more) common spelling in the U.S.?

[f29946bc-ee7b-48cd-9abc-3445948c551d]

comment:31

Still exmaples after #19601? Arghs.

[jhpalmieri]

comment:32

For my approach, I think I need to do

if l.startswith(indentation + ' '): ...

to make sure the match is indented more than "TESTS" was. I haven't tested it much (only on the example from pari_instance).

John, is "acknowledgment" really (the more) common spelling in the U.S.?

Yes.

So Sphinx has no mechanism to restrict possible blocks of docstring? IMO the system should always fail with input like EXXAMPLES::.

I think Sphinx just looks for indentations. It treats them specially if they are preceded by a Sphinx directive like .. NOTE:: or .. MATH::, but it doesn't care about EXXAMMPLES:: or TESTS::, etc., if it doesn't start with '.. '.

[jhpalmieri]

comment:33

And I think doctesting just looks for lines of the form

sage: INPUT
OUTPUT

and tests them: indentation seems to be ignored, "EXAMPLES" is unnecessary, etc.

[jhpalmieri]

comment:34

Replying to @jhpalmieri:

So Sphinx has no mechanism to restrict possible blocks of docstring? IMO the system should always fail with input like EXXAMPLES::.

I think Sphinx just looks for indentations. It treats them specially if they are preceded by a Sphinx directive like .. NOTE:: or .. MATH::, but it doesn't care about EXXAMMPLES:: or TESTS::, etc., if it doesn't start with '.. '.

We could write Sphinx directives .. EXAMPLES and .. TESTS. Since unindenting ends a Sphinx block, this would mean we couldn't do the following (AFAIK):

.. EXAMPLES::

    sage: blah
    blah

Oh, and also

    sage: blah
    blah

as a single EXAMPLES block.

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:35

Replying to @jm58660:

Still exmaples after #19601? Arghs.

src/sage/graphs/genus.pyx:692:    REFERENCS::
src/sage/modular/btquotients/btquotient.py:417:        OUPUT:
src/sage/modular/btquotients/btquotient.py:730:        OUPUT:
src/sage/modular/pollack_stevens/dist.pyx:1879:#     OUPUT:
src/sage/parallel/map_reduce.py:1903:        OUPUT: ``(z, )``
src/sage/combinat/composition_tableau.py:578:    OUTUT:
src/sage/combinat/sf/dual.py:266:        OUPUT
src/sage/combinat/enumeration_mod_permgroup.pyx:97:    OUPUT:
src/sage/combinat/matrices/dancing_links.pyx:342:        OUPUT:
src/sage/combinat/matrices/dancing_links.pyx:386:        OUPUT:
src/sage/combinat/words/finite_word.py:2006:        OUPUT
src/sage/combinat/words/word_datatypes.pyx:258:        OUPUT:
src/sage/combinat/words/word_datatypes.pyx:600:        OUPUT:
src/sage/combinat/words/word_datatypes.pyx:1107:        OUPUT:
src/sage/libs/linkages/padics/fmpz_poly_unram.pxi:70:    OUPUT:
src/sage/libs/linkages/padics/API.pxi:113:    OUPUT:
src/sage/libs/linkages/padics/mpz.pxi:70:    OUPUT:
src/sage/libs/ppl.pyx:3636:        OUPUT:
src/sage/libs/ppl.pyx:3904:        OUPUT:
src/sage/schemes/toric/sheaf/klyachko.py:359:        OUPUT:
src/sage/schemes/toric/sheaf/klyachko.py:454:        OUPUT:
src/sage/schemes/toric/sheaf/klyachko.py:497:        OUPUT:
src/sage/schemes/curves/affine_curve.py:441:        OUPUT: Boolean.
src/sage/schemes/curves/projective_curve.py:77:        EXMAPLES::
src/sage/schemes/curves/projective_curve.py:748:        OUPUT: Boolean.
src/sage/manifolds/scalarfield.py:1490:        OUPUT:
src/sage/manifolds/scalarfield.py:1770:        OUPUT:
src/sage/manifolds/scalarfield.py:1821:        OUPUT:
src/sage/manifolds/scalarfield.py:1873:        OUPUT:
src/sage/manifolds/scalarfield.py:1923:        OUPUT:
src/sage/manifolds/scalarfield.py:1981:        OUPUT:
src/sage/manifolds/scalarfield.py:2068:        OUPUT:

Don't know where "REFRENCES" originated from...

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:36

Replying to @nexttime:

Don't know where "REFRENCES" originated from...

src/sage/combinat/ribbon_tableau.py:197:    REFRENCES:
src/sage/combinat/sf/jack.py:991:        REFRENCES:

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:37

I've put the typos above on #21178.

[jhpalmieri]

Branch: u/jhpalmieri/TESTS

[jhpalmieri]

comment:39

Here is a branch which uses the indentation solution (plus hiding TESTS blocks by default in the reference manual). Feel free to post one with the version using whitelisting of section names.

---

New commits:

eb908ce

trac 21172: TESTS blocks should end not on any line starting "CAPITALS:", but

[jhpalmieri]

Commit: eb908ce

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:40

Looks like positive review (modulo related things that could be done on a follow-up as well).

But I'm still performing further tests... (i.e., I'm actually reviewing what gets removed, which is quite a bit of work)

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:41

Nope, needs work.

There's an else missing to handle the case that a TESTS block is just terminated by an "ordinary" line (not matching the patterns), which of course has to be less indented than the starting TESTS:[:] (otherwise it wouldn't terminate the block).

See for example src/sage/rings/asymptotic/asymptotic_expansion_generators.py, where incidentally (because of the ::) "only two" (non-empty) lines after the TESTS block get eaten up:

r"""
Common Asymptotic Expansions

.. WARNING::

    As this code is experimental, a warning is thrown when an
    asymptotic ring (or an associated structure) is created for the
    first time in a session (see
    :class:`sage.misc.superseded.experimental`).

    TESTS::

        sage: AsymptoticRing(growth_group='z^ZZ * log(z)^QQ', coefficient_ring=ZZ)
        doctest:...: FutureWarning: This class/method/function is marked as
        experimental. It, its functionality or its interface might change
        without a formal deprecation.
        See http://trac.sagemath.org/17601 for details.
        Asymptotic Ring <z^ZZ * log(z)^QQ> over Integer Ring

Asymptotic expansions in SageMath can be built through the
``asymptotic_expansions`` object. It contains generators for common
asymptotic expressions. For example,
::

    sage: asymptotic_expansions.Stirling('n', precision=5)
    sqrt(2)*sqrt(pi)*e^(n*log(n))*(e^n)^(-1)*n^(1/2) +
    1/12*sqrt(2)*sqrt(pi)*e^(n*log(n))*(e^n)^(-1)*n^(-1/2) +
    1/288*sqrt(2)*sqrt(pi)*e^(n*log(n))*(e^n)^(-1)*n^(-3/2) +
    O(e^(n*log(n))*(e^n)^(-1)*n^(-5/2))

generates the first 5 summands of Stirling's approximation formula for
factorials.

...
"""

(IMHO you shouldn't need previous anyway, except for adding a blank line, in which case you don't have to keep the actual contents, only maybe the indentation / number of spaces, but I'm not even sure that's necessary.)

---

EDIT: Removed [... has to be less] (or equally) [indented than ...] again I added afterwards, because that's wrong.

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:42

P.S.: John, just in case you don't have the time (or mood) to further work on this now, I could take over and present something in the next days.

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

Changed keywords from --no-tests to --no-tests --include-tests-blocks SAGE_SKIP_TESTS_BLOCKS

[jhpalmieri]

comment:44

A few comments: you need previous in the header case, since the previous line is the actual title of that header. Were you thinking of getting rid of it in general?

Good catch on the example in asymptotic_expansion_generators.py, but I think that's pretty rare. What may also occur, unfortunately, is something like

    TESTS::

        blah

    We should also test x::

        blah # should be considered part of the TESTS block

    Now the tests are over, so let's make some remarks.
    (This should not be part of the TESTS block.)

I don't know how to catch this. Maybe TESTS should be a Sphinx directive after all (but not on this ticket).

If you have time to work on your proposed changes, please go ahead.

[jhpalmieri]

comment:45

I am now testing the following change (on top of the current commit):

diff --git a/src/sage/misc/sagedoc.py b/src/sage/misc/sagedoc.py
index 9004bdb..0139a3e 100644
--- a/src/sage/misc/sagedoc.py
+++ b/src/sage/misc/sagedoc.py
@@ -322,7 +322,11 @@ def skip_TESTS_block(docstring):
                 s += "\n"
                 s += l
         else:
-            if end_of_block.match(l) and not tests_block.match(l):
+            if not l.startswith(indentation):
+                skip = False
+                s += "\n"
+                s += l
+            elif end_of_block.match(l) and not tests_block.match(l):
                 if l.startswith(indentation + " "):
                     continue
                 skip = False

[jhpalmieri]

comment:46

No, that doesn't work. Hold on.

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:47

Replying to @jhpalmieri:

No, that doesn't work. Hold on.

... which seemed obvious since when not l.startswith(indentation) doesn't hold l.startswith(indentation + " " won't ever hold either. ;-)

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:48

It may make sense to generally differentiate between TESTS blocks starting at top-level (indentation=="") and TESTS blocks that started with arbitrary (non-zero) indentation, i.e., to introduce a new explicit state.

(Perhaps also for headings. AFAIK these have to start at column 0, haven't they?)

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:49

Replying to @nexttime:

Replying to @jhpalmieri:

No, that doesn't work.

... which seemed obvious since when not l.startswith(indentation) doesn't hold l.startswith(indentation + " " won't ever hold either. ;-)

Seemed... Don't strike that not. ;-)