Hi Robert!
On Wed, Jan 27, 2010 at 03:24:07PM -0800, Robert Miller wrote:
> > * Merges cayley_graph with that for FiniteSemigroups. In the
> > merging, connecting_set was deprecated to generators. Also
> > providing a single element by itself as connecting set is no
> > more supported.
>
> Why is connecting_set being deprecated? Why is a connecting set of
> size 1 no longer supported? I can't say that I agree with any of these
> changes. What are the justifications? A particular user of Sage
> specifically requested that feature.
Ok, let's be precise: this *feature* is certainly not deprecated. In
particular, I did not remove any doctest. The proposal implemented in
the patch (among many other things) is:
Recommended use:
sage: G.cayley_graph(generators = [a,b,c])
Where [a,b,c] can be replaced by any iterable or family of
elements of G (or of data that can be coerced into elements
thereof). Of course this iterable can have a single element.
Deprecated use, but still functional:
sage: G.cayley_graph(connecting_set = [a,b,c])
Removed feature:
sage: G.cayley_graph(connecting_set = a)
Rationale:
(a) For the option name: that might be just me, but I find
``generators`` far more natural than ``connecting_set``.
This specifies an alternative set of generators for (a subgroup
of) G.
(b) For removing connecting_set = a: this feature cannot be
implemented in a robust way. If a is a tuple or an iterator, how
can cayley_graph determines generically whether a is an iterable
of elements of G, or some data that can be coerced into a single
element of G?
At the same time, this feature really does not bring much to the
user. It is not a fundamental and specific enough use case to save
him two brackets.
Altogether, I firmly vote for removing this feature. I am fine
with keeping it functional for a while, for backward
compatibility, but I deny any responsibility in any bug that could
occur because of that.
This is a call for votes!!!
> > Why do we care about the produced graph using implementation =
> > "networkx"? I would tend to remove this implementation detail;
> > this would change the order of the edges, which requires fixing
> > a test in sage.graphs.generic_graphs
>
> This is probably because of doctesting, back when I was trying to
> switch over and before vertex labels other than integers were
> supported.
Ok. I'll remove it then, unless someone complains loud.
Cheers,
Nicolas
PS: for the record: I did go the extra mile to write this patch, and
to use the occasion of a very specific feature I needed to do cleanup
in an area of code far from my own. It hence did take me a conscious
effort to wave as unintended the apparent aggressive and non-welcoming
tone of the feedback I got. No grumpy'o pa shooting please.
--
Nicolas M. Thiéry "Isil" <nthi...@users.sf.net>
http://Nicolas.Thiery.name/
--
To post to this group, send an email to sage-devel@googlegroups.com
To unsubscribe from this group, send an email to
sage-devel+unsubscr...@googlegroups.com
For more options, visit this group at http://groups.google.com/group/sage-devel
URL: http://www.sagemath.org
