Opened 11 years ago
Closed 8 years ago
#8708 closed enhancement (duplicate)
allow doctest script to handle docstrings with triple single quotes
Reported by:  mvngu  Owned by:  tbd 

Priority:  minor  Milestone:  sageduplicate/invalid/wontfix 
Component:  doctest coverage  Keywords:  sd41 
Cc:  ranosch  Merged in:  
Authors:  Reviewers:  John Palmieri, Burcin Erocal  
Report Upstream:  N/A  Work issues:  
Branch:  Commit:  
Dependencies:  Stopgaps: 
Description (last modified by )
As the subject says. This is related to #8699. See this sagedevel thread for some background information.
Apply
 trac_8708jhp.v2.patch to the scripts repository
 trac_8708doctests.patch to the Sage repository.
Attachments (5)
Change History (29)
Changed 10 years ago by
comment:1 Changed 10 years ago by
 Cc ranosch added
 Milestone changed from sage4.7.1 to sage4.7.2
 Status changed from new to needs_review
comment:2 followup: ↓ 3 Changed 10 years ago by
This doesn't work for me: it skips all doctests in a typical file. The issue is the line
min_ind = min(dq_ind, sq_ind)
which means that if either (not just both) search fails, then min_ind
is 1
, so the loop exits.
I also don't think we need to modify doc_prefix
and doc_suffix
, since this is part of the string written to the temporary doctesting file. Do the triple quotes in that file have to match the original ones, or can they all be """
?
I'm attaching a new patch which uses regular expressions instead, and which doesn't modify doc_prefix
and doc_suffix
. I'm also attaching a small file for testing purposes; testing on this file should produce three failures, and if you add some print statements to the function extract_doc
, it should extract the correct string.
comment:3 in reply to: ↑ 2 Changed 10 years ago by
Replying to jhpalmieri:
This doesn't work for me: it skips all doctests in a typical file. The issue is the line
min_ind = min(dq_ind, sq_ind)which means that if either (not just both) search fails, then
min_ind
is1
, so the loop exits.
Good catch! Thanks for looking into this.
I also don't think we need to modify
doc_prefix
anddoc_suffix
, since this is part of the string written to the temporary doctesting file. Do the triple quotes in that file have to match the original ones, or can they all be"""
?
What happens if you have something like:
def f(arg): ''' some text """ some more text """ '''
comment:4 Changed 10 years ago by
Oh, you're right. I'm replacing my patch with a new one, the difference being

sagedoctest
diff git a/sagedoctest b/sagedoctest
a b def change_warning_output(file): 452 452 tmpfiles.append(os.path.join(SAGE_TESTDIR, name + '.pyc')) 453 453 454 454 # Prefix/suffix for all doctests replacing the starting/ending """ 455 doc_prefix = 'r""">>> set_random_seed(0L)\n\n>>> change_warning_output(sys.stdout)\n\n' 456 doc_suffix = '\n>>> sig_on_count()\n0\n"""' 455 doc_prefix_dq = 'r""">>> set_random_seed(0L)\n\n>>> change_warning_output(sys.stdout)\n\n' 456 doc_prefix_sq = doc_prefix_dq.replace('"""',"'''") 457 doc_suffix_dq = '\n>>> sig_on_count()\n0\n"""' 458 doc_suffix_sq = doc_suffix_dq.replace('"""',"'''") 457 459 458 460 n = 0 459 461 # Now extract the docstring by using a regular expression search … … def change_warning_output(file): 499 501 name = "example" 500 502 s += "def %s_%s():"%(name,n_str) 501 503 n += 1 502 s += "\t"+ doc_prefix + doc + doc_suffix + "\n\n" 504 if m.groups()[0].find("'") > 1: # single quotes 505 s += "\t"+ doc_prefix_sq + doc + doc_suffix_sq + "\n\n" 506 else: 507 s += "\t"+ doc_prefix_dq + doc + doc_suffix_dq + "\n\n" 503 508 504 509 if n == 0: 505 510 return ''
I'll also add an example like this to the testing file.
comment:5 Changed 9 years ago by
 Reviewers set to Burcin Erocal
 Status changed from needs_review to positive_review
Looks good to me. Thanks for looking into this John.
comment:6 Changed 9 years ago by
 Description modified (diff)
comment:7 Changed 9 years ago by
 Dependencies set to #9739
 Status changed from positive_review to needs_work
 Work issues set to Rebase on #9739 (and perhaps also 10952).
comment:8 Changed 9 years ago by
 Priority changed from minor to blocker
 Status changed from needs_work to needs_review
