#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.