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:  sageduplicate/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: 
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 12 years ago by
Description:  modified (diff) 

Changed 12 years ago by
Attachment:  trac_8699documentation.patch added 

Changed 12 years ago by
Attachment:  trac_8699singlequotes.patch added 

comment:2 Changed 12 years ago by
Authors:  → Minh Van Nguyen 

Description:  modified (diff) 
Status:  new → needs_review 
comment:3 Changed 12 years ago by
Description:  modified (diff) 

comment:4 Changed 12 years ago by
Cc:  John Palmieri added 

comment:5 Changed 12 years ago by
Description:  modified (diff) 

comment:6 followup: 8 Changed 12 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 12 years ago by
Status:  needs_review → needs_work 

Work issues:  → Slight bug in implementation. 
comment:8 Changed 12 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 12 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 12 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 10 years ago by
Attachment:  trac_8699.patch added 

comment:11 Changed 10 years ago by
Description:  modified (diff) 

Reviewers:  → Mike Hansen 
Status:  needs_work → 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 10 years ago by
Cc:  William Stein added 

Keywords:  7 years added 
comment:13 Changed 10 years ago by
Milestone:  sage5.7 → 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 10 years ago by
Authors:  Minh Van Nguyen 

Reviewers:  Mike Hansen → Minh Van Nguyen, Mike Hansen 
Status:  needs_review → positive_review 
Work issues:  Slight bug in implementation. 
comment:15 Changed 10 years ago by
Resolution:  → duplicate 

Status:  positive_review → 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: