Opened 10 years ago

Closed 9 years ago

#7709 closed enhancement (fixed)

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

Reported by: ncohen Owned by: rlm
Priority: major Milestone: sage-4.6.1
Component: graph theory Keywords:
Cc: nthiery, rlm Merged in: sage-4.6.1.alpha1
Authors: Nathann Cohen Reviewers: Robert Miller
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

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

Attachments (1)

trac_7709.patch (9.9 KB) - added by ncohen 9 years ago.

Download all attachments as: .zip

Change History (16)

comment:1 Changed 10 years ago by ncohen

  • Milestone changed from sage-4.3 to sage-4.3.1

comment:2 Changed 9 years ago by ncohen

  • Status changed from new to needs_work

comment:3 Changed 9 years ago by ncohen

  • Cc nthiery added

comment:4 Changed 9 years ago by ncohen

  • Cc rlm added
  • Status changed from needs_work to needs_info

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

comment:5 Changed 9 years ago by ncohen

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

comment:6 Changed 9 years ago by schilly

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

comment:7 Changed 9 years ago by ncohen

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

comment:8 Changed 9 years ago by schilly

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

comment:9 follow-up: Changed 9 years ago by rlm

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

comment:10 in reply to: ↑ 9 Changed 9 years ago by rlm

Replying to rlm:

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.

comment:11 follow-up: Changed 9 years ago by ncohen

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

comment:12 in reply to: ↑ 11 Changed 9 years ago by rlm

Replying to ncohen:

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!

comment:13 Changed 9 years ago by ncohen

  • Status changed from needs_info to needs_review

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

Changed 9 years ago by ncohen

comment:14 Changed 9 years ago by rlm

  • Authors set to Nathann Cohen
  • Reviewers set to Robert Miller
  • Status changed from needs_review to positive_review

comment:15 Changed 9 years ago by jdemeyer

  • Merged in set to sage-4.6.1.alpha1
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.