Remove unused files from module_list.py

xcaruso commented 11 years ago

The file devel/sage/sage/matrix/matrix_domain_dense.pyx is not used for anything, so remove it (it was marked "nodoctest", so it wasn't tested either). Also remove other commented-out entries from module_list.py

Apply attachment: trac_15109_clean_distribution.patch

CC: @jdemeyer

Component: doctest coverage

Keywords: failing doctests

Author: Xavier Caruso

Reviewer: Travis Scrimshaw, Jeroen Demeyer

Merged: sage-5.12.beta5

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

tscrim commented 11 years ago

Reviewer: Travis Scrimshaw

tscrim commented 11 years ago

comment:2

Perhaps it's because the doctesting framework previously wasn't actually testing them since they aren't properly formatted as doctests? I really can't say. The patchbot didn't seem to catch these doctest failures as well...

I also get failures on 5.12.beta3 (13 of them to be precise) and the last time this file was touched was #10028. I've cc-ed Jeroen to let him know about this and put this ticket as a blocker.

In any case, I think you should also do the following:

not remove any doctests and instead figure out what needs to be changed so they don't fail,
change the documentation so that it is formatted correctly,
put a proper commit message for the patch.

Best,

Travis

xcaruso commented 11 years ago

comment:3

Replying to @tscrim:

not remove any doctests and instead figure out what needs to be changed so they don't fail,

I tried actually to do it but it appears that:

the method _assign_names is obsolete (at least as it is used in the doctest) - but I can of course use change_variable_name instead
I don't know what the attribute bound is supposed to do but it doesn't appear in the method charpoly for matrices over fields.

Apart from that, I agree with your two other comments.

xcaruso commented 11 years ago

comment:4

Revised version of the patch posted.

jdemeyer commented 11 years ago

comment:5

It seems that those tests (which tests really?) only fail for you, so you absolutely need to give more details.

xcaruso commented 11 years ago

comment:6

On a clean sage-5.12.beta3, I get:

sage@irma-cethop ~/sage-5.12.beta3/devel/sage/sage/matrix $ sage-5.12 -t matrix_domain_dense.pyx
Running doctests with ID 2013-08-28-08-09-58-e444f178.
Doctesting 1 file.
sage -t matrix_domain_dense.pyx
**********************************************************************
File "matrix_domain_dense.pyx", line 67, in sage.matrix.matrix_domain_dense.Matrix_domain_dense.charpoly
Failed example:
    R = PolynomialRing(IntegerRing(),2); x,y = R.gens()
Exception raised:
    Traceback (most recent call last):
      File "/home2/sage/sage-5.12.beta3/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 479, in _run
        self.execute(example, compiled, test.globs)
      File "/home2/sage/sage-5.12.beta3/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 838, in execute
        exec compiled in globs
      File "<doctest sage.matrix.matrix_domain_dense.Matrix_domain_dense.charpoly[12]>", line 1, in <module>
        R = PolynomialRing(IntegerRing(),Integer(2)); x,y = R.gens()
      File "/home2/sage/sage-5.12.beta3/local/lib/python2.7/site-packages/sage/rings/polynomial/polynomial_ring_constructor.py", line 448, in PolynomialRing
        raise TypeError("You must specify the names of the variables.")
    TypeError: You must specify the names of the variables.
**********************************************************************
File "matrix_domain_dense.pyx", line 68, in sage.matrix.matrix_domain_dense.Matrix_domain_dense.charpoly
Failed example:
    A = MatrixSpace(R,2)([x, y, x^2, y^2])
Exception raised:
    Traceback (most recent call last):
      File "/home2/sage/sage-5.12.beta3/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 479, in _run
        self.execute(example, compiled, test.globs)
      File "/home2/sage/sage-5.12.beta3/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 838, in execute
        exec compiled in globs
      File "<doctest sage.matrix.matrix_domain_dense.Matrix_domain_dense.charpoly[13]>", line 1, in <module>
        A = MatrixSpace(R,Integer(2))([x, y, x**Integer(2), y**Integer(2)])
    NameError: name 'y' is not defined
**********************************************************************
File "matrix_domain_dense.pyx", line 70, in sage.matrix.matrix_domain_dense.Matrix_domain_dense.charpoly
Failed example:
    f
Expected:
    x^2 + (-1*x1^2 - x0)*x + x0*x1^2 - x0^2*x1
Got:
    x^2 - 5*x - 2
**********************************************************************
File "matrix_domain_dense.pyx", line 75, in sage.matrix.matrix_domain_dense.Matrix_domain_dense.charpoly
Failed example:
    f.parent()._assign_names("Z")
Exception raised:
    Traceback (most recent call last):
      File "/home2/sage/sage-5.12.beta3/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 479, in _run
        self.execute(example, compiled, test.globs)
      File "/home2/sage/sage-5.12.beta3/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 838, in execute
        exec compiled in globs
      File "<doctest sage.matrix.matrix_domain_dense.Matrix_domain_dense.charpoly[16]>", line 1, in <module>
        f.parent()._assign_names("Z")
      File "parent_gens.pyx", line 329, in sage.structure.parent_gens.ParentWithGens._assign_names (sage/structure/parent_gens.c:3464)
      File "category_object.pyx", line 416, in sage.structure.category_object.CategoryObject._assign_names (sage/structure/category_object.c:4500)
    ValueError: variable names cannot be changed after object creation.
**********************************************************************
File "matrix_domain_dense.pyx", line 76, in sage.matrix.matrix_domain_dense.Matrix_domain_dense.charpoly
Failed example:
    f
Expected:
    Z^2 + (-1*x1^2 - x0)*Z + x0*x1^2 - x0^2*x1
Got:
    x^2 - 5*x - 2
**********************************************************************
File "matrix_domain_dense.pyx", line 82, in sage.matrix.matrix_domain_dense.Matrix_domain_dense.charpoly
Failed example:
    A.charpoly(bound=2)
Exception raised:
    Traceback (most recent call last):
      File "/home2/sage/sage-5.12.beta3/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 479, in _run
        self.execute(example, compiled, test.globs)
      File "/home2/sage/sage-5.12.beta3/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 838, in execute
        exec compiled in globs
      File "<doctest sage.matrix.matrix_domain_dense.Matrix_domain_dense.charpoly[19]>", line 1, in <module>
        A.charpoly(bound=Integer(2))
      File "matrix_integer_dense.pyx", line 986, in sage.matrix.matrix_integer_dense.Matrix_integer_dense.charpoly (sage/matrix/matrix_integer_dense.c:11541)
        def charpoly(self, var='x', algorithm='linbox'):
    TypeError: charpoly() got an unexpected keyword argument 'bound'
**********************************************************************
File "matrix_domain_dense.pyx", line 103, in sage.matrix.matrix_domain_dense.Matrix_domain_dense.determinant
Failed example:
    R = PolynomialRing(IntegerRing(),2); x,y = R.gens()
Exception raised:
    Traceback (most recent call last):
      File "/home2/sage/sage-5.12.beta3/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 479, in _run
        self.execute(example, compiled, test.globs)
      File "/home2/sage/sage-5.12.beta3/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 838, in execute
        exec compiled in globs
      File "<doctest sage.matrix.matrix_domain_dense.Matrix_domain_dense.determinant[0]>", line 1, in <module>
        R = PolynomialRing(IntegerRing(),Integer(2)); x,y = R.gens()
      File "/home2/sage/sage-5.12.beta3/local/lib/python2.7/site-packages/sage/rings/polynomial/polynomial_ring_constructor.py", line 448, in PolynomialRing
        raise TypeError("You must specify the names of the variables.")
    TypeError: You must specify the names of the variables.
**********************************************************************
File "matrix_domain_dense.pyx", line 104, in sage.matrix.matrix_domain_dense.Matrix_domain_dense.determinant
Failed example:
    A = MatrixSpace(R,2)([x, y, x**2, y**2])
Exception raised:
    Traceback (most recent call last):
      File "/home2/sage/sage-5.12.beta3/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 479, in _run
        self.execute(example, compiled, test.globs)
      File "/home2/sage/sage-5.12.beta3/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 838, in execute
        exec compiled in globs
      File "<doctest sage.matrix.matrix_domain_dense.Matrix_domain_dense.determinant[1]>", line 1, in <module>
        A = MatrixSpace(R,Integer(2))([x, y, x**Integer(2), y**Integer(2)])
      File "classcall_metaclass.pyx", line 330, in sage.misc.classcall_metaclass.ClasscallMetaclass.__call__ (sage/misc/classcall_metaclass.c:1224)
      File "/home2/sage/sage-5.12.beta3/local/lib/python2.7/site-packages/sage/matrix/matrix_space.py", line 215, in __classcall__
        if base_ring not in _Rings:
      File "category_singleton.pyx", line 81, in sage.categories.category_singleton.Category_contains_method_by_parent_class.__call__ (sage/categories/category_singleton.c:1428)
    TypeError: descriptor 'category' of 'sage.structure.parent.Parent' object needs an argument
**********************************************************************
File "matrix_domain_dense.pyx", line 105, in sage.matrix.matrix_domain_dense.Matrix_domain_dense.determinant
Failed example:
    A.determinant()
Exception raised:
    Traceback (most recent call last):
      File "/home2/sage/sage-5.12.beta3/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 479, in _run
        self.execute(example, compiled, test.globs)
      File "/home2/sage/sage-5.12.beta3/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 838, in execute
        exec compiled in globs
      File "<doctest sage.matrix.matrix_domain_dense.Matrix_domain_dense.determinant[2]>", line 1, in <module>
        A.determinant()
    NameError: name 'A' is not defined
**********************************************************************
File "matrix_domain_dense.pyx", line 142, in sage.matrix.matrix_domain_dense.Matrix_domain_dense.is_invertible
Failed example:
    R = PolynomialRing(IntegerRing())
Exception raised:
    Traceback (most recent call last):
      File "/home2/sage/sage-5.12.beta3/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 479, in _run
        self.execute(example, compiled, test.globs)
      File "/home2/sage/sage-5.12.beta3/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 838, in execute
        exec compiled in globs
      File "<doctest sage.matrix.matrix_domain_dense.Matrix_domain_dense.is_invertible[7]>", line 1, in <module>
        R = PolynomialRing(IntegerRing())
      File "/home2/sage/sage-5.12.beta3/local/lib/python2.7/site-packages/sage/rings/polynomial/polynomial_ring_constructor.py", line 448, in PolynomialRing
        raise TypeError("You must specify the names of the variables.")
    TypeError: You must specify the names of the variables.
**********************************************************************
File "matrix_domain_dense.pyx", line 143, in sage.matrix.matrix_domain_dense.Matrix_domain_dense.is_invertible
Failed example:
    x = R.gen()
Exception raised:
    Traceback (most recent call last):
      File "/home2/sage/sage-5.12.beta3/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 479, in _run
        self.execute(example, compiled, test.globs)
      File "/home2/sage/sage-5.12.beta3/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 838, in execute
        exec compiled in globs
      File "<doctest sage.matrix.matrix_domain_dense.Matrix_domain_dense.is_invertible[8]>", line 1, in <module>
        x = R.gen()
    AttributeError: type object 'R' has no attribute 'gen'
**********************************************************************
File "matrix_domain_dense.pyx", line 144, in sage.matrix.matrix_domain_dense.Matrix_domain_dense.is_invertible
Failed example:
    A = MatrixSpace(R,2)([1,x,0,-1])
Exception raised:
    Traceback (most recent call last):
      File "/home2/sage/sage-5.12.beta3/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 479, in _run
        self.execute(example, compiled, test.globs)
      File "/home2/sage/sage-5.12.beta3/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 838, in execute
        exec compiled in globs
      File "<doctest sage.matrix.matrix_domain_dense.Matrix_domain_dense.is_invertible[9]>", line 1, in <module>
        A = MatrixSpace(R,Integer(2))([Integer(1),x,Integer(0),-Integer(1)])
      File "classcall_metaclass.pyx", line 330, in sage.misc.classcall_metaclass.ClasscallMetaclass.__call__ (sage/misc/classcall_metaclass.c:1224)
      File "/home2/sage/sage-5.12.beta3/local/lib/python2.7/site-packages/sage/matrix/matrix_space.py", line 215, in __classcall__
        if base_ring not in _Rings:
      File "category_singleton.pyx", line 81, in sage.categories.category_singleton.Category_contains_method_by_parent_class.__call__ (sage/categories/category_singleton.c:1428)
    TypeError: descriptor 'category' of 'sage.structure.parent.Parent' object needs an argument
**********************************************************************
File "matrix_domain_dense.pyx", line 147, in sage.matrix.matrix_domain_dense.Matrix_domain_dense.is_invertible
Failed example:
    ~A
Expected:
    [ 1  x]
    [ 0 -1]
Got:
    [ 1 10]
    [ 0 -1]
**********************************************************************
3 items had failures:
   6 of  23 in sage.matrix.matrix_domain_dense.Matrix_domain_dense.charpoly
   3 of   4 in sage.matrix.matrix_domain_dense.Matrix_domain_dense.determinant
   4 of  13 in sage.matrix.matrix_domain_dense.Matrix_domain_dense.is_invertible
    [47 tests, 13 failures, 0.30 s]
----------------------------------------------------------------------
sage -t matrix_domain_dense.pyx  # 13 doctests failed
----------------------------------------------------------------------
Total time for all tests: 0.4 seconds
    cpu time: 0.3 seconds
    cumulative wall time: 0.3 seconds

jdemeyer commented 11 years ago

comment:7

Replying to @xcaruso:

On a clean sage-5.12.beta3

Are you very sure about this? And is it reproducible?

jdemeyer commented 11 years ago

Description changed:

--- 
+++ 
@@ -1,3 +1 @@
-It seems that there are some failing doctests in `sage.matrix.matrix_domain_dense`. (How is it possible?).
-
-I attach to this ticket a small patch fixing this.
+The file `devel/sage/sage/matrix/matrix_domain_dense.pyx` is marked `nodoctest` for no obvious reason. Fix this and also fix some failing doctests and their formatting.

jdemeyer commented 11 years ago

comment:9

That nodoctest (together with a bunch of others) was introduced by

changeset:   1775:0c9a4478bc53
user:        William Stein <wstein@gmail.com>
date:        Sun Nov 05 02:37:30 2006 -0800
summary:     Stabilizing SAGE (i.e., getting doctests to pass) after all these changes...

tscrim commented 11 years ago

comment:10

Ahhh I see. I was thinking that was only for that block and not the whole file and explains why it was not tested by testall/the patchbot. Thank you for clarifying Jeroen.

jdemeyer commented 11 years ago

Description changed:

--- 
+++ 
@@ -1 +1,3 @@
 The file `devel/sage/sage/matrix/matrix_domain_dense.pyx` is marked `nodoctest` for no obvious reason. Fix this and also fix some failing doctests and their formatting.
+
+While we're at it, remove `nodoctest` from `sage/schemes/affine/all.py` and `sage/schemes/projective/all.py` which have no doctests anyway...

jdemeyer commented 11 years ago

comment:12

This looks like a reasonable test which should be kept:

sage: A = 1000*MatrixSpace(ZZ,10)(range(100))
sage: A.charpoly('x')
x^10 - 495000*x^9 - 8250000000*x^8

jdemeyer commented 11 years ago

comment:13

"we rename use another name" isn't proper English.

jdemeyer commented 11 years ago

comment:14

I personally prefer ZZ to IntegerRing() but that's just a choice.

jdemeyer commented 11 years ago

Changed reviewer from Travis Scrimshaw to Travis Scrimshaw, Jeroen Demeyer

jdemeyer commented 11 years ago

comment:15

If you make these changes and remove the nodoctest from the 3 files mentioned in the ticket description, I'll give a final review.

xcaruso commented 11 years ago

comment:16

Done.

jdemeyer commented 11 years ago

comment:17

One more detail: TESTS: should be TESTS::

xcaruso commented 11 years ago

Attachment: trac_15109_failing_doctests.patch.gz

xcaruso commented 11 years ago

comment:18

Fixed.

tscrim commented 11 years ago

comment:20

So it turns out that matrix_domain_dense.pyx is currently not built as part of Sage. When you apply the patch, no cython files are rebuilt, and I tried to add it to the documentation but got a "cannot find file" error. This is why it wasn't doctested (and whose doctests are not actually testing the file) and I'm thinking this file is obsolete and should be removed.

xcaruso commented 11 years ago

comment:21

This module is actually not included in module_list.py:

#Extension('sage.matrix.matrix_domain_dense',
#          sources = ['sage/matrix/matrix_domain_dense.pyx']),

It's the same for the module sage.matrix.matrix_domain_sparse. Should we remove the two corresponding files from the distribution?

By the way, the following commented lines in modules_list.py do not correspond to actual files:

#Extension('sage.matrix.matrix_cyclo_sparse',
#          sources = ['sage/matrix/matrix_cyclo_sparse.pyx']),

#Extension('sage.matrix.matrix_pid_dense',
#          sources = ['sage/matrix/matrix_pid_dense.pyx']),

#Extension('sage.matrix.matrix_pid_sparse',
#          sources = ['sage/matrix/matrix_pid_sparse.pyx']),

#Extension('sage.matrix.padics.matrix_padic_capped_relative_dense',
#          sources = ['sage/matrix/padics/matrix_padic_capped_relative_dense.pyx']),

#Extension('sage.rings.real_rqdf',
#          sources = ["sage/rings/real_rqdf.pyx"],
#          libraries = ['qd', 'm', 'stdc++','gmp','mpfr' ],
#          language='c++'),

Can we delete them?

tscrim commented 11 years ago

comment:22

The sparse is just a shell class which just inherits from the general matrix class. I'm thinking we should delete them both and remove the commented out lines.

jdemeyer commented 11 years ago

comment:23

Good point, I hadn't noticed that that file is unused. +1 to completely removing it.

xcaruso commented 11 years ago

Attachment: trac_15109_clean_distribution.patch.gz

xcaruso commented 11 years ago

comment:24

Ok, let's do it.

apply only trac_15109_clean_distribution.patch

tscrim commented 11 years ago

comment:25

Looks good to me.

jdemeyer commented 11 years ago

Description changed:

--- 
+++ 
@@ -1,3 +1,3 @@
-The file `devel/sage/sage/matrix/matrix_domain_dense.pyx` is marked `nodoctest` for no obvious reason. Fix this and also fix some failing doctests and their formatting.
+The file `devel/sage/sage/matrix/matrix_domain_dense.pyx` is not used for anything, so remove it (it was marked "nodoctest", so it wasn't tested either). Also remove other commented-out entries from `module_list.py`

-While we're at it, remove `nodoctest` from `sage/schemes/affine/all.py` and `sage/schemes/projective/all.py` which have no doctests anyway...
+**Apply** [attachment: trac_15109_clean_distribution.patch](https://github.com/sagemath/sage/files/ticket15109/561ef01e7b4dd1022cb3cff6e1390c4b.gz)

jdemeyer commented 11 years ago

Merged: sage-5.12.beta5

sagemath / sage

Remove unused files from module_list.py #15109