tol and optional in doctests don't play well together

[kcrisman]

See this sage-release thread and posts by Marco Streng. This bug is caused by #10923.

sage -t -only-optional=magma 
"devel/sage/sage/symbolic/integration/integral.py" 
was fine on beta2, but fails on beta3. This file has no magma doctests, 
but does have mathematica and maple doctests. The issue seems to be 
related to the doctesting framework, but I don't see any relevant 
tickets mentioned below. Could someone check whether this test works for 
them? 
Here's the traceback: 
sage -t -only-optional=magma 
"devel/sage/sage/symbolic/integration/integral.py" 
Traceback (most recent call last): 
   File "/storage/marco/sage-5.0.beta3/local/bin/sage-doctest", line 
924, in <module> 
     test_file(argv[1], library_code = library_code) 
   File "/storage/marco/sage-5.0.beta3/local/bin/sage-doctest", line 
721, in test_file 
     s = extract_doc(file, library_code=library_code) 
   File "/storage/marco/sage-5.0.beta3/local/bin/sage-doctest", line 
551, in extract_doc 
     doc = doc_preparse(F[i:j+3]) 
   File "/storage/marco/sage-5.0.beta3/local/bin/sage-doctest", line 
370, in doc_preparse 
     v = [i for i in range(len(t)) if 
only_optional_include(comment_modifiers[i])] 
IndexError: list index out of range 
      [0.1 s] 

This is verified on several systems.

---

Apply attachment: trac_12493.patch to the scripts repo.

CC: @mstreng @sagetrac-tmonteil @orlitzky

Component: doctest coverage

Author: John Palmieri

Reviewer: Marco Streng

Merged: sage-5.0.beta14

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

[kcrisman]

comment:1

If we add print statement

            if contains_optional_doctests:
                # make a list of each optional line cut out by our tag choices
                print comment_modifiers
                print len(t), len(comment_modifiers)
                v = [i for i in range(len(t)) if only_optional_include(comment_modifiers[i])]

we get the very interesting

<snip lots of stuff>
341 339

[kcrisman]

comment:2

Removing the final empty line changes things, though it doesn't help.

         sage: error.numerical_approx() # abs tol 10e-10
         0
