Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Graph constructor : Graph(edges=[ ... ] ) #7709

Closed
nathanncohen mannequin opened this issue Dec 16, 2009 · 14 comments
Closed

Graph constructor : Graph(edges=[ ... ] ) #7709

nathanncohen mannequin opened this issue Dec 16, 2009 · 14 comments

Comments

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Dec 16, 2009

I often need to create graphs defined by a set of edges, and it should not be hard to add a new constructor of this shape :

g = Graph(edges=[ ... ] )

Nathann

CC: @nthiery @rlmill

Component: graph theory

Author: Nathann Cohen

Reviewer: Robert Miller

Merged: sage-4.6.1.alpha1

Issue created by migration from https://trac.sagemath.org/ticket/7709

@nathanncohen nathanncohen mannequin added this to the sage-4.3 milestone Dec 16, 2009
@nathanncohen nathanncohen mannequin assigned rlmill Dec 16, 2009
@nathanncohen nathanncohen mannequin modified the milestones: sage-4.3, sage-4.3.1 Dec 16, 2009
@nathanncohen nathanncohen mannequin added the s: needs work label Jun 6, 2010
@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Aug 2, 2010

comment:4

Here is a patch to update graph.py. I have been sitting a while, trying to find a efficient way to write this, and found none, so I ended up converting it all to a dict_of_lists of dict_of_dicts... As it took some time to write, I would gladly ask for your advice before rewriting it all for digraphs.py (if you agree). Otherwise, let's try to find a better solution together :-)

Nathann

@nathanncohen nathanncohen mannequin added s: needs info and removed s: needs work labels Aug 2, 2010
@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Aug 2, 2010

comment:5

Oh, by the way... This is no a new feature, as it was already possible to create a Graph by giving as an argument a list of edges, but until now it was forwarded to NetworkX..

Nathann

@haraldschilly
Copy link
Member

comment:6

I think data.setdefault(u, []) instead of if not u in data: data[u] = [] could make it a litte bit faster ;-)

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Aug 2, 2010

comment:7

Actually, I wondered... Faster to write, of course, but do you think it is also more efficient ? I had no idea, so I stuck to the most basic tools :-)

Nathann

@haraldschilly
Copy link
Member

comment:8

sorry, forget my comment, data.setdefault(...[]).append(u) is slower than your solution.

@rlmill
Copy link
Mannequin

rlmill mannequin commented Aug 4, 2010

comment:9

I would change the ValueError message to something much shorter and more comprehensive, such as "Edges input must all be of the same format" or length or something...

@rlmill
Copy link
Mannequin

rlmill mannequin commented Aug 4, 2010

comment:10

Replying to @rlmill:

I would change the ValueError message to something much shorter and more comprehensive, such as "Edges input must all be of the same format" or length or something...

Also, the doctests don't expose this error.

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Aug 4, 2010

comment:11

I just updated the ticket to fix it ! Do you think this method is acceptable and I can now do the same for DiGraphs ?

Nathann

@rlmill
Copy link
Mannequin

rlmill mannequin commented Aug 6, 2010

comment:12

Replying to @nathanncohen:

I just updated the ticket to fix it ! Do you think this method is acceptable and I can now do the same for DiGraphs ?

Nathann,

This looks good (maybe in the multiedges=False test you can show the list of edges afterward to demonstrate what actually happens). Please implement this in the DiGraph case as well!

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Aug 8, 2010

comment:13

Here it is ! Actually, the constructor raises an exception when the same edge receives different labels with multiedges = False. I had forgotten to fill the doctests, as it was just a preview !:-)

Nathann

@rlmill
Copy link
Mannequin

rlmill mannequin commented Nov 10, 2010

Author: Nathann Cohen

@rlmill
Copy link
Mannequin

rlmill mannequin commented Nov 10, 2010

Reviewer: Robert Miller

@rlmill
Copy link
Mannequin

rlmill mannequin commented Nov 10, 2010

comment:14

Attachment: trac_7709.patch.gz

@jdemeyer
Copy link

Merged: sage-4.6.1.alpha1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants