Doctest for a minor bugfix?

[Kwankyu]

Hi,

I wrote a patch to remove the redundant minus sign in the last output
in the following:

sage: from sage.rings.polynomial.multi_polynomial_ring import
MPolynomialRing_polydict
sage: R.<x,y>=MPolynomialRing_polydict(GF(2),2,order='lex')
sage: R
Multivariate Polynomial Ring in x, y over Finite Field of size 2
sage: f=x+y
sage: f.lt()
-x

And a reviewer of the patch requested me to add a doctest in the patch
to show that the bug is fixed. At this point, I wonder if it is a good
idea to put such a doctest in the Examples section of the document
string. I think the Examples section is for the user to learn how to
use the given function. Then an example just to check a simple bugfix
is not very useful for this purpose.

Do I need to put a doctest anyway?


Kwankyu

[Minh Nguyen]

Hi Kwankyu,

On Mon, Oct 12, 2009 at 3:23 PM, Kwankyu <ekwa...@gmail.com> wrote:

<SNIP>

> Do I need to put a doctest anyway?

Yes. Put the doctest under a section called "TESTS:" and make sure to
reference the relevant ticket number. Doctests under the section
"TESTS:" are meant to demonstrate that a bug has been fixed. They
should also demonstrate the exceptions that are caught by the relevant
method/function/class. See, for example, the following page from the
reference manual:

http://www.sagemath.org/doc/reference/sage/rings/polynomial/multi_polynomial_ring.html#sage.rings.polynomial.multi_polynomial_ring.MPolynomialRing_polydict_domain.monomial_divides

--
Regards
Minh Van Nguyen

[Kwankyu]

Thanks. I forgot the Tests section.

Kwankyu

[Martin Albrecht]

On Monday 12 October 2009, Kwankyu wrote:
> Hi,
>
> I wrote a patch to remove the redundant minus sign in the last output
> in the following:
>
> sage: from sage.rings.polynomial.multi_polynomial_ring import
> MPolynomialRing_polydict
> sage: R.<x,y>=MPolynomialRing_polydict(GF(2),2,order='lex')
> sage: R
> Multivariate Polynomial Ring in x, y over Finite Field of size 2
> sage: f=x+y
> sage: f.lt()
> -x

Alex Ghitza wrote a patch to fix printing of multivariate polynomials in
general

http://trac.sagemath.org/sage_trac/ticket/6551

which might contain your fix. However, it needs some work before it can go in.

Martin

--
name: Martin Albrecht
_pgp: http://pgp.mit.edu:11371/pks/lookup?op=get&search=0x8EF0DC99
_otr: 47F43D1A 5D68C36F 468BAEBA 640E8856 D7951CCF
_www: http://www.informatik.uni-bremen.de/~malb
_jab: martinr...@jabber.ccc.de

[Kwankyu]

Hi Minh,

I still have a problem on this issue.

1. "TESTS:" section is not documented in the developer manual as of
Sage 4.1.1
2. Examples in "TESTS:" section are still included in the Sage
reference manual. If they are intended for test purpose only, then
they should not be included in the reference manual, which is for the
users.

Do I misunderstand something?


Kwankyu

[William Stein]

No, we just have not got around to actually implementing 2 yet. Want to help?

William

[Minh Nguyen]

Hi Kwankyu,

On Tue, Oct 13, 2009 at 10:45 AM, Kwankyu <ekwa...@gmail.com> wrote:

<SNIP>

> Do I misunderstand something?

The section "TESTS:" is not documented in the Developers' Guide
because no one so far has written a patch to update the Guide. Patches
welcome :-) However, note that the section "Reviewing Patches" [1]
has this dot point:

{{{
In particular, is there a doctest illustrating that the bug has been
fixed? If a function used to give the wrong answer and this patch
fixes that, then if possible, it should include a doctest illustrating
its new success.
}}}

[1] http://www.sagemath.org/doc/developer/trac.html#reviewing-patches

