#14806: Immutable graph backend
---------------------------------+------------------------------------------
       Reporter:  ncohen         |         Owner:  jason, ncohen, rlm
           Type:  enhancement    |        Status:  needs_review      
       Priority:  major          |     Milestone:  sage-5.12         
      Component:  graph theory   |    Resolution:                    
       Keywords:                 |   Work issues:                    
Report Upstream:  N/A            |     Reviewers:                    
        Authors:  Nathann Cohen  |     Merged in:                    
   Dependencies:  #14805         |      Stopgaps:                    
---------------------------------+------------------------------------------

Comment (by ncohen):

 Hello !

 > * Its either "Thou shalt not add a vertex to an immutable graph!"
 >   (Shakespeare) or "cannot add vertex to an immutable graph"
 >   (Python). In particular, lower case and no exclamation mark at the
 >   end ;)

 I obviously chosed the "Thou shalt not add a vertex to an immutable
 graph".

 > * Methods that fail due to being immutable should raise `ValueError`,
 >   not `NotImplementedError` (are you going to implement it in the
 >   future?)

 I change my mind so often that I wouldn't put it beyond reach. But just to
 please you I fixed it `:-P`

 > * It would be nice if INPUT/OUTPUT were consistent and would actually
 >   list all arguments, for example (though there are many more)
 >   `StaticSparseBackend.degree` does not document the "directed"
 >   argument.

 Well, I defined the "INPUT" where it was not, but usually the outputs are
 pretty clear... Not to mention that those are rather meant to Sage
 developpers.

 > * static_sparse_backend module docstring "Two classes" names the same
 >   class twice.

 It's a poetic license.

 > * StaticSparseCGraph docstring: ":mod:`CGraph
 >   <sage.graphs.base.c_graph>` class over :mod:`static sparse graphs
 >   <sage.graphs.base.static_dense_graph>`". I don't know what thats
 >   supposed to mean.

 That it uses static sparse graphs as a data structure. By the way, the
 link and the name did not match. I rephrased it.

 > * The Python special `__copy__()` method does not allow any
 >   arguments. In particular, `copy(graph)` should return a mutable copy
 >   if the original `graph` is immutable.

 There were arguments there before I learnt that Sage existed, and it is
 because this method can also be used through `G.copy(whatever)`. This
 being said, I don't see why one should get a mutable graph as a copy of an
 immutable one. Copying a tuple certainly does not make it a list, and it
 would produce rather unexpected behaviour `O_o`

 > * Having to write `data_structure="sparse"` is a lot less intuitive
 >   than the old `sparse=True`. It would be better to allow for both
 >   and raise a `ValueError` if both are changed from their default
 >   values.

 You have no idea how many hours I wasted on that. Unfortunately, you are
 right. I reversed those modifications and both keywords can now be used.

 Patch updated !

 Nathann

-- 
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/14806#comment:9>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica, 
and MATLAB

-- 
You received this message because you are subscribed to the Google Groups 
"sage-trac" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/groups/opt_out.


Reply via email to