Removes sage.combinat.generator

[nathanncohen]

Vincent noticed in #10534 that this module was completely useless because of itertools, and it even happens that nothing in Sage uses it.

What about getting rid of it ?

It implements:

• concat -- replaced by itertools.chain
• map -- replaced by ... map
• element -- replaced by itertools.repeat
• select -- replaced by itertools.ifilter
• successor -- "almost" replaced by itertools.takewhile

Nathann

CC: @videlec

Component: combinatorics

Author: Nathann Cohen

Reviewer: Travis Scrimshaw

Merged: sage-5.10.beta0

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

[videlec]

comment:2

I love patches that remove code and make Sage works better ! But, the best strategy would be to introduce deprecations and not to remove the functions. The reason is that some people may use the iterators in their own codes and that code should not be broken by a new Sage version.

Vincent

[nathanncohen]

comment:3

Come ooooooooooooon. These things are useless. Nobody knows they exist, and those who do will know about itertools....

[nathanncohen]

comment:4

So what do we do, we keep this useless thing for another year and ADD code to it ? -_-

Nathann

[nathanncohen]

comment:5

Ok let's solve it with Sage-devel. I'll deprecate this stuff if needed but that's really a waste of time.

Nathann

[nathanncohen]

comment:6

https://groups.google.com/forum/?fromgroups=#!topic/sage-devel/yGrfoB73u0c

[nathanncohen]

comment:8

Well, given that the only answers on sage-devel seemed to agree with plain removal, and that after my message on sage-combinat [1] nothing was added, I guess that this ticket is still waiting for a review.

Nathann

[1] https://groups.google.com/d/topic/sage-combinat-devel/xW_zvggLm84/discussion

[tscrim]

Author: Nathann Cohen

[tscrim]

Reviewer: Travis Scrimshaw

[tscrim]

comment:9

While I'm not 100% comfortable with outright removal of something in mid-level sage, I doubt anyone will really notice that this is gone (especially since it is only referenced in one place). So on that note, I'm setting this to positive review.

[jdemeyer]

comment:11