[John H Palmieri]

On Oct 12, 5:08 pm, William Stein <wst...@gmail.com> wrote:

I would point out that in the code, some examples in the TESTS section
can be quite helpful to users. So if someone wants to implement 2,
they had better check whether each such example is worth including in
the reference manual (and so should be moved to the EXAMPLES block) or
not. Including too many examples is better than including too few.

John

[Kwankyu]

On Oct 13, 9:08 am, William Stein <wst...@gmail.com> wrote:

> On Mon, Oct 12, 2009 at 4:45 PM, Kwankyu <ekwan...@gmail.com> wrote:
>
> > Hi Minh,
>
> > I still have a problem on this issue.
>
> > 1. "TESTS:" section is not documented in the developer manual as of
> > Sage 4.1.1
> > 2. Examples in "TESTS:" section are still included in the Sage
> > reference manual. If they are intended for test purpose only, then
> > they should not be included in the reference manual, which is for the
> > users.
>
> > Do I misunderstand something?
>
> No, we just have not got around to actually implementing 2 yet.  

I see.

> Want to help?

Not now.


Kwankyu

[William Stein]

Maybe we should not bother implementing 2? Nobody has pushed to do
it, and your remark above just made it a difficult task. It will also
be potentially be difficult to maintain, since it means potentially
tricky and bug-prone patches to how we use Sphinx *and* IPython *and*
the Notebook.

-- William

[Jason Grout]

Perhaps collapsible headings (so "TESTS" was automatically collapsed)
would answer the need to not inundate the user with a million tests for
each case of a function, while also letting the user access every bit of
documentation if they so desire.

Jason

--
Jason Grout

[Nick Alexander]

> Perhaps collapsible headings (so "TESTS" was automatically collapsed)

+1

Nick

[William Stein]

On Mon, Oct 12, 2009 at 8:14 PM, Nick Alexander <ncale...@gmail.com> wrote:
>
>> Perhaps collapsible headings (so "TESTS" was automatically collapsed)
>
> +1
>

To summarize all these great ideas:

1. In the notebook, we do some sort of post processing (?) to the
output of Sphinx to add in Javascript to make a collapsible TESTS
section

2. The TESTS section is never shown in Ipython introspection? Or
always shown?

3. Somebody goes through all TESTS sections in all of Sage and
makes a judgment calls to move some things out of TESTS, because
evidently the author of some tests in TESTS didn't understand what
"TESTS" means.

4. What happens in Emacs sage-mode?

Is 3 not needed because of 1 and 2?

-- William

[Robert Bradshaw]

On Oct 12, 2009, at 8:27 PM, William Stein wrote:

>
> On Mon, Oct 12, 2009 at 8:14 PM, Nick Alexander
> <ncale...@gmail.com> wrote:
>>
>>> Perhaps collapsible headings (so "TESTS" was automatically
>>> collapsed)
>>
>> +1
>>
>
> To summarize all these great ideas:
>
> 1. In the notebook, we do some sort of post processing (?) to the
> output of Sphinx to add in Javascript to make a collapsible TESTS
> section

+1

> 2. The TESTS section is never shown in Ipython introspection? Or
> always shown?

ipython introspection is usually handled by something like less, so
because they're at the bottom they never get in the way until the user
scrolls all the way down.

> 3. Somebody goes through all TESTS sections in all of Sage and
> makes a judgment calls to move some things out of TESTS, because
> evidently the author of some tests in TESTS didn't understand what
> "TESTS" means.
>
> 4. What happens in Emacs sage-mode?
>
>
> Is 3 not needed because of 1 and 2?

I think so.

I only put things in TESTS if it's clearly uninteresting to the end
user, but something we want to guard against. If someone wants to
volunteer to do this, that would be great, but I think there's higher
priorities (e.g. adding documentation and examples to functions that
don't have any/many). Of course, if I see something amiss when I'm
editing relevant code I'll just fix it there.

- Robert