#18929: Include igraph library
-------------------------------------+-------------------------------------
Reporter: borassi | Owner:
Type: enhancement | Status: needs_review
Priority: major | Milestone: sage-6.8
Component: packages: | Resolution:
optional | Merged in:
Keywords: igraph library | Reviewers: Nathann Cohen
Authors: Michele Borassi | Work issues:
Report Upstream: N/A | Commit:
Branch: | 24920b6ffa69b6e92efabf359970a96464912d8e
u/borassi/include_igraph_library | Stopgaps:
Dependencies: |
-------------------------------------+-------------------------------------
Comment (by borassi):
Hellooooooo!
Well, I go for a variation of the first solution you proposed: simplifying
as much as possible the interface with igraph. In particular, I have
removed the graph name, and vertex and edge attributes are provided by the
user in "igraph-style" ([http://igraph.org/python/doc/igraph.Graph-
class.html#__init__ http://igraph.org/python/doc/igraph.Graph-
class.html#__init__]). There are several examples on how to do it in my
new code.
For the igraph=>sage conversion, I have used your relabel function for
vertex names, while the edge label is simply kept as it was in igraph
(only problem: empty labels in igraph translate to `{}` label in Sage, not
to `None`).
More answers below! Hope you like this new commit!
See you,
Michele
Replying to [comment:25 ncohen]:
> Helooooooo Michele,
>
> I finished reviewing this ticket again. I'm sorry but if there is one
thing I cannot stand, it is to see heaps of code meant to deal with edge
labels/multile edges/loops/weights. Dozens of line which do nothing but
wrap junk into another kind of junk.
>
> Well.
>
> I added a commit at public/18929 which does the following modifications,
which you can of course accept/discuss/reject:
>
> - documentation of `igraph` input -- I removed the end of the sentence
about what is done with weights/names/etc. It is not said for other
formats and it is how it is expected to work. I thought that the shortest
was the best.
Accept
> - removed some trailing whitespaces
Accept
> - changed the message in which you mentionned a 'network'. Graphs are..
Well, nobody exactly knows. Depending on who you talk to, when you say "a
graph" people hear "social network graph", "router graph", "planar graph",
"vertex-transitive graph", "cayley graph", and many others. Networks do
exist, but that would not make *any* sense for some of those guys, so I
stuck to the 'graph' terminology.
Accept
> - I "compressed" a bit the copy/paste meant to handle names, by using
the relabel function. For this I had to change a line in
`GenericGraph.relabel` related to `_boundary`, but this `#O&O#&&^@)#%*(`
is meant to be removed in #17462 anyway.
Accept
> - Try to not use `except Exception` if you can avoid it. That's too
large. If there is a typo in the line before it, it will catch this too
and you don't want that. Also, try to keep as little as possible in a
try/except.
Accept (modified also in the digraph file)
> Finally, I have to say that I can't bring myself to give a positive
review to this latest commit. Much of what it does is, to me, meant to be
done by the user. We cannot adapt to the many forms he wants his data to
take in the igraph graph, so I think that the best we should do is ...
"keep it simple and reliable". Guessing that 'label' is a special key in
the dictionary is (to me) going way too far, and I am tempted to do the
same for weights. Of course their labels *have* to be dictionaries and
ours can be anything. That's unpleasant, but turning our label into a
`{'label':label}` in their data structure, and setting their dictionary to
be our edge label is to me the best we can do.
Unfortunately, I don't think so. The problem occurs if we want to convert
weighted graphs (and some of the algorithms we want to "steal" are on
weighted graph). Indeed, an igraph graph is weighted if edges contain an
attribute 'weight', and with this conversion there is no way to obtain
that. My solution is to let the user provide the labels.
> So yes, if you apply translation several times you will not converge.
Honestly, I do not care at all.
Now, if the starting graph has "igraph-friendly" labels and the user
provides the right attributes, it converges (see the examples).
> Sooooo, sorry for that. You have several ways out:
>
> - Get this ticket reviewed by somebody else (I will not force my view, I
just won't review it myself -- no way)
> - Remove the commits you added since my last 'positive review' so that
this ticket can get merged, and move those label commits to another
ticket.
>
> As you wish.
>
> Have fun,
>
> Nathann
--
Ticket URL: <http://trac.sagemath.org/ticket/18929#comment:26>
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/d/optout.
