doctest coverage 100% ntl library

[lftabera]

Add documentation, tests and/or examples to every method in the ntl library wrappers.

Issues I have found:

• ntl calls lack in several places sig_on/sig_off, this can lead to Sage segfaults. I am adding test for the segfaults I can reproduce with Sage 8.3 (division by zero, changing a polynomial by a negative index).

• int(ntl.GF2X([])) raises a ValueError, it should return 0.

• matrices over GF2, GF2E, IsIdent returns true if a parameter n is negative and the matrix is the identity matrix.

Component: doctest coverage

Keywords: days94

Author: Luis Felipe Tabera Alonso

Branch/Commit: u/lftabera/ticket/25717 @ 0237bfa

Issue created by migration from https://trac.sagemath.org/ticket/25717

[lftabera]

Description changed:

--- 
+++ 
@@ -1 +1,7 @@
 Add documentation, tests and/or examples to every method in the ntl library wrappers.
+
+Issues I have found:
+
+- ntl calls lack in several places sig_on/sig_off, this can lead to Sage segfaults. I am adding test for the segfaults I can reproduce with Sage 8.3 (division by zero, changing a polynomial by a negative index).
+
+- int(ntl.GF2X([])) raises a ValueError, it should return 0.:wq

[lftabera]

Branch: u/lftabera/ticket/25717

[lftabera]

New commits:

685a32f

- Added several EXAMPLES and TESTS

[lftabera]

Commit: 685a32f

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 685a32f to ff00283

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

592c161	Now, every method is doctested, except some __cinit__.
ff00283	Finish writing examples and tests.

[lftabera]

comment:4

The coverage was not bad. I have added some documentations to some methods that lacked one and several examples and test.

There were occasions were sage segfaults, mainly division by zero errors. I hope I have found all. I have added sig_on sig_off and tests in those places.

[lftabera]

Description changed:

--- 
+++ 
@@ -4,4 +4,6 @@

 - ntl calls lack in several places sig_on/sig_off, this can lead to Sage segfaults. I am adding test for the segfaults I can reproduce with Sage 8.3 (division by zero, changing a polynomial by a negative index).

-- int(ntl.GF2X([])) raises a ValueError, it should return 0.:wq
+- int(ntl.GF2X([])) raises a ValueError, it should return 0.
+
+- matrices over GF2, GF2E, IsIdent returns true if a parameter n is negative.

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

fcf8fb2

The same problem in IsIdent in ntl_mat_GF2E is also in ntl_mat_GF2

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from ff00283 to fcf8fb2

[lftabera]

Description changed:

--- 
+++ 
@@ -6,4 +6,4 @@

 - int(ntl.GF2X([])) raises a ValueError, it should return 0.

-- matrices over GF2, GF2E, IsIdent returns true if a parameter n is negative.
+- matrices over GF2, GF2E, IsIdent returns true if a parameter n is negative and the matrix is the identity matrix.

[tscrim]

comment:9

A few comments:

• You need to add your (real) name to the authors field.
• def IsIdent(self, n = None): -> def IsIdent(self, n=None):
• See `self.truediv` -> See :meth:`self.truediv`
• if modulus<=0: -> if modulus <= :

• Bad copy/paste:

  def __repr__(self):
      """
      Return the degree of self

  Also should be ``self``. (same for degree).

• Errors should start with a lower case letter (this is a Python convention that is not always followed in Sage).

• (I know this is done elsewhere in the file, but it is wrong) Change

         INPUT:
  
  -            modulus - a monic `ntl_ZZ_pX` object of degree less than that of `self`.
  +        - ``modulus`` -- a monic ``ntl_ZZ_pX`` object of degree less than that of ``self``
  
         OUTPUT:
  
  -            b - An element such that `self*b==1 mod modulus`.
  +        An element ``b`` such that ``self * b == 1`` mod ``modulus``.

• Change

  -Only really makes mathematical sense when n divides self.c.p
  +Only really makes mathematical sense when ``n`` divides ``self.c.p``.

• You should doctest those places where Sage was segfaulting with zero division errors show that Sage now simply raises an error.

[lftabera]

Author: Luis Felipe Tabera Alonso

[lftabera]

comment:10

Thanks for your comments, I will update this.

Concerning this

Replying to @tscrim:

• You should doctest those places where Sage was segfaulting with zero division errors show that Sage now simply raises an error.

The tests are already there, I even added some where sage does not segfault right now but to check that errors rise. I can add a docstring stating that those are a segfault tests.

[tscrim]

comment:11

Replying to @lftabera:

Thanks for your comments, I will update this.

Concerning this

Replying to @tscrim:

• You should doctest those places where Sage was segfaulting with zero division errors show that Sage now simply raises an error.

The tests are already there, I even added some where sage does not segfault right now but to check that errors rise. I can add a docstring stating that those are a segfault tests.

It would be good to add a quick statement saying something like Check that :trac:`25717` is fixed::.

[lftabera]

comment:12

Replying to @tscrim:

• Bad copy/paste:

  def __repr__(self):
      """
      Return the degree of self

  Also should be ``self``. (same for degree).

I change this for notmal methods, but I think that for __repr__ I should delete the sentence, I have not documented standard underscore python methods.

[tscrim]

comment:13

Replying to @lftabera:

Replying to @tscrim:

• Bad copy/paste:

  def __repr__(self):
      """
      Return the degree of self

  Also should be ``self``. (same for degree).

I change this for notmal methods, but I think that for __repr__ I should delete the sentence, I have not documented standard underscore python methods.

That's fine. When I do it, I add a simple one-liner, but I don't care at all.

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from fcf8fb2 to 0237bfa

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

0237bfa

Merge branch 'develop' into u/lftabera/ticket/25717