Opened 6 years ago

Closed 3 years ago

#10983 closed enhancement (fixed)

new doctest for french book about Sage

Reported by: casamayou Owned by: mvngu
Priority: major Milestone: sage-5.13
Component: doctest coverage Keywords:
Cc: zimmerma, saliola Merged in: sage-5.13.beta3
Authors: Alexandre Casamayou, Marc Mezzarobba, Paul Zimmermann Reviewers: Dmitrii Pasechnik
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by zimmerma)

The attached patch includes a new doctest for a book (in french) some people are writing about Sage, under the supervision of Paul Zimmermann.

This patch contains only the doctests for two chapters (about calculus and graphics and solutions for corresponding exercices).

This doctest was tested successfully with Sage 4.6.2 under Debian Squeeze.

Apply trac_10983_doctests_from_french_book_v3.patch

Attachments (20)

calculus.2.py (9.7 KB) - added by casamayou 6 years ago.
calculus.3.py (9.7 KB) - added by casamayou 6 years ago.
trac_10983.patch (31.8 KB) - added by casamayou 6 years ago.
calculus_fixed.py (9.9 KB) - added by casamayou 5 years ago.
graphique_1_fixed.py (1.8 KB) - added by casamayou 5 years ago.
graphique_2_fixed.py (1.8 KB) - added by casamayou 5 years ago.
graphique_3_fixed.py (4.4 KB) - added by casamayou 5 years ago.
sol_calculus_fixed.py (7.7 KB) - added by casamayou 5 years ago.
sol_graphiques_1_fixed.py (3.0 KB) - added by casamayou 5 years ago.
sol_graphiques_2_fixed.py (802 bytes) - added by casamayou 5 years ago.
calculus.py (10.1 KB) - added by casamayou 5 years ago.
updated file
graphique_1.py (1.9 KB) - added by casamayou 5 years ago.
updated file
graphique_2.py (1.8 KB) - added by casamayou 5 years ago.
updated file
graphique_3.py (4.4 KB) - added by casamayou 5 years ago.
updated file
sol_calculus.py (7.7 KB) - added by casamayou 5 years ago.
updated file
sol_graphiques_1.py (3.0 KB) - added by casamayou 5 years ago.
updated file
sol_graphiques_2.py (804 bytes) - added by casamayou 5 years ago.
updated file
trac_10983_doctests_from_french_book.patch (40.0 KB) - added by zimmerma 4 years ago.
Combined patch
trac_10983_doctests_from_french_book_v2.patch (41.5 KB) - added by chapoton 4 years ago.
trac_10983_doctests_from_french_book_v3.patch (23.9 KB) - added by zimmerma 3 years ago.

Download all attachments as: .zip

Change History (59)

comment:1 Changed 6 years ago by zimmerma

  • Cc zimmerma added

Changed 6 years ago by casamayou

comment:2 Changed 6 years ago by casamayou

comment:3 Changed 6 years ago by zimmerma

  • Cc saliola added
  • Status changed from new to needs_review