-
     """
340 338

I can confirm that manually getting rid of this causes the tests to pass. A lot of trial and error as to which lines cause the problem has had no luck so far...

[kcrisman]

comment:3

Okay, it's these lines.

        sage: error.numerical_approx() # abs tol 10e-10
        0

Somehow we must be using this abs tol thing incorrectly.

[kcrisman]

comment:5

In case I can't track this down and someone from #10952 can help, here is the end of the comment modifiers.

' abs tol 10e-10', ' abs tol 10e-10', ' abs tol 10e-10', '']

[orlitzky]

comment:6

I think it's something else in integral.py (or, more likely, a combination of things). If I move the abs tol test to another file, the tests all pass.

[kcrisman]

comment:7

Replacing this comment with # optional -- internet requires causes no problems. Same with # not tested. But any other only optional (e.g., internet, maple) fails with the same problem.

Apparently the doctest framework has trouble with tol and only-optional. I'm making a note of this ticket at #10952.

[orlitzky]

comment:8

Yup. Not guilty! Here's a minimal failing test case:

def dummy():
    """

    Test whether or not optional doctests and `abs tol` get along.

        sage: 1+1 # abs tol 1e-10
        2
        sage: 2+2 # optional - requires base 5 or higher
        4

    """
    pass

[kcrisman]

comment:9

LOL on the base 5 optional test! Though I wouldn't say it's minimal, strictly speaking - you actually explained yourself.

Hopefully someone at #10952 will fix this; I'm not interested in breaking the doctest script by fixing this in some stupid fashion.

[jhpalmieri]

comment:11

I think I've found the problem and fixed it, but please test it: try it on the Sage library, try it on files with doctests which are intentionally supposed to fail (modify dummy above, for instance), both old-fashioned ones and ones with bad tolerance tests. It works for me, but it needs much more testing.

[jhpalmieri]

Author: John Palmieri

[mstreng]

Reviewer: Marco Streng

[mstreng]

comment:12

I think the count is still wrong. I added a print statement to sage-doctest that lists the lines and the corresponding modifiers:

    print "Printing lines and their modifiers"
    for i in range(len(t)):
        print "\n ========\n line:"
        print t[i]
        print "---\n comment:"
        print comment_modifiers[i]
    print "in total, there were %s lines with %s modifiers" % (len(t), len(comment_modifiers))
    print "remaining lines:"
    print t[len(comment_modifiers):]
    print "remaining modifiers:"
    print comment_modifiers[len(t):]

Lines with tol give a mismatch, even with the patch.

As a consequence, the following file passes all tests with sage -t -only-optional=magma, while it should fail!

def dummy():
    """

    Test whether or not optional doctests and `abs tol` get along.


        sage: 1+1 # rel tol 1e-10
        2
        sage: 2+2 # optional - requires magma and is wrong
        1

    """
    pass

(13 lines, 15 modifiers, and the magma modifiers ended up on empty lines at the end, hence the pass with -only-optional=magma)

The file fails the test with sage -t -optional, as that runs all lines, regardless of modifiers. The file passes the test with sage -t.

[jhpalmieri]

comment:13

If you have a doctest marked "# optional - requires magma and is wrong", then it is supposed to only get tested if you run something like

sage -t --only-optional=requires,magma,and,is,wrong ...

Actually, there is a bug so it only gets tested if you run

sage -t --only-optional=requiresmagmaandiswrong ...

In any case, it's not surprising that your file passed doctests: the optional doctest wasn't run, because you didn't give the right command-line flags.

When I try your file with

sage -t --only-optional=requiresmagmaandiswrong ...

I get a doctest failure.

See #11615 for a fix to make --only-optional=requires,magma,and,is,wrong work; the patch there also allows you to omit "requires" from this list.

[mstreng]

comment:14

Thanks for explaining why this particular example was wrong. Could you explain what is wrong with the rest of my comment too?

[jhpalmieri]

comment:15

It looks like you're right, the new comment was being added too many times. Here's a new patch; try this instead.

[jhpalmieri]

scripts repo

[mstreng]

comment:16

Attachment: trac_12493.patch.gz

All tests pass with the patch. But since this changes the doctesting framework, I needed to construct a doctest that is supposed to fail, and check whether it gives the correct failures. This made me stumble upon many problems with tol in doctesting.

Take this example:

"""
EXAMPLES:

The name blah is undefined::

    sage: blah+blah
    blahblah

Here's a doctest that is out of tolerance::

    sage: 100+10000000000 # abs tol 0.1
    10000000000
    sage: 1+1
    2
"""

The output when doctesting is

File "/home/marco/sage-5.0.beta11/small.sage", line 6:
    sage: blah+blah
Out of tolerance 10000000000.0 vs 10000000100.0

So the out of tolerance is mentioned for the wrong test. And the error of the test "blah+blah" is not even listed.

It gets worse if you remove the part that says

    sage: 1+1
    2

Then the python file produced for doctesting has a syntax error due to a blank line after the tolerance output.

  File "/home/marco/.sage//tmp/small_8271.py", line 63
    ... ''', res, 0.1, 'ab

Any test with tolerance followed by a blank line will produce a syntax error and end the doctesting of the whole file.

I guess I will have to work harder to find a set of doctests that avoids these issues, but still adequately tests whether attachment: trac_12493.patch is a good idea.

[kcrisman]

comment:17

Yikes! Thanks for working hard to get something that actually works for this. Or you can just fix the tol keyword :) I'll do my best to help review any changes if you do come up with a doctest patch.

[mstreng]

comment:19

Replying to @mstreng:

So the out of tolerance is mentioned for the wrong test. And the error of the test "blah+blah" is not even listed.

This is a new ticket: #12815

[jdemeyer]

Merged: sage-5.0.beta14