-
-
Notifications
You must be signed in to change notification settings - Fork 482
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
improve doctest coverage of libs/mpmath/ext_main.pyx #8791
Comments
comment:1
Argh. Who wrote this code? The actual code coverage, indirectly through mpmath's unit tests (which however aren't run automatically by Sage) is close to 99%, but writing Sage doctests slipped my mind entirely. Some of these functions have doctests, but they're added at runtime when importing mpmath. I'm considering whether statically duplicating them is a good idea, or whether it's better to just add static placeholders. Most of these are probably trivial to write, e.g.:
But I can't say when I'll have time to do this. |
comment:2
While I started to write some tests, I stumbled about
be a doctest or a total abuse?! |
comment:3
Thanks a lot, Harald! to_fixed converts a number to binary fixed-point format. Perhaps for clarity something like the following could be used (1.25 is for exactness):
|
comment:4
ok, i've done some more but i don't know how to really test init, cinit, call and some others. |
comment:5
I have another question, is this a bug:
|
comment:6
I don't know how to test the remaining ones, but shouldn't be that hard to finish. Only 16 are still to go. |
comment:7
Thanks! Very nice work. The comparisons are definitely a bug. I created #8924 for this. |
comment:8
Here is a new patch with almost full coverage: http://sage.math.washington.edu/home/fredrik/mpmath_doctests.patch (I did not use Harald's old patch as a base, though I think that one will mostly still be working. Perhaps they could be combined.) The functions still missing doctests are pickling support for classes that shouldn't be pickable (not sure why they are there). |
review this, not Harald's |
comment:9
Attachment: mpmath_doctests.patch.gz |
Author: Fredrik Johansson, Harald Schilly |
comment:11
Apply mpmath_doctests.patch (for patchbot) |
comment:12
Wow -- this is a monumental piece of doctesting! It looks great. This will get us a long way towards the 90% doctest coverage goal. |
Reviewer: David Loeffler |
comment:13
This obviously fails on 32-bit systems:
|
apply over previous patch |
Changed author from Fredrik Johansson, Harald Schilly to Fredrik Johansson, Harald Schilly, David Loeffler |
comment:14
Attachment: trac_8791-fix.patch.gz Apply mpmath_doctests.patch, trac_8791-fix.patch This is a one-line fix -- anyone willing to quickly sign off on it? |
comment:15
Good! |
Changed reviewer from David Loeffler to David Loeffler, Jeroen Demeyer |
Merged: sage-5.0.beta11 |
As the subject says. As of Sage 4.4, the doctest coverage of
sage/libs/mpmath/ext_main.pyx
is:CC: @fredrik-johansson @haraldschilly
Component: documentation
Author: Fredrik Johansson, Harald Schilly, David Loeffler
Reviewer: David Loeffler, Jeroen Demeyer
Merged: sage-5.0.beta11
Issue created by migration from https://trac.sagemath.org/ticket/8791
The text was updated successfully, but these errors were encountered: