#9395 closed enhancement (fixed)
new doctest for french book about Sage
Reported by: | zimmerma | Owned by: | mvngu |
---|---|---|---|
Priority: | major | Milestone: | sage-4.6 |
Component: | doctest coverage | Keywords: | |
Cc: | was, cremona, ylchapuy | Merged in: | sage-4.6.alpha1 |
Authors: | Paul Zimmermann, Yann Laigle-Chapuy | Reviewers: | Yann Laigle-Chapuy, Paul Zimmermann |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
The attached patch includes a new doctest for a book (in french) some people are writing about Sage (see the README file for the list of authors).
This book will be available under a CC-by-sa license.
This patch contains only the doctests for one chapter (about number theory). Some more doctests will follow, one per chapter, but we already submit that one to see if some things need to be fixed.
This doctest was tested successfully with Sage 4.4.2 under Fedora 12.
Attachments (6)
Change History (25)
comment:1 Changed 11 years ago by
- Cc was cremona added
comment:2 Changed 11 years ago by
- Status changed from new to needs_review
comment:3 follow-up: ↓ 4 Changed 11 years ago by
- Status changed from needs_review to needs_work
You don't need to put your doctests inside a function. I think it's much simpler to put your doctests inside a docstring. See the files under tests/ in the Sage library for examples. You should also consider giving your book's directory name a more descriptive name. Something like "number_theory_zimmermann", not just "sagebook". Or do you envision the directory "sagebook" to contain doctests of books that leverage the Sage doctesting framework? In that case, see this script for a proof of concept for automatic extraction of Sage code and doctesting the extracted code. That script has been tested on this Sage book.
comment:4 in reply to: ↑ 3 Changed 11 years ago by
- Status changed from needs_work to needs_review
comment:5 Changed 11 years ago by
The version 1.0 of the book has now appeared, and is available from http://www.loria.fr/~zimmerma/sagebook.html. Any feedback is most welcome!
comment:6 follow-ups: ↓ 7 ↓ 9 Changed 11 years ago by
- Cc ylchapuy added
Yann, please could you review this? What you have to do (William please correct me if needed):
1) [optional] check the content of the numbertheory.py file matches the corresponding chapter of the book. I write "optional" since this should be our (the authors of the book) responsability.
2) check the new doctests pass with the latest Sage version (we used 4.4.4)
Paul
comment:7 in reply to: ↑ 6 ; follow-up: ↓ 8 Changed 11 years ago by
Replying to zimmerma:
Yann, please could you review this? What you have to do (William please correct me if needed):
Sounds good to me.
I've been advertising your book to all the French people I meet in Paris, e.g., at Euroscipy, and also to Bernandi at Jussieu today (he's one of the original PARI guys).
comment:8 in reply to: ↑ 7 Changed 11 years ago by
Replying to was:
I've been advertising your book to all the French people I meet in Paris, e.g., at Euroscipy, and also to Bernandi at Jussieu today (he's one of the original PARI guys).
thanks, some people of my lab who were at Euroscipy indeed told me today that they heard of our book there! Paul
comment:9 in reply to: ↑ 6 Changed 11 years ago by
- Status changed from needs_review to needs_info
Replying to zimmerma:
Yann, please could you review this? What you have to do (William please correct me if needed):
1) [optional] check the content of the numbertheory.py file matches the corresponding chapter of the book. I write "optional" since this should be our (the authors of the book) responsability.
2) check the new doctests pass with the latest Sage version (we used 4.4.4)
Paul
Sorry for the delay. All tests passed with Sage 4.5.2.
Regarding the timeit
issue, you could change
sage: %timeit (a^e) # not tested
for
sage: timeit('a^e') # random
(and also # long
for some of them).
I put it to need info
for now, but feel free either to change this and ask me for a review (I'll be faster this time) or don't change anything and give a positive review.
Yann
comment:10 Changed 11 years ago by
- Status changed from needs_info to needs_review
thank you Yann for your comments. I attach a new patch taking them into account. I left the
last time r=...
as not tested
since I do not know how to make it work, without
changing time
into timeit
, which would be not coherent with the book.
Paul
Changed 11 years ago by
comment:11 Changed 11 years ago by
- Reviewers set to Yann Laigle-Chapuy
Everything is still ok for me. I added 'long time' to the longest tests, this reduces the time for normal testing to 12 seconds on my computer compared to 67 for the long version.
Paul, if you agree with my reviewer's patch, you can give this ticket a positive review.
Yann
comment:12 Changed 11 years ago by
- Reviewers changed from Yann Laigle-Chapuy to Yann Laigle-Chapuy, Paul Zimmermann
- Status changed from needs_review to positive_review
Paul, if you agree with my reviewer's patch, you can give this ticket a positive review.
yes I agree. Paul
comment:13 follow-up: ↓ 14 Changed 11 years ago by
- Status changed from positive_review to needs_work
Could someone please update both patches with more descriptive commit strings (and change the status back to "positive review")?
comment:14 in reply to: ↑ 13 Changed 11 years ago by
Replying to mpatel:
Could someone please update both patches with more descriptive commit strings (and change the status back to "positive review")?
done with a combined patch (apply only that one). Is that what you wanted?
Paul
comment:15 Changed 11 years ago by
I apologize for not being clearer. The first line of the commit string for each patch to be merged should start with the ticket number and contain a short description of what the patch does. This line should be < 80 characters in length.
For example: #9395: Number theory doctests for French book about Sage
and #9395: Reviewer patch: tag longest tests as long
.
The reason for these policies is so that hg log
and hg log filename.py
tell us what a changeset did and which Sage trac ticket we can consult for background.
Of course, extra lines are welcome and may help to explain details to a reviewer or someone who uses hg log -p
.
Could you update the just first line of the combined patch?
comment:16 Changed 11 years ago by
- Status changed from needs_work to positive_review
Could you update the just first line of the combined patch?
done. Paul
comment:17 Changed 11 years ago by
- Merged in set to sage-4.6.alpha1
- Resolution set to fixed
- Status changed from positive_review to closed
comment:18 Changed 11 years ago by
I've added a release manager's patch that ensures the files added here are included in a new Sage source distribution.
comment:19 Changed 11 years ago by
Ticket #9951, about a missing __init__.py
file, follows up on this ticket.
I found no way so that the
%timeit ...
ortime ...
lines are tested, thus I've added# not tested
. If somebody knows how to do so that in%timeit a = b + c
at least the instructiona = b + c
is performed, please tell me.