Circulant digraphs

[nathanncohen]

A new constructor for digraphs !

CC: @dcoudert

Component: graph theory

Author: Nathann Cohen

Reviewer: David Coudert

Merged: sage-5.9.beta0

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

[dcoudert]

Reviewer: David Coudert

[dcoudert]

comment:2

Attachment: trac_14251.patch.gz

Hello,

• I have read somewhere that imports should now be at module level, but I'm not sure...
• You should test that n>=0
• You should check that integers is an iterable (list, set, dict?) with valid numbers (can we have negative numbers? what if a number is larger than n? what to do with 0?)

David.

[nathanncohen]

comment:3

Yo !

• I have read somewhere that imports should now be at module level, but I'm not sure...

Cython imports should be, but that's all. Anyway you cannot have all imports at module level, it wouldn't work. You need to use graph generators in digraph.py, and you need to use digraphs indigraph_generators too.

• You should test that n>=0

Why not...

• You should check that integers is an iterable (list, set, dict?) with valid numbers (can we have negative numbers? what if a number is larger than n? what to do with 0?)

If it is not iterable the "for j in integers" would fail with a clear message :

sage: digraphs.Circulant(4,5)      
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-2-50bde61731c6> in <module>()
----> 1 digraphs.Circulant(Integer(4),Integer(5))

/home/ncohen/.Sage/local/lib/python2.7/site-packages/sage/graphs/digraph_generators.pyc in Circulant(self, n, integers)
    407 
    408         for v in range(n):
--> 409             G.add_edges([(v,(v+j)%n) for j in integers])
    410 
    411         return G

TypeError: 'sage.rings.integer.Integer' object is not iterable
sage:

I will add some checks.. And I really hate when a 3-lines code takes 10 lines to check bad input... Users should learn to deal with them too.

Nathann

[nathanncohen]

Attachment: trac_14251-input.patch.gz

[nathanncohen]

comment:4

Done !

Nathann

[dcoudert]

comment:5

That's much better ;-)

I have plenty of warnings when building the documentation (with 5.8.beta4), but this has nothing to do with your patch (a sample bellow).

For me the patch is good to go.

Sample of warnings:

[graphs   ] Exception OverflowError: 'long int too large to convert to int' in <sage.structure.coerce_dict.TripleDictEraser object at 
0x8c83f44> ignored

[misc     ] Exception OverflowError: 'long int too large to convert to int' in <sage.structure.coerce_dict.TripleDictEraser object at 
0x8c83f44> ignored

[notebook ] Exception OverflowError: 'long int too large to convert to int' in <sage.structure.coerce_dict.TripleDictEraser object at 
0x8c83f44> ignored

[categorie] Exception OverflowError: 'long int too large to convert to int' in <sage.structure.coerce_dict.TripleDictEraser object at 
0x8c83f44> ignored

[geometry ] Exception OverflowError: 'long int too large to convert to int' in <sage.structure.coerce_dict.TripleDictEraser object at 
0x8c83f44> ignored

[interface] Exception OverflowError: 'long int too large to convert to int' in <sage.structure.coerce_dict.TripleDictEraser object at 
0x8c83f44> ignored

[libs     ] Exception OverflowError: 'long int too large to convert to int' in <sage.structure.coerce_dict.TripleDictEraser object at 
0x8c83f44> ignored

[matrices ] Exception OverflowError: 'long int too large to convert to int' in <sage.structure.coerce_dict.TripleDictEraser object at 
0x8c83f44> ignored

[rings_sta] Exception OverflowError: 'long int too large to convert to int' in <sage.structure.coerce_dict.TripleDictEraser object at 0xa7d0f44> ignored

[structure] Exception OverflowError: 'long int too large to convert to int' in <sage.structure.coerce_dict.TripleDictEraser object at 0xa7d0f44> ignored

[nathanncohen]

comment:6

Thaaaaaaanks !!! And I don't think that the doc warnings are related either. Actually the doc is such a mess these days... I'm eager to see it turn back to normal. It's almost impossible t check the doc of a patch for me right now :-/

Nathann

[jdemeyer]

Merged: sage-5.9.beta0