Opened 12 years ago

Closed 10 years ago

#8699 closed enhancement (duplicate)

allow doctest coverage script to handle triple single quotes

Reported by: Minh Van Nguyen Owned by: tbd
Priority: minor Milestone: sage-duplicate/invalid/wontfix
Component: doctest coverage Keywords: 7 years
Cc: John Palmieri, William Stein Merged in:
Authors: Reviewers: Minh Van Nguyen, Mike Hansen
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by Mike Hansen)

As the subject says. See this sage-devel thread for some background information.

This might conflict with ticket #7716. See #8708 for a related ticket.

Apply patches in this order:

  • trac_8699-documentation.patch --- Document the doctest coverage script. Don't redefine the built-in function name "file"; use "afile" instead. Consistently use double quotes wherever possible. General stylistic clean-ups a la PEP 008.
  • trac_8699.patch --- Allow the doctest coverage script to handle triple single quotes.

Attachments (3)

trac_8699-documentation.patch (13.1 KB) - added by Minh Van Nguyen 12 years ago.
trac_8699-single-quotes.patch (1.1 KB) - added by Minh Van Nguyen 12 years ago.
trac_8699.patch (1.1 KB) - added by Mike Hansen 10 years ago.

Download all attachments as: .zip

Change History (18)

comment:1 Changed 12 years ago by Minh Van Nguyen

Description: modified (diff)

Changed 12 years ago by Minh Van Nguyen

Changed 12 years ago by Minh Van Nguyen

comment:2 Changed 12 years ago by Minh Van Nguyen

Authors: Minh Van Nguyen
Description: modified (diff)
Status: newneeds_review

The two patches on this ticket provide documentation for the doctest coverage script, in addition to allowing that script to handle docstrings that are delimited by triple single quotes. Incidentally, using those patches I found that throughout the whole Sage library, only one method uses triple single quotes. The method in question is BubbleSortGraph() of the module sage.graphs.graph_generators.py. This means that for the 90% doctest coverage goal of Sage 5.0, we have one less method to document because it's already documented. See the following command line transcript:

[mvngu@sage sage-4.4.alpha0-8699-quotes]$ ./sage -coverageall > coverage-before.log
[mvngu@sage sage-4.4.alpha0-8699-quotes]$ cd local/bin/
[mvngu@sage bin]$ hg qimport http://trac.sagemath.org/sage_trac/raw-attachment/ticket/8699/trac_8699-documentation.patch && hg qpush 
adding trac_8699-documentation.patch to series file
applying trac_8699-documentation.patch
now at: trac_8699-documentation.patch
[mvngu@sage bin]$ hg qimport http://trac.sagemath.org/sage_trac/raw-attachment/ticket/8699/trac_8699-single-quotes.patch && hg qpush 
adding trac_8699-single-quotes.patch to series file
applying trac_8699-single-quotes.patch
now at: trac_8699-single-quotes.patch
[mvngu@sage bin]$ cd ../..
[mvngu@sage sage-4.4.alpha0-8699-quotes]$ ./sage -coverageall > coverage-after.log
[mvngu@sage sage-4.4.alpha0-8699-quotes]$ diff -Naur coverage-before.log coverage-after.log 
--- coverage-before.log 2010-04-17 03:37:50.663727239 -0700
+++ coverage-after.log  2010-04-17 03:38:45.181442502 -0700
@@ -381,7 +381,7 @@
 geometry/polytope.py: 27% (6 of 22)
 geometry/polyhedra.py: 100% (186 of 186)
 graphs/graph_bundle.py: 100% (5 of 5)
-graphs/graph_generators.py: 98% (73 of 74)
+graphs/graph_generators.py: 100% (74 of 74)
 graphs/planarity.pyx: 100% (1 of 1)
 graphs/graph_latex.py: 100% (10 of 10)
 graphs/schnyder.py: 100% (8 of 8)
@@ -1185,6 +1185,6 @@
 
 Overall weighted coverage score:  81.6%
 Total number of functions:  25377
-We need  852 more function to get to 85% coverage.
-We need 2121 more function to get to 90% coverage.
-We need 3390 more function to get to 95% coverage.
+We need  851 more function to get to 85% coverage.
+We need 2120 more function to get to 90% coverage.
+We need 3388 more function to get to 95% coverage.

comment:3 Changed 12 years ago by Minh Van Nguyen

Description: modified (diff)

comment:4 Changed 12 years ago by John Palmieri

Cc: John Palmieri added

comment:5 Changed 12 years ago by Minh Van Nguyen

Description: modified (diff)

comment:6 Changed 12 years ago by Tim Dumol

attachment:trac_8699-documentation.patch works as advertised, but I believe attachment:trac_8699-single-quotes.patch will choke on something like this:

def foo():
   '''
   foobarbaz
   """

   """
   [..]
   bar baz
   '''
   pass

