fix that _hnf_mod segfaults sage completely

[was]

teragon:algebras wstein$ sage
----------------------------------------------------------------------
| Sage Version 3.4, Release Date: 2009-03-11                         |
| Type notebook() for the GUI, and license() for information.        |
----------------------------------------------------------------------
sage: a = random_matrix(ZZ,16,4)
sage: a._hnf_mod(100)


------------------------------------------------------------
Unhandled SIGFPE: An unhandled floating point exception occured in SAGE.
This probably occured because a *compiled* component
of SAGE has a bug in it (typically accessing invalid memory)
or is not properly wrapped with _sig_on, _sig_off.
You might want to run SAGE under gdb with 'sage -gdb' to debug this.
SAGE will now terminate (sorry).
------------------------------------------------------------

[was]

(the problem is that the input matrix isn't square...)

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​8c43ba0

Check matrix is square

[gh-black-stones]

Raised a ValueError? if the matrix is not square, and made a doctest out of the sample code in the description.

[slelievre]

Shouldn't the added doctest block start with a TEST:: block header?

[gh-black-stones]

Replying to slelievre:

Shouldn't the added doctest block start with a TEST:: block header?

I believe that a line ending in :: signifies the code following it is a doctest, it doesn't necessarily need to be TEST::. There are a few exceptions, like how MATH:: indicates latex, but there are many doctests that say something like "This fixes trac X::", followed by a doctest.

[slelievre]

Although the code does get tested even without a TEST header, my understanding of the ​documentation strings section of Sage's ​coding basics is that testable examples in docstrings are usually placed inside "TEST" or "TESTS" or "EXAMPLE" or "EXAMPLES" blocks, which can go either

TESTS::

    sage: 1 + 1
    2
    sage: 1 + 2
    3

or

TESTS:

We test that one plus one is two::

    sage: 1 + 1
    2

We test that one plus two is three::

    sage: 1 + 2
    3

(or similarly for TEST, EXAMPLE, EXAMPLES).

In addition, TEST and TESTS sections being meant more for developers than users, they get removed when building the documentation, unless one sets a dedicate environment variable:

SAGE_DOCBUILD_OPTS="--include-tests-blocks"

to keep them in.

Are there any doctests in docstrings elsewhere in the Sage sources that are not introduced by an EXAMPLE, EXAMPLES, TEST or TESTS header?

[jdemeyer]

I agree with Samual that there should have been a TESTS: block. I missed that during review. It's not a big problem though...

[gh-black-stones]

I see, I didn't realize that the TESTS:: header indicated a big block for all the tests, sorry for leaving that out. That being said, I did a quick search and found that there are some (very few) files that don't follow this...

• set_calculus_method() of sage-master/src/sage/manifolds/manifold.py
• from_tamari_sorting_tuple() of sage-master/src/sage/combinat/binary_tree.py ("EXEMPLES" instead of "EXAMPLES")
• init() of GenericGraphQuery? in sage-master/src/sage/graphs/graph_database.py
• this file

My "quick search" was a bad Python script that has most likely left out other mistakes. Furthermore, some docstrings use "Examples" or "Tests" instead of the all uppercase version, and others have "Examples:" instead of "Examples::". I wasn't sure if they mattered so I didn't include them (also there were a lot of them).

This ticket is already closed, so let's not reopen it. Do you think it would be worth it to make a new ticket to search for and fix all these docstring mistakes?

[slelievre]

A new ticket dedicated to fixing all missing or non-uppercase EXAMPLES and TESTS block headers would be nice.

[jdemeyer]

If you want to fix those mistakes, great! Reviewing that should be trivial.