Opened 11 years ago

Closed 8 years ago

#8699 closed enhancement (duplicate)

allow doctest coverage script to handle triple single quotes

Reported by: mvngu Owned by: tbd
Priority: minor Milestone: sage-duplicate/invalid/wontfix
Component: doctest coverage Keywords: 7 years
Cc: jhpalmieri, was 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 mhansen)

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 mvngu 11 years ago.
trac_8699-single-quotes.patch (1.1 KB) - added by mvngu 11 years ago.
trac_8699.patch (1.1 KB) - added by mhansen 9 years ago.

Download all attachments as: .zip

Change History (18)

comment:1 Changed 11 years ago by mvngu

  • Description modified (diff)

Changed 11 years ago by mvngu

Changed 11 years ago by mvngu

comment:2 Changed 11 years ago by mvngu

  • Authors set to Minh Van Nguyen
  • Description modified (diff)
  • Status changed from new to needs_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 11 years ago by mvngu

  • Description modified (diff)

comment:4 Changed 11 years ago by jhpalmieri

  • Cc jhpalmieri added

comment:5 Changed 11 years ago by mvngu

  • Description modified (diff)

comment:6 follow-up: Changed 11 years ago by timdumol

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 11 years ago by timdumol

  • Status changed from needs_review to needs_work
  • Work issues set to Slight bug in implementation.

comment:8 in reply to: ↑ 6 Changed 11 years ago by mvngu

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 11 years ago by timdumol

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 11 years ago by mhansen

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 9 years ago by mhansen

comment:11 Changed 9 years ago by mhansen

  • Description modified (diff)
  • Reviewers set to Mike Hansen
  • Status changed from needs_work to needs_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 9 years ago by mhansen

  • Cc was added
  • Keywords 7 years added

comment:13 Changed 8 years ago by tscrim

  • Milestone changed from sage-5.7 to sage-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 8 years ago by jdemeyer

  • Authors Minh Van Nguyen deleted
  • Reviewers changed from Mike Hansen to Minh Van Nguyen, Mike Hansen
  • Status changed from needs_review to positive_review
  • Work issues Slight bug in implementation. deleted

comment:15 Changed 8 years ago by jdemeyer

  • Resolution set to duplicate
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.