Opened 10 years ago

Closed 9 years ago

#7569 closed enhancement (fixed)

random_vertex and random_edge functions

Reported by: ncohen Owned by: rlm
Priority: major Milestone: sage-4.4
Component: graph theory Keywords:
Cc: abmasse Merged in: sage-4.4.alpha0
Authors: Nathann Cohen Reviewers: Alexandre Blondin Massé
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by ncohen)

In many algorithms we want to find a random vertex or a random edge in a graph.

Here it is !

Nathann

Attachments (2)

trac_7569.patch​ (2.1 KB) - added by ncohen 9 years ago.
trac_7569_review-abm.patch​ (2.6 KB) - added by abmasse 9 years ago.
Review patch with formatting of code and doc -- apply on top of Nathann's patch

Download all attachments as: .zip

Change History (16)

comment:1 Changed 9 years ago by ncohen

  • Description modified (diff)
  • Status changed from new to needs_review

comment:2 Changed 9 years ago by ncohen

  • Status changed from needs_review to positive_review

comment:3 Changed 9 years ago by ncohen

  • Status changed from positive_review to needs_work

comment:4 Changed 9 years ago by ncohen

  • Status changed from needs_work to needs_review

comment:5 Changed 9 years ago by rlm

  • Status changed from needs_review to needs_work

All patches must include doctests, especially new functions.

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

I agree, but I did not know how to comment a random generator.... How would you do that ? ;-)

Nathann

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

Replying to ncohen:

I agree, but I did not know how to comment a random generator.... How would you do that ? ;-)

Nathann

There are several ways, e.g. 

sage: v = G.random_vertex()
sage: v in G
sage: G.has_vertex(v)
True

etc. You can also do 

sage: G.random_edge(labels=False)
(...,...)
sage: G.random_edge(labels=True)
(...,...,...)

comment:8 Changed 9 years ago by ncohen

  • Status changed from needs_work to needs_review

Got it !

Here is the new version :-)

Nathann

comment:9 Changed 9 years ago by abmasse

Hello, Nathann ! I guess this patch won't be hard to review. I have a question, though. Why do you have the **kwds parameter ? It seems to me that there shouldn't be any parameter at all. Or if you want to keep the same parameters as for the vertex_iterator and edge_iterator functions, wouldn't it be better to use the same names (vertices for vertex_iterator for instance) ? What do you think ? If you still prefer to keep the **kwds argument, then I think it would be better to indicate at least that these are passed to the vertex_iterator and edge_iterator functions.

comment:10 Changed 9 years ago by abmasse

  • Cc abmasse added

comment:11 Changed 9 years ago by ncohen

Hello !!

I added this parameter because I can not stand the fact that Graph.edges() returns triples instead of pairs, so I constantly use the labels = False argument :-)

Patch updated !

Nathann

Changed 9 years ago by ncohen

Changed 9 years ago by abmasse

Review patch with formatting of code and doc -- apply on top of Nathann's patch

comment:12 Changed 9 years ago by abmasse

  • Authors set to Nathann Cohen
  • Reviewers set to Alexandre Blondin Massé
  • Status changed from needs_review to positive_review

I've tested this patch on sage 4.3.4. All tests passed, and the documentation generated with the browse_sage_doc function looks ok (we really need to include those graph files in the tree for the reference manual). I made a few changes, only consisting of formatting of code and documentation.

Positive review.

comment:13 Changed 9 years ago by ncohen

Thank you very much again :-)

Nathann

comment:14 Changed 9 years ago by jhpalmieri

  • Merged in set to sage-4.4.alpha0
  • Resolution set to fixed
  • Status changed from positive_review to closed

Merged in 4.4.alpha0:

  • trac_7569.patch
  • trac_7569_review-abm.patch
Note: See TracTickets for help on using tickets.