Opened 15 years ago
Closed 14 years ago
#3476 closed enhancement (fixed)
[with patch, positive review] save timeit information with sage -t -timeit
Reported by: | ncalexan | Owned by: | failure |
---|---|---|---|
Priority: | major | Milestone: | sage-3.2 |
Component: | doctest coverage | Keywords: | testing doctest timing timeit profile regression, editor_mhansen |
Cc: | Merged in: | ||
Authors: | Reviewers: | ||
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
There are several parts to this patch.
The first is an update to sage.misc.sage_timeit
that generalizes the interface to return an object that prints itself as a timing string rather than the string itself. The advantage is that the information does not need to be parsed from the string later. This patch is a requirement of the later ones but is conceptually independent.
The second adds a file ncadoctest.py
to scripts that is a slightly modified version of Python's doctest.py
file. It is easier to subclass the various classes with this version.
The third uses ncadoctest.py
to subclass the doctest architecture and updates sage-doctest
to use these updated classes.
Attachments (5)
Change History (20)
Changed 15 years ago by
Attachment: | 3476-ncalexan-sage-scripts-doctest-1.patch added |
---|
Changed 15 years ago by
Attachment: | 3476-ncalexan-sage-timeit-1.patch added |
---|
comment:1 Changed 15 years ago by
Changed 15 years ago by
Attachment: | diff-python-doctest-to-ncadoctest added |
---|
comment:2 Changed 15 years ago by
diff-python-doctest-to-ncadoctest
records the changes from upstream Python doctest.py
to ncadoctest.py
for future reference.
comment:3 Changed 15 years ago by
Keywords: | editor_mabshoff added |
---|
comment:4 Changed 14 years ago by
Keywords: | editor_gfurnish added |
---|
comment:6 Changed 14 years ago by
Keywords: | editor_mhansen added; editor_mabshoff editor_gfurnish removed |
---|
comment:7 Changed 14 years ago by
Summary: | [with patch, needs review] save timeit information with sage -t -timeit → [with patch, positive review] save timeit information with sage -t -timeit |
---|
Since my one main concern at #3982 is taken care of, I think this can go in.
comment:8 Changed 14 years ago by
There is a reject apllying Nick's first patch:
--- sage-doctest +++ sage-doctest @@ -218,8 +241,8 @@ def extract_doc(file_name, module): doc = doc_preparse(F[i:j+3]) except SyntaxError: doc = F[i:j+3] - if len(doc): - doc = '""">>> set_random_seed(0L)\n\n' + doc[3:] +# if len(doc): +# doc = '""">>> print "YYY"; print random() # ; set_random_seed(0L)\n\n' + doc[3:] s += "\tr"+ doc + "\n\n" F = F[j+3:]
I am attempting to merge this manually.
Cheers,
Michael
comment:9 Changed 14 years ago by
Ok, the reject seems to happen due to merging the warning patch into sage-doctest.
Cheers,
Michael
comment:10 Changed 14 years ago by
Summary: | [with patch, positive review] save timeit information with sage -t -timeit → [with patch, needs works] save timeit information with sage -t -timeit |
---|
If I merge the patch without the troublesome hunk I see 4 doctests related to the random framework and timeit itself fail:
sage -t -long devel/sage/sage/misc/sage_timeit_class.pyx # 7 doctests failed sage -t -long devel/sage/sage/misc/sage_timeit.py # 2 doctests failed sage -t -long devel/sage/sage/misc/prandom.py # 2 doctests failed sage -t -long devel/sage/sage/misc/randstate.pyx # 6 doctests failed
Oh well, life sucks :)
Cheers,
Michael
comment:11 Changed 14 years ago by
Summary: | [with patch, needs works] save timeit information with sage -t -timeit → [with patch, needs work] save timeit information with sage -t -timeit |
---|
Changed 14 years ago by
Attachment: | trac_3476.patch added |
---|
Changed 14 years ago by
Attachment: | trac_3476-scripts.patch added |
---|
comment:12 Changed 14 years ago by
Summary: | [with patch, needs work] save timeit information with sage -t -timeit → [with patch, positive review] save timeit information with sage -t -timeit |
---|
I've added two updated patches which fix the issue. The problem was that the 'timeit' in test.globs was set to "False" from the options in sage-doctest instead of being the actual timeit function from Sage. Thus, you'd only hit the problem with doctests that used timeit.
comment:13 Changed 14 years ago by
Milestone: | sage-3.2.1 → sage-3.2 |
---|
For "sage -sdist" to work we need to copy ncadoctest.py, sagedoctest.py in sage-make_devel_packages after
cp -p $SAGE_ROOT/local/bin/SbuildHack.pm $SCRIPTS/
I will take care of this once the patch passes doctests.
Cheers,
Michael
comment:14 Changed 14 years ago by
Hi Mike,
there is one tiny easy to fix doctest issue left:
sage -t -long devel/sage/sage/misc/sage_timeit.py ********************************************************************** File "/scratch/mabshoff/release-cycle/sage-3.2.rc0/devel/sage/sage/misc/sage_timeit.py", line 48: sage: sage_timeit("a = 2\nb=131\nfactor(a^b-1)", globals(), number=10) Expected: '10 loops, best of 3: ... per loop' Got: 10 loops, best of 3: 18.4 ms per loop **********************************************************************
I will fix this via a followup patch.
Cheers,
Michael
The attachments came in the wrong order -- the one with
sage-scripts
applies to sage/local/bin.