Traceback (most recent call last):
  File "./spkg-install", line 4, in <module>  
    from sage.all import save
  File "/mazur/release/merger/sage-5.10.beta0/local/lib/python2.7/site-packages/sage/all.py", line 96, in <module>
    from sage.algebras.all   import *
  File "/mazur/release/merger/sage-5.10.beta0/local/lib/python2.7/site-packages/sage/algebras/all.py", line 20, in <module>
    from quatalg.all import *
  File "/mazur/release/merger/sage-5.10.beta0/local/lib/python2.7/site-packages/sage/algebras/quatalg/__init__.py", line 3, in <module>
    import all
  File "/mazur/release/merger/sage-5.10.beta0/local/lib/python2.7/site-packages/sage/algebras/quatalg/all.py", line 1, in <module>
    from quaternion_algebra import QuaternionAlgebra
  File "/mazur/release/merger/sage-5.10.beta0/local/lib/python2.7/site-packages/sage/algebras/quatalg/quaternion_algebra.py", line 62, in <module>
    import quaternion_algebra_cython
  File "quaternion_algebra_cython.pyx", line 227, in init sage.algebras.quatalg.quaternion_algebra_cython (sage/algebras/quatalg/quaternion_algebra_cython.cpp:4439)
  File "classcall_metaclass.pyx", line 279, in sage.misc.classcall_metaclass.ClasscallMetaclass.__call__ (sage/misc/classcall_metaclass.c:932)
  File "/mazur/release/merger/sage-5.10.beta0/local/lib/python2.7/site-packages/sage/matrix/matrix_space.py", line 228, in __classcall__
    return super(MatrixSpace, cls).__classcall__(cls, base_ring, nrows, ncols, sparse)
  File "cachefunc.pyx", line 992, in sage.misc.cachefunc.WeakCachedFunction.__call__ (sage/misc/cachefunc.c:5175)
  File "/mazur/release/merger/sage-5.10.beta0/local/lib/python2.7/site-packages/sage/structure/unique_representation.py", line 447, in __classcall__
    instance = typecall(cls, *args, **options)
  File "classcall_metaclass.pyx", line 467, in sage.misc.classcall_metaclass.typecall (sage/misc/classcall_metaclass.c:1294)
  File "/mazur/release/merger/sage-5.10.beta0/local/lib/python2.7/site-packages/sage/matrix/matrix_space.py", line 256, in __init__
    from sage.categories.all import Modules, Algebras
  File "/mazur/release/merger/sage-5.10.beta0/local/lib/python2.7/site-packages/sage/categories/all.py", line 126, in <module>
    from crystals import Crystals
  File "/mazur/release/merger/sage-5.10.beta0/local/lib/python2.7/site-packages/sage/categories/crystals.py", line 17, in <module>
    from sage.combinat.subset import Subsets  
  File "/mazur/release/merger/sage-5.10.beta0/local/lib/python2.7/site-packages/sage/combinat/subset.py", line 33, in <module>
    import sage.combinat.subword as subword   
  File "/mazur/release/merger/sage-5.10.beta0/local/lib/python2.7/site-packages/sage/combinat/subword.py", line 50, in <module>
    import sage.combinat.combination as combination
  File "/mazur/release/merger/sage-5.10.beta0/local/lib/python2.7/site-packages/sage/combinat/combination.py", line 23, in <module>
    from integer_vector import IntegerVectors 
  File "/mazur/release/merger/sage-5.10.beta0/local/lib/python2.7/site-packages/sage/combinat/integer_vector.py", line 35, in <module>
    import integer_list
  File "/mazur/release/merger/sage-5.10.beta0/local/lib/python2.7/site-packages/sage/combinat/integer_list.py", line 34, in <module>
    import generator
ImportError: No module named generator

[nathanncohen]

comment:12

Green light from the patchbot in beta2 and something uses it in 5.10.beta0 ?... Honestly -_-

Nathann

[nathanncohen]

comment:13

I don't get it. This import line IS in the beta1 version, but all tests do pass on that file O_o

Doctesting 1 file.
sage -t --long integer_list.py
    [197 tests, 0.2 s]
----------------------------------------------------------------------
All tests passed!
----------------------------------------------------------------------

And... With the patch applied, this works :

sage: import sage.combinat.generator

even though the file does not exist anymore O_o

Nathann

[nathanncohen]

comment:14

Riiiiiiiight.... Because even though the module has been removed the .pyc files still exists, and is not removed by sage -b. >_<

Nathann

[tscrim]

comment:15

The only place I could find another reference to it was in integer_list.py (without patch on 5.9.beta1):

travis@travis-virtualbox:~/sage-5.9.beta1/devel/sage-main/sage$ grep -R "combinat\.generator" .
./combinat/generator.py:        sage: list(sage.combinat.generator.concat([[1,2,3],[4,5,6]]))
./combinat/generator.py:        sage: list(sage.combinat.generator.map(f,[4,5,6]))
./combinat/generator.py:        sage: list(sage.combinat.generator.element(1))
./combinat/generator.py:        sage: list(sage.combinat.generator.element(1, n=3))
./combinat/generator.py:        sage: list(sage.combinat.generator.select(f,range(7)))
./combinat/generator.py:        sage: list(sage.combinat.generator.successor(0,f))

travis@travis-virtualbox:~/sage-5.9.beta1/devel/sage-main/sage/combinat$ grep "import *generator" *
integer_list.py:import generator

so it should just be a matter of replacing it there as well.

[nathanncohen]

comment:16

Attachment: trac_14355.patch.gz

Fixed ! And ready for another review ^^;

Nathann

[tscrim]

comment:17

Looks good to me after running sync-build.

[jdemeyer]

Merged: sage-5.10.beta0