comment:7 Changed 12 years ago by Tim Dumol

Status: needs_reviewneeds_work
Work issues: Slight bug in implementation.

comment:8 in reply to:  6 Changed 12 years ago by Minh Van Nguyen

Replying to timdumol:

attachment:trac_8699-documentation.patch works as advertised, but I believe attachment:trac_8699-single-quotes.patch will choke on something like this:

I don't think I understand your test case. Say I put your test case in a module, e.g. devel/sage-main/sage/graphs/graph_generators.py:

  • sage/graphs/graph_generators.py

    [mvngu@sage sage-main]$ hg diff
    diff --git a/sage/graphs/graph_generators.py b/sage/graphs/graph_generators.py
    a b  
    168168from   math import sin, cos, pi
    169169from sage.misc.randstate import current_randstate
    170170
     171def foo():
     172   '''
     173   foobarbaz
     174   """
     175
     176   """
     177   [..]
     178   bar baz
     179   '''
     180   pass
     181
    171182class GraphGenerators():
    172183    r"""
    173184    A class consisting of constructors for several common graphs, as

Running the doctest coverage script over this modified module reported the following:

[mvngu@sage sage-4.4.alpha0-8699-quotes]$ ./sage -coverage devel/sage-main/sage/graphs/graph_generators.py 
----------------------------------------------------------------------
devel/sage-main/sage/graphs/graph_generators.py
ERROR: Please add a `TestSuite(s).run()` doctest.
SCORE devel/sage-main/sage/graphs/graph_generators.py: 98% (74 of 75)

Missing doctests:
         * foo():

----------------------------------------------------------------------

So we have added another function, which is missing doctests. Or do you mean that the coverage script needs to ensure a balance of triple single/double quotes? In that case, I think a decent text editor that supports Python/Cython? syntax would do a better job to highlight if we have, say, a triple single quotes for starting a docstring but triple double quotes to end the docstring.

comment:9 Changed 12 years ago by Tim Dumol

I realize that test case is wrong. Here's what I meant:

Inserting this:

def foo():
   '''
   foobarbaz
   """

   """
   [..]
   
   EXAMPLES::

      sage: print 5
      5
   '''
   pass

into a file will result in a missing doctests warning, despite the fact that it does have a doctest. This is because the first triple single quote is matched with the first triple double quote, instead of the second triple single quote. It may be an extremely unlikely test case, but it is probably better to get this right now rather than puzzle over it in the far future.

comment:10 Changed 12 years ago by Mike Hansen

Also, isn't there an issue if qd1 and qs1 are both != -1, but qd1 comes before qs1. I think you need to use the minimum of them.

Changed 10 years ago by Mike Hansen

Attachment: trac_8699.patch added

comment:11 Changed 10 years ago by Mike Hansen

Description: modified (diff)
Reviewers: Mike Hansen
Status: needs_workneeds_review

Minh, I think your documentation changes are fine, but I've gone ahead and updated the second patch to work in situations where there are both double and single triple quotes. Can you look at my patch?

comment:12 Changed 10 years ago by Mike Hansen

Cc: William Stein added
Keywords: 7 years added

comment:13 Changed 10 years ago by Travis Scrimshaw

Milestone: sage-5.7sage-duplicate/invalid/wontfix

This seems to be fixed by #14061:

travis@travis-virtualbox:~/sage-5.7.beta3/devel/sage-reviews/sage$ sage -coverage test_8699.py 
----------------------------------------------------------------------
test_8699.py
SCORE test_8699.py: 0% (0 of 4)

Missing documentation:
	 * foo():
	 * bar():
	 * baz():
	 * correct():

----------------------------------------------------------------------

with #14061 applied:

------------------------------------------------------------------------
SCORE test_8699.py: 50.0% (2 of 4)

Missing doctests:
     * line 1: def foo()
     * line 12: def bar()

Possibly wrong (function name doesn't occur in doctests):
     * line 18: def baz()
------------------------------------------------------------------------

My test file:

def foo():
    '''
    foobarbaz
    """

    """
    [..]
    bar baz
    '''
    pass

def bar():
    '''
    Fixme
    '''
    pass

def baz():
    '''
    Still working?
    """

    """
    [..]

    EXAMPLES::

        sage: print 5
        5
    '''
    pass

def correct():
    '''
    Although abit strange.
    """

    """
    [..]

    EXAMPLES::

        sage: correct()
    '''
    pass

Best,
Travis

comment:14 Changed 10 years ago by Jeroen Demeyer

Authors: Minh Van Nguyen
Reviewers: Mike HansenMinh Van Nguyen, Mike Hansen
Status: needs_reviewpositive_review
Work issues: Slight bug in implementation.

comment:15 Changed 10 years ago by Jeroen Demeyer

Resolution: duplicate
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.