We missed an or two issue in #9739: with nonlibrary files, first we just had a bug, using "source" instead of "file_name" or "root_name" or something. Second, in the import statement added to the file,
s += "from %s import *\n\n" % target_name
we need to replace target_name
with os.path.basename(target_name)
.
In the new patch, I've made these two changes and also rebased. Given that there is actually new content, this needs a new review. Without the new content, doctesting .py nonlibrary files completely fails, so I'm marking this as a blocker. Feel free to downgrade if you think it's appropriate.
Oh, I also realized that the attached file for testing purposes has a bad name: replace the hyphen with an underscore.
The part needing review:

sagedoctest
diff git a/sagedoctest b/sagedoctest
a b def check_with_tolerance(expected, actua 510 510 511 511 elif ext in ['.py', '.sage']: 512 512 513 target_name = "%s_%d" % ( file_name, os.getpid()) # like 'name', but513 target_name = "%s_%d" % (root_name, os.getpid()) # like 'name', but 514 514 target_base = os.path.join(SAGE_TESTDIR, target_name) # like 'base' 515 515 516 516 if ext == '.sage': … … def check_with_tolerance(expected, actua 528 528 # TODO: instead of copying the file, add its source 529 529 # directory to PYTHONPATH. We would also have to 530 530 # import from 'name' instead of 'target_name'. 531 os.system("cp '%s' %s.py" % ( source, target_base))531 os.system("cp '%s' %s.py" % (file_name, target_base)) 532 532 533 s += "from %s import *\n\n" % target_name533 s += "from %s import *\n\n" % os.path.basename(target_name) 534 534 535 535 tmpfiles.append(target_base + ".py") # preparsed or copied original 536 536 tmpfiles.append(target_base + ".pyc") # compiled version of it
comment:9 Changed 9 years ago by
 Description modified (diff)
comment:10 Changed 9 years ago by
Sorry, here's a new version. The relevant changes:

sagedoctest
diff git a/sagedoctest b/sagedoctest
a b def check_with_tolerance(expected, actua 510 510 511 511 elif ext in ['.py', '.sage']: 512 512 513 target_name = "%s_%d" % (file_name, os.getpid()) # like 'name', but unique 514 target_base = os.path.join(SAGE_TESTDIR, target_name) # like 'base', also unique 513 root_name = os.path.basename(root_name) 514 target_name = "%s_%d" % (root_name, os.getpid()) # like 'root_name', but unique 515 target_base = os.path.join(SAGE_TESTDIR, target_name) # like 'target_name' but with full path 515 516 516 517 if ext == '.sage': 517 518 # TODO: preparse "<file>.sage" with a Sage library call … … def check_with_tolerance(expected, actua 528 529 # TODO: instead of copying the file, add its source 529 530 # directory to PYTHONPATH. We would also have to 530 531 # import from 'name' instead of 'target_name'. 531 os.system("cp '%s' %s.py" % ( source, target_base))532 os.system("cp '%s' %s.py" % (file_name, target_base)) 532 533 533 534 s += "from %s import *\n\n" % target_name
Changing "source" to "file_name" is necessary: doctesting nonlibrary py files fails otherwise. The other change is to avoid failures if you specify a path name for the file to be tested: sage t ./test.py
will fail without this, as compared to sage t test.py
.
comment:11 Changed 9 years ago by
Well, at some point in the evolution of #9739 it worked (or was correct)... ;)
Can anybody [else] review this [quickly]?
Sorry, I don't have the time, and I'm actually also not in the mood right now.
comment:12 Changed 9 years ago by
Some things to test:
 First apply the patches from #9739 and #10952.
 Create a file "test.py" (not a Sage library file) and test it with "sage t test.py", and also with "sage t /path/to/test.py". Try before and after applying trac_8708jhp.v2.patch. You could just use trac_8708testing.py, but rename it so the name doesn't have any hyphens. This file is supposed to have 3 failures.
 Do the same with a file "test.sage". (Just renaming your Python file to "test.sage" should be good enough.)
comment:13 Changed 9 years ago by
 Status changed from needs_review to needs_work
I now get the following doctest failures, certainly due to the triple single quote patch / modification:
The following tests failed: sage t long force_lib devel/sagenbmain/sagenb/notebook/worksheet.py # Exception from doctest framework sage t long force_lib devel/sage/sage/interacts/library_cython.pyx # 1 doctests failed sage t long force_lib devel/sage/sage/misc/sageinspect.py # Exception from doctest framework
With verbose
:
$ ./sage t long verbose devel/sagenbmain/sagenb/notebook/worksheet.py sage t long verbose "devel/sagenbmain/sagenb/notebook/worksheet.py" Traceback (most recent call last): File "/home/leif/.sage//tmp/worksheet_15100.py", line 3046, in <module> runner=runner) File "/home/leif/Sage/sage4.7.2.alpha3prerelease3/local/bin/sagedoctest.py", line 54, in testmod_returning_runner runner=runner) File "/home/leif/Sage/sage4.7.2.alpha3prerelease3/local/bin/ncadoctest.py", line 1819, in testmod_returning_runner for test in finder.find(m, name, globs=globs, extraglobs=extraglobs): File "/home/leif/Sage/sage4.7.2.alpha3prerelease3/local/bin/ncadoctest.py", line 839, in find self._find(tests, obj, name, module, source_lines, globs, {}) File "/home/leif/Sage/sage4.7.2.alpha3prerelease3/local/bin/ncadoctest.py", line 893, in _find globs, seen) File "/home/leif/Sage/sage4.7.2.alpha3prerelease3/local/bin/ncadoctest.py", line 881, in _find test = self._get_test(obj, name, module, globs, source_lines) File "/home/leif/Sage/sage4.7.2.alpha3prerelease3/local/bin/ncadoctest.py", line 965, in _get_test filename, lineno) File "/home/leif/Sage/sage4.7.2.alpha3prerelease3/local/bin/ncadoctest.py", line 594, in get_doctest return DocTest(self.get_examples(string, name), globs, File "/home/leif/Sage/sage4.7.2.alpha3prerelease3/local/bin/ncadoctest.py", line 608, in get_examples return [x for x in self.parse(string, name) File "/home/leif/Sage/sage4.7.2.alpha3prerelease3/local/bin/ncadoctest.py", line 570, in parse self._parse_example(m, name, lineno) File "/home/leif/Sage/sage4.7.2.alpha3prerelease3/local/bin/ncadoctest.py", line 640, in _parse_example lineno + len(source_lines)) File "/home/leif/Sage/sage4.7.2.alpha3prerelease3/local/bin/ncadoctest.py", line 726, in _check_prefix (lineno+i+1, name, line)) ValueError: line 11 of the docstring for __main__.example_109 has inconsistent leading whitespace: ' "' Exception raised by doctesting framework. Use verbose for details.
$ ./sage t long verbose devel/sage/sage/misc/sageinspect.py sage t long verbose "devel/sage/sage/misc/sageinspect.py" Traceback (most recent call last): File "/home/leif/.sage//tmp/sageinspect_15131.py", line 1327, in <module> runner=runner) File "/home/leif/Sage/sage4.7.2.alpha3prerelease3/local/bin/sagedoctest.py", line 54, in testmod_returning_runner runner=runner) File "/home/leif/Sage/sage4.7.2.alpha3prerelease3/local/bin/ncadoctest.py", line 1819, in testmod_returning_runner for test in finder.find(m, name, globs=globs, extraglobs=extraglobs): File "/home/leif/Sage/sage4.7.2.alpha3prerelease3/local/bin/ncadoctest.py", line 839, in find self._find(tests, obj, name, module, source_lines, globs, {}) File "/home/leif/Sage/sage4.7.2.alpha3prerelease3/local/bin/ncadoctest.py", line 893, in _find globs, seen) File "/home/leif/Sage/sage4.7.2.alpha3prerelease3/local/bin/ncadoctest.py", line 881, in _find test = self._get_test(obj, name, module, globs, source_lines) File "/home/leif/Sage/sage4.7.2.alpha3prerelease3/local/bin/ncadoctest.py", line 965, in _get_test filename, lineno) File "/home/leif/Sage/sage4.7.2.alpha3prerelease3/local/bin/ncadoctest.py", line 594, in get_doctest return DocTest(self.get_examples(string, name), globs, File "/home/leif/Sage/sage4.7.2.alpha3prerelease3/local/bin/ncadoctest.py", line 608, in get_examples return [x for x in self.parse(string, name) File "/home/leif/Sage/sage4.7.2.alpha3prerelease3/local/bin/ncadoctest.py", line 570, in parse self._parse_example(m, name, lineno) File "/home/leif/Sage/sage4.7.2.alpha3prerelease3/local/bin/ncadoctest.py", line 640, in _parse_example lineno + len(source_lines)) File "/home/leif/Sage/sage4.7.2.alpha3prerelease3/local/bin/ncadoctest.py", line 726, in _check_prefix (lineno+i+1, name, line)) ValueError: line 53 of the docstring for __main__.example_28 has inconsistent leading whitespace: ' """' Exception raised by doctesting framework. Use verbose for details.
$ ./sage t long verbose devel/sage/sage/interacts/library_cython.pyxsage t long verbose "devel/sage/sage/interacts/library_cython.pyx" ... Trying: from sage.interacts.library_cython import cellular###line 91:_sage_ >>> from sage.interacts.library_cython import cellular Expecting nothing ok Trying: rule = [Integer(1), Integer(0), Integer(1), Integer(0), Integer(0), Integer(1), Integer(1), Integer(0)]###line 92:_sage_ >>> rule = [1, 0, 1, 0, 0, 1, 1, 0] Expecting nothing ok Trying: N = Integer(3)###line 93:_sage_ >>> N = 3 Expecting nothing ok Trying: print cellular(rule, Integer(3))###line 94:_sage_ >>> print cellular(rule, 3) Expecting nothing ********************************************************************** File "/home/leif/Sage/sage4.7.2.alpha3prerelease3/devel/sage/sage/interacts/library_cython.pyx", line ?, in __main__.example_3 Failed example: print cellular(rule, Integer(3))###line 94:_sage_ >>> print cellular(rule, 3) Expected nothing Got: [[0 0 0 1 0 0 0] [0 0 0 1 0 0 0] [0 1 0 1 0 1 0]] Trying: sig_on_count() Expecting: 0 ok 4 items had no tests: __main__ __main__.change_warning_output __main__.check_with_tolerance __main__.warning_function 3 items passed all tests: 3 tests in __main__.example_0 9 tests in __main__.example_1 8 tests in __main__.example_2 ********************************************************************** 1 items had failures: 1 of 7 in __main__.example_3 27 tests in 8 items. 26 passed and 1 failed. ***Test Failed*** 1 failures.
comment:14 Changed 9 years ago by
P.S.:
John, could you perhaps separate the fix concerning doctesting nonlibrary files, and attach it to #9739 (not modifying the other patches there, i.e., as a patch to be applied on top of the others)?
comment:15 followup: ↓ 16 Changed 9 years ago by
 Priority changed from blocker to minor
