#13440: Adding reverse_edge() function to DiGraph
---------------------------------+------------------------------------------
Reporter: egunawan | Owner: jason, ncohen, rlm
Type: enhancement | Status: needs_review
Priority: major | Milestone: sage-5.7
Component: graph theory | Resolution:
Keywords: digraph | Work issues:
Report Upstream: N/A | Reviewers: Gregg Musiker
Authors: Emily Gunawan | Merged in:
Dependencies: | Stopgaps:
---------------------------------+------------------------------------------
Comment (by gmoose05):
Replying to [comment:11 ncohen]:
> If I may express a personal opinion, I really think that adding methods
that replace the two lines of code you have to write -- when you know the
digraph you deal with -- is not really what we need right now, considering
the length of the list of DiGraph's functions.... These methods spend more
time checking the input (labelled ? Multigraph ?) than actually doing what
they are asked to do, even do unnecessary operations (your code begins by
checking that the edge given as an argument actually exists in the
graph....).
>
> And as usual when dealing with this mess of options, you have the usual
corner-cases which have no good solution in general
>
> Example :
> {{{
> sage: d = DiGraph()
> sage: d.add_edge(1,2)
> sage: d.add_edge(2,1)
> sage: d.edges()
> [(1, 2, None), (2, 1, None)]
> sage: dd= d.reverse_edge((1,2))
> sage: dd.edges()
> [(2, 1, None)]
> }}}
>
> As there is no good way to fix this, it makes the function unreliable
(you have no idea what the output could be unless you read the code) -->
it is better (and faster) to rewrite your own than to use a method whose
behaviour could be unexpected (also called : wrong). Like with the merge_*
methods, and many others ....
>
> Of course, it is just my personal opinion...
>
> Nathann
I see some of your points Nathann regarding this ticket, however some
users might still find it useful. I should mention that the main reason
we decided to write this ticket was that reversal of a single edge or
group of edges is a function that we will need to use over and over again
in a future patch for the Cluster Algebra and Quiver package. We
discussed this at Sage Days 40 and it appeared that other users present
would also utilize this command, although I am not sure how many avid
graphs sage developers were at this meeting. Also Ticket #7720 seemed to
request a command for reversing an edge in place as well.
There are indeed a few more cases to deal with than we first anticipated,
given the flexibility of inputs for digraphs, which made the code longer
than we first thought it would be, but if the reverse_edge() method is
called within another subroutine, breaking into such cases appear
unavoidable anyway. Although we are open to suggestion.
As far as the issue "it makes the function unreliable" goes, the example
you wrote out is not an issue with the reverse_edge() code but just a
manifestation of the fact that the DiGraph d being used does not allow
multiple edges. For instance, d.add_edge(1,2); d.add_edge(1,2) would
cause the same behavior. We now have an example in the documentation
clarifying the expected output for the user.
In summary, given that this method will be needed for other sage code
anyway, we were interested in getting the opinions from those developing
the graphs code. Is it better to have reverse_edge() and reverse_edges()
in digraph where they are methods for the digraph class, or as
_reverse_edge() and _reverse_edges() as auxiliary commands in separate
sage code?
Thanks,
Gregg
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/13440#comment:15>
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?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.
