Opened 11 years ago

Closed 9 years ago

Last modified 6 years ago

#8791 closed enhancement (fixed)

improve doctest coverage of libs/mpmath/ext_main.pyx

Reported by: mvngu Owned by: mvngu
Priority: major Milestone: sage-5.0
Component: documentation Keywords:
Cc: fredrik.johansson, schilly Merged in: sage-5.0.beta11
Authors: Fredrik Johansson, Harald Schilly, David Loeffler Reviewers: David Loeffler, Jeroen Demeyer
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by chapoton)

As the subject says. As of Sage 4.4, the doctest coverage of sage/libs/mpmath/ext_main.pyx is:

[mvngu@sage mpmath]$ sage -coverage ext_main.pyx 
----------------------------------------------------------------------
ext_main.pyx
ERROR: Please add a `TestSuite(s).run()` doctest.
SCORE ext_main.pyx: 0% (0 of 102)

Missing documentation:
 * __cinit__(ctx):
 * default(ctx):
 * _get_prec(ctx):
 * _set_prec(ctx, prec):
 * _set_dps(ctx, n):
 * _get_dps(ctx):
 * _get_prec_rounding(ctx):
 * mpf make_mpf(ctx, tuple v):
 * mpc make_mpc(ctx, tuple v):
 * _convert_param(ctx, x):
 * _wrap_libmp_function(ctx, mpf_f, mpc_f=None, mpi_f=None, doc="<no doc>"):
 * _wrap_specfun(cls, name, f, wrap):
 * __init__(self, mpf_f, mpc_f=None, mpi_f=None, doc="<no doc>"):
 * __call__(self, x, **kwargs):
 * __init__(self, name, f):
 * __call__(self, *args, **kwargs):
 * __richcmp__(self, other, int op):
 * __add__(self, other):
 * __sub__(self, other):
 * __mul__(self, other):
 * __div__(self, other):
 * __mod__(self, other):
 * __pow__(self, other, mod):
 * ae(s, t, rel_eps=None, abs_eps=None):
 * __hash__(self):
 * __repr__(self):
 * __str__(self):
 * real(self):
 * imag(self):
 * conjugate(self):
 * man(self):
 * exp(self):
 * bc(self):
 * __int__(self):
 * __long__(self):
 * __float__(self):
 * __complex__(self):
 * to_fixed(self, prec):
 * __getstate__(self):
 * __setstate__(self, val):
 * __init__(self, x=0, **kwargs):
 * __reduce__(self):
 * _get_mpf(self):
 * _set_mpf(self, v):
 * __nonzero__(self):
 * __hash__(self):
 * real(self):
 * imag(self):
 * conjugate(self):
 * man(self):
 * exp(self):
 * bc(self):
 * to_fixed(self, long prec):
 * __int__(self):
 * __float__(self):
 * __getstate__(self):
 * __setstate__(self, val):
 * __cinit__(self):
 * __neg__(s):
 * __pos__(s):
 * __abs__(s):
 * sqrt(s):
 * __richcmp__(self, other, int op):
 * __init__(self, func, name, docname=''):
 * __call__(self, prec=None, dps=None, rounding=None):
 * _mpf_(self):
 * __repr__(self):
 * __nonzero__(self):
 * __neg__(self):
 * __pos__(self):
 * __abs__(self):
 * sqrt(self):
 * to_fixed(self, prec):
 * __getstate__(self):
 * __setstate__(self, val):
 * __hash__(self):
 * __richcmp__(self, other, int op):
 * __init__(self, real=0, imag=0):
 * __cinit__(self):
 * __reduce__(self):
 * __setstate__(self, val):
 * __repr__(self):
 * __str__(s):
 * __nonzero__(self):
 * __complex__(self):
 * _get_mpc(self):
 * _set_mpc(self, tuple v):
 * real(self):
 * imag(self):
 * __hash__(self):
 * __neg__(s):
 * conjugate(s):
 * __pos__(s):
 * __abs__(s):
 * __richcmp__(self, other, int op):

Missing doctests:
 * convert(ctx, x, strings=True):
 * isnan(ctx, x):
 * isinf(ctx, x):
 * isint(ctx, x):
 * fsum(ctx, terms, bint absolute=False, bint squared=False):
 * fdot(ctx, A, B=None):
 * mag(ctx, x):

Attachments (2)

mpmath_doctests.patch (45.9 KB) - added by was 9 years ago.
review this, not Harald's
trac_8791-fix.patch (877 bytes) - added by davidloeffler 9 years ago.
apply over previous patch

Download all attachments as: .zip

Change History (19)

comment:1 Changed 11 years ago by fredrik.johansson

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.:

sage: import mpmath
sage: mpmath.mpf(1).__add__(1)
mpf('2.0')

But I can't say when I'll have time to do this.

comment:2 Changed 11 years ago by schilly

While I started to write some tests, I stumbled about to_fixed(). What does it do? Would be

sage: import mpmath
sage: mpmath.mpf('88494.93').to_fixed(6)
5663675

be a doctest or a total abuse?!

comment:3 Changed 11 years ago by fredrik.johansson

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):

sage: mpmath.mpf(1.25).to_fixed(10)
1280
sage: int(1.25 * 2^10)
1280

comment:4 Changed 11 years ago by schilly

ok, i've done some more but i don't know how to really test init, cinit, call and some others.

comment:5 Changed 11 years ago by schilly

I have another question, is this a bug:

sage: mpmath.mpf(1) < 3
True
sage: 1 < mpmath.mpf(3)
False
sage: 4 == mpmath.mpf(4)
False

comment:6 Changed 11 years ago by schilly

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 Changed 11 years ago by fredrik.johansson

Thanks! Very nice work.

The comparisons are definitely a bug. I created #8924 for this.

comment:8 Changed 9 years ago by fredrik.johansson

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).

Changed 9 years ago by was

review this, not Harald's

comment:9 Changed 9 years ago by was

  • Status changed from new to needs_review

comment:10 Changed 9 years ago by kcrisman

  • Authors set to Fredrik Johansson, Harald Schilly

comment:11 Changed 9 years ago by davidloeffler

Apply mpmath_doctests.patch

(for patchbot)

comment:12 Changed 9 years ago by davidloeffler

  • Reviewers set to David Loeffler
  • Status changed from needs_review to positive_review

Wow -- this is a monumental piece of doctesting! It looks great. This will get us a long way towards the 90% doctest coverage goal.

comment:13 Changed 9 years ago by jdemeyer

  • Status changed from positive_review to needs_work

This obviously fails on 32-bit systems:

sage -t  --long -force_lib devel/sage/sage/libs/mpmath/ext_main.pyx
**********************************************************************
File "/export/home/buildbot/build/sage/hawk-1/hawk_full/build/sage-5.0.beta10/devel/sage-main/sage/libs/mpmath/ext_main.pyx", line 2023:
    sage: int(7.25 * 2**30)
Expected:
    7784628224
Got:
    7784628224L
**********************************************************************

Changed 9 years ago by davidloeffler

apply over previous patch

comment:14 Changed 9 years ago by davidloeffler

  • Authors changed from Fredrik Johansson, Harald Schilly to Fredrik Johansson, Harald Schilly, David Loeffler
  • Status changed from needs_work to needs_review

Apply mpmath_doctests.patch, trac_8791-fix.patch

This is a one-line fix -- anyone willing to quickly sign off on it?

comment:15 Changed 9 years ago by jdemeyer

  • Reviewers changed from David Loeffler to David Loeffler, Jeroen Demeyer
  • Status changed from needs_review to positive_review

Good!

comment:16 Changed 9 years ago by jdemeyer

  • Merged in set to sage-5.0.beta11
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:17 Changed 6 years ago by chapoton

  • Description modified (diff)
Note: See TracTickets for help on using tickets.