comment:16 in reply to: ↑ 15 Changed 9 years ago by
Replying to jhpalmieri:
Okay, I've done that now.
Thanks; just the commit message there refers to this ticket (and its subject), i.e., is unrelated.
#9739 is still marked as closed, by the way. I didn't reopen it.
Unless nobody else reopens or closes tickets, or attaches files to / updates files on tickets I've already closed (and I didn't ask him to) I don't really care.
Reopening tickets IMHO only makes sense if I'm going to actually back it out.
comment:17 Changed 9 years ago by
s/nobody/somebody/ or s/unless/as long as/
It's getting late...
comment:18 Changed 9 years ago by
 Description modified (diff)
 Status changed from needs_work to needs_info
The test failure in interacts/libary_cython.pyx is because of a doctest which would have failed in the past, but it was enclosed in triple single quotes, and so was skipped by the doctesting framework. (There is a line saying sage: print cellular(rule, 3)
with no output after it.) We can certainly just stick in the output, but I have no idea if it's right.
I have a fix for sageinspect.py, but I'm not sure about worksheet.py. I can work on it some more, but we should check whether there are failures in the flask notebook before working too hard on this one: if the file is going to go away soon, it's not worth working too hard fixing it.
I'm attaching a patch for the Sage library fixing the two files there; I'll try to get information about whether the patch to library_cython.pyx is actually the right thing to do.
comment:19 Changed 9 years ago by
For the sagenb problem, this patch seems to fix things:

