optional doctests: clean up and document "known bug", "optional: requires PKG"

[jhpalmieri]

Marking a doctest # known bug means that it is skipped unless the --optional flag is passed to sage -t. This needs to be documented.

Also, many doctests in Sage are marked # optional - requires PKG, and this means that sage -t -only-optional=PKG ... does not run the test: the word "requires" is interpreted as a package name. This is not ideal.

The attached patch to the scripts repo removes the word "requires" (and also "needs") automatically. It also allows a colon or comma instead of a hyphen, as in # optional: requires PKG. It also converts tests marked as "known bug" to tests marked as "optional bug", so they are run when you do sage -t -only-optional=bug .... The attached patch to the main Sage library documents all of this.

---

Apply

• attachment: trac_11615-scripts-repo.patch to the scripts repo
• attachment: trac_11615-document-known-bug.patch to the main Sage library repo

CC: @nexttime

Component: doctest coverage

Author: John Palmieri

Reviewer: Karl-Dieter Crisman

Merged: sage-5.0.beta10

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

[jhpalmieri]

comment:2

By the way, the patch to the scripts library also fixes a bug: if you have a doctest marked

    sage: 2+2 # optional: pkg1 pkg2
    17

and run sage -t -only-optional=pkg1,pkg2 ..., the test would not be run: the function only_optional_include in sage-doctest would convert "pkg1 pkg2" to "pkg1pkg2" with no spaces, so it would only be run with sage -t -optional or sage -t -only-optional=pkg1pkg2. With the new version, the string gets converted to 'pkg1,pkg2', and then split into ['pkg1', 'pkg2'].

Previously, you could use the undocumented

   sage: 2+2 # optional: pkg1,pkg2
   17

and it would work. With the patch, this no longer works unless there is white space between pkg1 and pkg2 (commas are still allowed). I couldn't find any instances of tags separated by commas but no spaces in the Sage library. The patch to the Sage library clarifies the documentation, saying that you need spaces between the package names.

[nexttime]

comment:3

This should perhaps be added to the meta-ticket #11337.

[kcrisman]

comment:4

I like this, and it still applies!

Here is something to ask. Could have two if statements (instead of some sort of elif or something) cause trouble if a doctest was marked as optional and a bug? I've tried to make something break, but I'm not sure if this is the "right" behavior or not. Here is my example.

Adding doctest

            sage: 2+2 # optional -- known bug
            5

to a file gives

$ ../../sage -t -only-optional=known,bug ../../devel/sage/sage/symbolic/expression_conversions.py 
sage -t -only-optional=known,bug "devel/sage/sage/symbolic/expression_conversions.py"
	 [0.1 s]
 
----------------------------------------------------------------------
All tests passed!
Total time for all tests: 0.1 seconds

and the same happens with -only-optional=bug, but it fails (correctly) with -optional.

Otherwise the patch seems fine, and doc looks good (naturally!).

[kcrisman]

Reviewer: Karl-Dieter Crisman

[jhpalmieri]

comment:5

Good point. I've changed the code. Now if I doctest this function:

def testing():
    """
    EXAMPLES::

        sage: 2+2 # optional -- known bug
        5
        sage: 2+2 # known bug
        6
        sage: 2+2 # optional
        7
    """
    pass

I get three failures with sage -t ... --optional and two failures with sage -t ... --only-optional=bug.

[jhpalmieri]

scripts repo

[kcrisman]

comment:6

Attachment: trac_11615-scripts-repo.patch.gz

Okay, so it looks like # known bug is run by sage -t --optional. I think this was clear before, but I didn't realize it for some reason. I feel like this should be made more clear in the documentation in the other patch. ``sage -t --only-optional=bug ...`` isn't the only way this can be revealed, that is. Just

-so it is also detected by ``--only-optional=bug``.)
+so it is detected by ``--only-optional=bug`` and also ``--optional``.)

Here is a very very corner case.

        sage: 2+2 # optional -- magma bug
        7

This only runs with --only-optional=magma,bug. I guess that's ok.

What do you think about these two things? Maybe neither one is an issue.

Otherwise I do think this is in pretty good shape.

[jhpalmieri]

comment:7

Replying to @kcrisman:

Okay, so it looks like # known bug is run by sage -t --optional. I think this was clear before, but I didn't realize it for some reason. I feel like this should be made more clear in the documentation in the other patch.

I can do that.

Here is a very very corner case.

        sage: 2+2 # optional -- magma bug
        7

This only runs with --only-optional=magma,bug.

This is the old behavior: marking a doctest # optional X Y Z means it should only be run with --only-optional=X,Y,Z. So I don't think we need to add anything about it.

[kcrisman]

comment:8

Ok, great. I hope I tested this enough.

[jhpalmieri]

comment:9

Here is a new documentation patch, which just makes the following change:

diff --git a/doc/en/developer/conventions.rst b/doc/en/developer/conventions.rst
--- a/doc/en/developer/conventions.rst
+++ b/doc/en/developer/conventions.rst
@@ -880,7 +880,7 @@ mind:
   Then the doctest will be skipped by default, but could be revealed
   by running ``sage -t --only-optional=bug ...``.  (A doctest marked
   as ``known bug`` gets automatically converted to ``optional bug``,
-  so it is also detected by ``--only-optional=bug``.)
+  so it is also detected by ``--optional`` or  ``--only-optional=bug``.)
 
 -  If the entire documentation string contains all three words
    ``optional``, ``package``, and ``installed``, then the entire

[jhpalmieri]

Sage library

[kcrisman]

comment:10

Attachment: trac_11615-document-known-bug.patch.gz

Thanks, this should be a good addition.

---

Hmm, Trac upgrade smells nice so far. And still seems left-aligned, nice.

[jdemeyer]

Merged: sage-5.0.beta10