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:  sageduplicate/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: 
Description (last modified by )
As the subject says. See this sagedevel thread for some background information.
This might conflict with ticket #7716. See #8708 for a related ticket.
Apply patches in this order:
 trac_8699documentation.patch  Document the doctest coverage script. Don't redefine the builtin function name "file"; use "afile" instead. Consistently use double quotes wherever possible. General stylistic cleanups a la PEP 008.
 trac_8699.patch  Allow the doctest coverage script to handle triple single quotes.
Attachments (3)
Change History (18)
comment:1 Changed 11 years ago by
 Description modified (diff)
Changed 11 years ago by
Changed 11 years ago by
comment:2 Changed 11 years ago by
 Description modified (diff)
 Status changed from new to needs_review
comment:3 Changed 11 years ago by
 Description modified (diff)
comment:4 Changed 11 years ago by
 Cc jhpalmieri added
comment:5 Changed 11 years ago by
 Description modified (diff)
comment:6 followup: ↓ 8 Changed 11 years ago by
attachment:trac_8699documentation.patch works as advertised, but I believe attachment:trac_8699singlequotes.patch will choke on something like this:
def foo(): ''' foobarbaz """ """ [..] bar baz ''' pass
comment:7 Changed 11 years ago by
 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
Replying to timdumol:
attachment:trac_8699documentation.patch works as advertised, but I believe attachment:trac_8699singlequotes.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/sagemain/sage/graphs/graph_generators.py:

sage/graphs/graph_generators.py
[mvngu@sage sagemain]$ hg diff diff git a/sage/graphs/graph_generators.py b/sage/graphs/graph_generators.py
a b 168 168 from math import sin, cos, pi 169 169 from sage.misc.randstate import current_randstate 170 170 171 def foo(): 172 ''' 173 foobarbaz 174 """ 175 176 """ 177 [..] 178 bar baz 179 ''' 180 pass 181 171 182 class GraphGenerators(): 172 183 r""" 173 184 A class consisting of constructors for several common graphs, as
Running the doctest coverage script over this modified module reported the following:
[mvngu@sage sage4.4.alpha08699quotes]$ ./sage coverage devel/sagemain/sage/graphs/graph_generators.py  devel/sagemain/sage/graphs/graph_generators.py ERROR: Please add a `TestSuite(s).run()` doctest. SCORE devel/sagemain/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
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
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
comment:11 Changed 9 years ago by
 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
 Cc was added
 Keywords 7 years added
comment:13 Changed 8 years ago by
 Milestone changed from sage5.7 to sageduplicate/invalid/wontfix
This seems to be fixed by #14061:
travis@travisvirtualbox:~/sage5.7.beta3/devel/sagereviews/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
 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
 Resolution set to duplicate
 Status changed from positive_review to closed
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 modulesage.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: