On Nov 28, 2008, at 10:21 AM, mabshoff wrote: > On Nov 28, 7:48 am, "William Stein" <[EMAIL PROTECTED]> wrote: >> On Fri, Nov 28, 2008 at 4:49 AM, Simon King <[EMAIL PROTECTED] >> jena.de> wrote: >> >>> Dear developers, > > Hi, > >>> I have a couple of basic methods (such as "save part of the >>> attributes >>> in some file" or "put some data on a to-do-list used by another >>> method"). I wonder if their doc strings need examples, for the >>> following reasons: >> >>> 1. I think they are of internal use only, and a normal user doesn't >>> even need to know about their existence. So, the user doesn't >>> need to >>> see an example for the basic method. > > But developers will. And users who hit a bug can start reading and > debugging themselves. > >>> 2. The methods are heavily used by high-level methods, so the basic >>> methods are in fact tested by the doc tests of the high-level >>> methods. >>> But since they *are* tested, there is no need for a separate test of >>> the basic methods. > > The doctests provide examples on how to use the method and that is > invaluable. See also the next argument. > > On top of that: We are usually hunting for memory problems by > valgrinding the examples. In the future we will also use muppy to look > or leaks in the python heap which are very hard to find via valgrind. > One problem right now is for example that a very high level modular > forms computation leaks. We have an idea what leaks thanks to muppy, > but buy sheer accident we then found a case where _echelonize() leaks. > That is an internal function :) - and once we run muppy over every > docstring in the Sage library we will find all such leaks. And having > *every* function including all the low level and internal stuff > provide doctest examples is *absolutely* essential here. The same > applies to the already existing -timeit doctest mode. > >>> 3. A developer might care about the basic methods. But since the >>> basic >>> methods just comprise 5-10 lines, an example might not be needed. >>> And >>> I have comments in the code telling what the basic method does. > > When one does doctesting on a new platform one often runs into strange > bugs. The more low level the doctest failure the easier it is for > someone less familiar with the code to make a precise bug report. This > is not just theoretical: On Solaris/Sparc we seem to have trouble when > passing commands to Singular using a file. The doctest failures we saw > caused by this bug are very strange and it let me to believe that > pexpect was at fault. Had we had a doctest that tested the passing > command by file this would have likely been much quicker to determine > the problem here. > >>> So, I tend to have the following doc-string for a basic 'export' >>> method used in a high-level method 'blah' >>> "Of internal use only. Export data to disk. Implicit test in >>> 'blah'" >> >>> What is the official opinion on that matter? >> >> I would reject any patch with functions like that which aren't >> doctested. > > I would also consider this doctest policy absolute with one exception: > the deallocation of certain structures like the gray tables in m4ri > cannot be tested without segfaulting Sage. So that one gets a pass, > but there are maybe three or four such cases in all of Sage. > > So all of the above leads me to believe that we must have a 100% > doctest coverage policy. As soon as we start giving out free passes > people will start to argue that their code should be exempted due to > BLAH BLAH BLAH and arguing with various people will take longer than > writing the damn doctest in the first place. Doctests are what enables > us to develop at this pace and find bugs, i.e. I consider this one a > huge advantage of Sage and the 100% policy is good for Sage. Just > imaging how many bugs and leaks will be found by doctesting for > example the padics code to 100%. That code got in since the consensus > at SD 7 was that leaving it out would have been painful, but if I were > confronted with the same problem today I would actually vote -1 on > merging that code since a year and a couple months after SD 7 when the > code was merged it is still not properly doctested.
I agree with all of the above. The *only* functions that I think don't need a doctest are abstract base class methods whose entire body is "raise NotImplementedError," and even on this there are lots of people who would disagree with me. - Robert --~--~---------~--~----~------------~-------~--~----~ To post to this group, send email to [email protected] To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/sage-devel URLs: http://www.sagemath.org -~----------~----~----~----~------~----~------~--~---