Franco, please could you review this ticket for inclusion in the Sage doctests? This will ensure stability of the example of our book (see #9395).

Thank you, Paul

comment:4 follow-up: Changed 6 years ago by saliola

  • Status changed from needs_review to needs_info

Hello Paul. I think it is a great idea to include these tests in Sage. However, the files are just python files that contain docstrings; I suspect that one would prefer a patch instead (perhaps I am wrong about this). In order to produce a patch, one needs to first find the best place to put these tests within the sage library. Is there a standard location for such tests?

comment:5 in reply to: ↑ 4 Changed 6 years ago by zimmerma

Replying to saliola:

Hello Paul. I think it is a great idea to include these tests in Sage. However, the files are just python files that contain docstrings; I suspect that one would prefer a patch instead (perhaps I am wrong about this). In order to produce a patch, one needs to first find the best place to put these tests within the sage library. Is there a standard location for such tests?

Franco, you are right. The standard location is tests/french_book where there is already the Python file corresponding to #9395.

Alexandre, please could you make a real patch? Do you know how to do that?

Paul

Changed 6 years ago by casamayou

Changed 6 years ago by casamayou

comment:6 Changed 6 years ago by casamayou

Paul, Here is the log-message for the patch.

Alexandre.

comment:7 Changed 5 years ago by dimpase

  • Status changed from needs_info to needs_review

comment:8 Changed 5 years ago by dimpase

  • Status changed from needs_review to positive_review

comment:9 Changed 5 years ago by jdemeyer

  • Authors set to Alexandre Casamayou
  • Description modified (diff)
  • Reviewers set to Dmitrii Pasechnik

comment:10 follow-up: Changed 5 years ago by jdemeyer

I am slightly worried about timings. Did you check how long the tests take? You might need to add #long time in various places.

comment:11 in reply to: ↑ 10 ; follow-up: Changed 5 years ago by dimpase

Replying to jdemeyer:

I am slightly worried about timings. Did you check how long the tests take? You might need to add #long time in various places.

the longest test took about 2 minutes on 1.8Gz Intel. Is it something that should be marked as long? Do we have any specific guidelines on this?

comment:12 in reply to: ↑ 11 ; follow-up: Changed 5 years ago by jdemeyer

Replying to dimpase:

the longest test took about 2 minutes on 1.8Gz Intel. Is it something that should be marked as long? Do we have any specific guidelines on this?

Guidelines say: anything more than 1 second (on sage.math) should be marked as long time. No test should take more than 30 seconds (even though we have several tests taking more than 30 seconds).

comment:13 in reply to: ↑ 12 ; follow-up: Changed 5 years ago by dimpase

  • Status changed from positive_review to needs_work

Replying to jdemeyer:

Replying to dimpase:

the longest test took about 2 minutes on 1.8Gz Intel. Is it something that should be marked as long? Do we have any specific guidelines on this?

Guidelines say: anything more than 1 second (on sage.math) should be marked as long time. No test should take more than 30 seconds (even though we have several tests taking more than 30 seconds).

Oops, sorry. I'll revert to needs_work then! As well, there are tests that ought to be broken into several subtests - as they are really long sequences of independent tests. Would be great if this it taken care of, too.

comment:14 in reply to: ↑ 13 ; follow-up: Changed 5 years ago by casamayou

Replying to dimpase:

As well, there are tests that ought to be broken into several subtests - as they are really long sequences of independent tests. Would be great if this it taken care of, too.

By breaking tests into several subtests, did you think test as file or as code line ? Shall I break the files 'graphique*.py' and 'sol_graphiques*.py' into shortest files ?

comment:15 in reply to: ↑ 14 ; follow-up: Changed 5 years ago by dimpase

Replying to casamayou:

Replying to dimpase:

As well, there are tests that ought to be broken into several subtests - as they are really long sequences of independent tests. Would be great if this it taken care of, too.

By breaking tests into several subtests, did you think test as file or as code line ? Shall I break the files 'graphique*.py' and 'sol_graphiques*.py' into shortest files ?

Yes, this looks like a good idea . Perhaps there is also a way to have several independent tests in one file, I don't know.

comment:16 in reply to: ↑ 15 ; follow-up: Changed 5 years ago by jdemeyer

Replying to dimpase:

Perhaps there is also a way to have several independent tests in one file, I don't know.

Of course you can have several independent tests in one file, just like you can have more than one function or more than one class in a file. Just put several test blocks in the file:

r"""
test1
"""

r"""
test2
"""

instead of

r"""
test1
test2
"""

comment:17 in reply to: ↑ 16 Changed 5 years ago by casamayou

Replying to jdemeyer:

Replying to dimpase:

Perhaps there is also a way to have several independent tests in one file, I don't know.

Of course you can have several independent tests in one file, just like you can have more than one function or more than one class in a file. Just put several test blocks in the file:

Attached you will find the test files corrected.

The too long tests were commented: # long time

Does that sound correct ?

Here are the execution time :

$ for i in *.py ; do sage -t $i; done; 
sage -t  "calculus_fixed.py"                                
	 [11.2 s]
 
----------------------------------------------------------------------
All tests passed!
Total time for all tests: 11.2 seconds
sage -t  "graphique_1_fixed.py"                             
	 [14.9 s]
 
----------------------------------------------------------------------
All tests passed!
Total time for all tests: 15.0 seconds
sage -t  "graphique_2_fixed.py"                             
	 [19.4 s]
 
----------------------------------------------------------------------
All tests passed!
Total time for all tests: 19.4 seconds
sage -t  "graphique_3_fixed.py"                             
	 [8.3 s]
 
----------------------------------------------------------------------
All tests passed!
Total time for all tests: 8.4 seconds
sage -t  "sol_calculus_fixed.py"                            
	 [8.9 s]
 
----------------------------------------------------------------------
All tests passed!
Total time for all tests: 9.0 seconds
sage -t  "sol_graphiques_1_fixed.py"                        
	 [13.0 s]
 
----------------------------------------------------------------------
All tests passed!
Total time for all tests: 13.1 seconds
sage -t  "sol_graphiques_2_fixed.py"                        
	 [4.8 s]
 
----------------------------------------------------------------------
All tests passed!
Total time for all tests: 4.8 seconds

And with the long tests :

$ for i in *.py ; do sage -t -long $i; done; 
sage -t -long "calculus_fixed.py"                           
	 [11.2 s]
 
----------------------------------------------------------------------
All tests passed!
Total time for all tests: 11.3 seconds
sage -t -long "graphique_1_fixed.py"                        
	 [323.1 s]
 
----------------------------------------------------------------------
All tests passed!
Total time for all tests: 323.1 seconds
sage -t -long "graphique_2_fixed.py"                        
	 [20.3 s]
 
----------------------------------------------------------------------
All tests passed!
Total time for all tests: 20.3 seconds
sage -t -long "graphique_3_fixed.py"                        
	 [8.1 s]
 
----------------------------------------------------------------------
All tests passed!
Total time for all tests: 8.1 seconds
sage -t -long "sol_calculus_fixed.py"                       
	 [8.9 s]
 
----------------------------------------------------------------------
All tests passed!
Total time for all tests: 9.0 seconds
sage -t -long "sol_graphiques_1_fixed.py"                   
	 [47.3 s]
 
----------------------------------------------------------------------
All tests passed!
Total time for all tests: 47.3 seconds
sage -t -long "sol_graphiques_2_fixed.py"                   
	 [64.9 s]
 
----------------------------------------------------------------------
All tests passed!
Total time for all tests: 65.0 seconds

It was the execution of the following instruction that is very long :

sage: f = lambda x, y : (abs(cos((x + I * y) ** 4)) - 1)
sage: g2 = implicit_plot(f, (-3, 3), (-3, 3), plot_points=400) # long time 

Changed 5 years ago by casamayou

Changed 5 years ago by casamayou

Changed 5 years ago by casamayou

Changed 5 years ago by casamayou

Changed 5 years ago by casamayou

Changed 5 years ago by casamayou

Changed 5 years ago by casamayou

Changed 5 years ago by casamayou

updated file

Changed 5 years ago by casamayou

updated file

Changed 5 years ago by casamayou

updated file

Changed 5 years ago by casamayou

updated file

Changed 5 years ago by casamayou

updated file

Changed 5 years ago by casamayou

updated file

Changed 5 years ago by casamayou

updated file

comment:18 Changed 5 years ago by casamayou

The files *_fixed.py are outdated, because they were tested under sage 7.4.1.

The following files have been updated and tested under sage 7.4.2 and are the only ones to consider:

calculus.py graphique_1.py graphique_2.py graphique_3.py sol_calculus.py sol_graphiques_1.py sol_graphiques_2.py

comment:19 follow-up: Changed 5 years ago by casamayou

One more question.

Should I create a new file trac_10983.patch with a "hg commit" ?

(Sorry... I'm a newbie)

comment:20 in reply to: ↑ 19 Changed 5 years ago by dimpase

Replying to casamayou:

One more question.

Should I create a new file trac_10983.patch with a "hg commit" ?

(Sorry... I'm a newbie)

hg commit is what you might sometimes do after applying a patch. It certainly does not create patches. You can create patches using hg, e.g. by hg import. But you don't have to. I'll look at your updates later today.

comment:21 Changed 4 years ago by mmezzarobba

  • Authors changed from Alexandre Casamayou to Alexandre Casamayou, Marc Mezzarobba
  • Description modified (diff)
  • Status changed from needs_work to needs_review

Here are updated tests that correspond to the current version of the book. (The division of chapters changed a little bit, but the tests I'm including here roughly correspond to those posted by Alexandre.)

On my machine, using Sage-5.9β0:

Running doctests with ID 2013-03-22-21-36-07-c7afe8cf.
Doctesting 3 files.
sage -t calculus_doctest.py
    [246 tests, 6.9 s]
sage -t graphique_doctest.py
    [167 tests, 26.8 s]
sage -t premierspas_doctest.py
    [49 tests, 1.5 s]
----------------------------------------------------------------------
All tests passed!
----------------------------------------------------------------------
Total time for all tests: 35.4 seconds
    cpu time: 30.6 seconds
    cumulative wall time: 35.2 seconds

comment:22 Changed 4 years ago by ncohen

  • Status changed from needs_review to needs_work

Most of the doctests break on sage 5.1.beta3. Most output are ordered differently, so there's nothing to be worried about either.

----------------------------------------------------------------------
sage -t --long calculus_doctest.py  # 12 doctests failed
sage -t --long graphique_doctest.py  # 1 doctest failed
----------------------------------------------------------------------

Nathann

Apply trac_10983_doctests_from_french_book.patch

comment:23 Changed 4 years ago by ncohen

Ahem. 5.11.beta3 :-P

Nathann

comment:24 Changed 4 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

Changed 4 years ago by zimmerma

Combined patch

comment:25 Changed 4 years ago by zimmerma

  • Authors changed from Alexandre Casamayou, Marc Mezzarobba to Alexandre Casamayou, Marc Mezzarobba, Paul Zimmermann
  • Status changed from needs_work to needs_review

I've made a new patch with a different ordering in the outputs, due to a recent change in Sage (I can't find the corresponding ticket). Unfortunately the book output now slightly differs with what we get in Sage. This patch works fine with Sage 5.11.

Paul

comment:26 Changed 4 years ago by jdemeyer

The ticket which changed the ordering is #9880.

comment:27 Changed 4 years ago by chapoton

  • Status changed from needs_review to needs_work

indentation should be 4 spaces rather than 2

comment:28 Changed 4 years ago by chapoton

for patchbot:

apply trac_10983_doctests_from_french_book.patch​

comment:29 Changed 4 years ago by chapoton

apply trac_10983_doctests_from_french_book.patch

Changed 4 years ago by chapoton

comment:30 follow-up: Changed 4 years ago by chapoton

I have made a new patch with correct indentations.

Unfortunately, there are now 2 doctest errors with simplify_full !!!

for the patchbots:

apply trac_10983_doctests_from_french_book_v2.patch

comment:31 Changed 4 years ago by chapoton

  • Description modified (diff)

comment:32 in reply to: ↑ 30 Changed 4 years ago by dimpase

Replying to chapoton:

I have made a new patch with correct indentations.

Unfortunately, there are now 2 doctest errors with simplify_full !!!

How about the following changes: (I suppose this also means minor corrections to the main text, which I can't do)

diff --git a/sage/tests/french_book/calculus_doctest.py b/sage/tests/french_book/calculus_doctest.py
--- a/sage/tests/french_book/calculus_doctest.py
+++ b/sage/tests/french_book/calculus_doctest.py
@@ -643,7 +643,7 @@
     sage: assume(8*n+1>0)
     sage: u(n) = integrate((4*sqrt(2)-8*t^3-4*sqrt(2)*t^4\
     ....:                  -8*t^5) * t^(8*n), t, 0, 1/sqrt(2))
-    sage: (u(n)-v(n)).simplify_full()
+    sage: (u(n)-v(n)).simplify_exp()
     0
 
 Sage example in ./sol/calculus.tex, line 258::
@@ -652,8 +652,8 @@
     sage: J = integrate((4*sqrt(2)-8*t^3 \
     ....:      - 4*sqrt(2)*t^4-8*t^5)\
     ....:      / (1-t^8), t, 0, 1/sqrt(2))
-    sage: J.simplify_full()
-    pi + 2*log(sqrt(2) + 1) + 2*log(sqrt(2) - 1)
+    sage: J.simplify_log().simplify_full()
+    pi
 
 Sage example in ./sol/calculus.tex, line 272::
 

comment:33 follow-up: Changed 3 years ago by zimmerma

the second doctest failure looks like a regression in Sage, since the expression 2*e is not simplified similarly to e:

sage: e=log(1/2*sqrt(2)*(sqrt(2) + 1)) + log(1/2*sqrt(2)*(sqrt(2) - 1))
sage: e.simplify_full()
-log(2)
sage: (2*e).simplify_full()
2*log(1/2*sqrt(2)*(sqrt(2) + 1)) + 2*log(1/2*sqrt(2)*(sqrt(2) - 1))

Should I open a ticket for that?

Paul

comment:34 in reply to: ↑ 33 Changed 3 years ago by dimpase

Replying to zimmerma:

the second doctest failure looks like a regression in Sage, since the expression 2*e is not simplified similarly to e:

sage: (2*e).simplify_full()
2*log(1/2*sqrt(2)*(sqrt(2) + 1)) + 2*log(1/2*sqrt(2)*(sqrt(2) - 1))

Should I open a ticket for that?

Just as in the patch proposed, you may chain simplifies, getting

sage: (2*e).simplify_log().simplify_full()
-log(4)

Not sure it's a Maxima regression, or a proper Sage regression. Opening a trac ticket won't hurt, I suppose.

comment:35 Changed 3 years ago by zimmerma

Opening a trac ticket won't hurt, I suppose.

done, cf #15362.

Paul

comment:36 Changed 3 years ago by zimmerma

a simpler fix is to replace simplify_full by simplify_radical in the two failing doctests. See #15362. Since that change also works before #12737 (merged in Sage 5.12) I will do the change in our book too.

Paul

Changed 3 years ago by zimmerma

comment:37 Changed 3 years ago by zimmerma

  • Description modified (diff)
  • Status changed from needs_work to needs_review

with the new patch, the doctests pass again. Ready for review.

Paul

comment:38 Changed 3 years ago by dimpase

  • Status changed from needs_review to positive_review

looks good to me.

comment:39 Changed 3 years ago by jdemeyer

  • Merged in set to sage-5.13.beta3
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.