sagenb/notebook/worksheet.py
diff git a/sagenb/notebook/worksheet.py b/sagenb/notebook/worksheet.py
a b except (KeyError, IOError): 3881 3881 C.delete_output() 3882 3882 3883 3883 3884 __internal_test1 = ''' 3885 def foo(x): 3886 " 3887 EXAMPLES: 3888 sage: 2+2 3889 4 3890 " 3891 return x 3892 '''.lstrip() 3884 __internal_test1 = '\ndef foo(x):\n "\n EXAMPLES:\n sage: 2+2\n 4\n "\n return x'.lstrip() 3893 3885 3894 3886 __internal_test2 = ''' 3895 3887 sage: 2 + 2
I don't know if it's the best approach. Why does this use single double quotes, anyway? Shouldn't the EXAMPLES block be surrounded by """
? Modifying the patch here to use """
still passes doctests.
comment:20 Changed 9 years ago by
 Dependencies changed from #9739 to #9739, #11871
The problem with library_cython.pyx should be dealt with at #11871, so I'm making that a dependency. I'll remove that part of the doctest patch.
comment:21 Changed 9 years ago by
 Status changed from needs_info to needs_review
I've submitted a patch for the flask notebook to deal with the issue with notebook/worksheet, and that should be merged soon. There is a patch at #11871 for interacts/library_cython.pyx. So I think this is now ready to go.
comment:22 Changed 9 years ago by
 Keywords sd41 added
 Work issues Rebase on #9739 (and perhaps also 10952). deleted
comment:23 Changed 8 years ago by
 Dependencies #9739, #11871 deleted
 Milestone changed from sage5.8 to sageduplicate/invalid/wontfix
 Reviewers changed from Burcin Erocal to John Palmieri, Burcin Erocal
 Status changed from needs_review to positive_review
Fixed by #12415.
comment:24 Changed 8 years ago by
 Resolution set to duplicate
 Status changed from positive_review to closed
attachment:trac_8708doctest_quotes.patch fixes this problem.