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: | sage-duplicate/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 sage-devel thread for some background information.
Apply
- trac_8708-jhp.v2.patch to the scripts repository
- trac_8708-doctests.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 sage-4.7.1 to sage-4.7.2
- Status changed from new to needs_review
comment:2 follow-up: ↓ 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
is-1
, 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
-
sage-doctest
diff --git a/sage-doctest b/sage-doctest
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 non-library 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 non-library 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:
-
sage-doctest
diff --git a/sage-doctest b/sage-doctest
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:
-
sage-doctest
diff --git a/sage-doctest b/sage-doctest
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 non-library 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_8708-jhp.v2.patch. You could just use trac_8708-testing.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/sagenb-main/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/sagenb-main/sagenb/notebook/worksheet.py sage -t -long -verbose "devel/sagenb-main/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/sage-4.7.2.alpha3-prerelease3/local/bin/sagedoctest.py", line 54, in testmod_returning_runner runner=runner) File "/home/leif/Sage/sage-4.7.2.alpha3-prerelease3/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/sage-4.7.2.alpha3-prerelease3/local/bin/ncadoctest.py", line 839, in find self._find(tests, obj, name, module, source_lines, globs, {}) File "/home/leif/Sage/sage-4.7.2.alpha3-prerelease3/local/bin/ncadoctest.py", line 893, in _find globs, seen) File "/home/leif/Sage/sage-4.7.2.alpha3-prerelease3/local/bin/ncadoctest.py", line 881, in _find test = self._get_test(obj, name, module, globs, source_lines) File "/home/leif/Sage/sage-4.7.2.alpha3-prerelease3/local/bin/ncadoctest.py", line 965, in _get_test filename, lineno) File "/home/leif/Sage/sage-4.7.2.alpha3-prerelease3/local/bin/ncadoctest.py", line 594, in get_doctest return DocTest(self.get_examples(string, name), globs, File "/home/leif/Sage/sage-4.7.2.alpha3-prerelease3/local/bin/ncadoctest.py", line 608, in get_examples return [x for x in self.parse(string, name) File "/home/leif/Sage/sage-4.7.2.alpha3-prerelease3/local/bin/ncadoctest.py", line 570, in parse self._parse_example(m, name, lineno) File "/home/leif/Sage/sage-4.7.2.alpha3-prerelease3/local/bin/ncadoctest.py", line 640, in _parse_example lineno + len(source_lines)) File "/home/leif/Sage/sage-4.7.2.alpha3-prerelease3/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/sage-4.7.2.alpha3-prerelease3/local/bin/sagedoctest.py", line 54, in testmod_returning_runner runner=runner) File "/home/leif/Sage/sage-4.7.2.alpha3-prerelease3/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/sage-4.7.2.alpha3-prerelease3/local/bin/ncadoctest.py", line 839, in find self._find(tests, obj, name, module, source_lines, globs, {}) File "/home/leif/Sage/sage-4.7.2.alpha3-prerelease3/local/bin/ncadoctest.py", line 893, in _find globs, seen) File "/home/leif/Sage/sage-4.7.2.alpha3-prerelease3/local/bin/ncadoctest.py", line 881, in _find test = self._get_test(obj, name, module, globs, source_lines) File "/home/leif/Sage/sage-4.7.2.alpha3-prerelease3/local/bin/ncadoctest.py", line 965, in _get_test filename, lineno) File "/home/leif/Sage/sage-4.7.2.alpha3-prerelease3/local/bin/ncadoctest.py", line 594, in get_doctest return DocTest(self.get_examples(string, name), globs, File "/home/leif/Sage/sage-4.7.2.alpha3-prerelease3/local/bin/ncadoctest.py", line 608, in get_examples return [x for x in self.parse(string, name) File "/home/leif/Sage/sage-4.7.2.alpha3-prerelease3/local/bin/ncadoctest.py", line 570, in parse self._parse_example(m, name, lineno) File "/home/leif/Sage/sage-4.7.2.alpha3-prerelease3/local/bin/ncadoctest.py", line 640, in _parse_example lineno + len(source_lines)) File "/home/leif/Sage/sage-4.7.2.alpha3-prerelease3/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/sage-4.7.2.alpha3-prerelease3/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 non-library 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 follow-up: ↓ 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 sage-5.8 to sage-duplicate/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_8708-doctest_quotes.patch fixes this problem.