Opened 9 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 )
In many algorithms we want to find a random vertex or a random edge in a graph.
Here it is !
Nathann
Attachments (2)
Change History (16)
comment:1 Changed 9 years ago by
- Description modified (diff)
- Status changed from new to needs_review
comment:2 Changed 9 years ago by
- Status changed from needs_review to positive_review
comment:3 Changed 9 years ago by
- Status changed from positive_review to needs_work
comment:4 Changed 9 years ago by
- Status changed from needs_work to needs_review
comment:5 Changed 9 years ago by
- Status changed from needs_review to needs_work
comment:6 follow-up: ↓ 7 Changed 9 years ago by
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
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
- Status changed from needs_work to needs_review
Got it !
Here is the new version :-)
Nathann
comment:9 Changed 9 years ago by
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
- Cc abmasse added
comment:11 Changed 9 years ago by
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
Changed 9 years ago by
Review patch with formatting of code and doc -- apply on top of Nathann's patch
comment:12 Changed 9 years ago by
- 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
Thank you very much again :-)
Nathann
comment:14 Changed 9 years ago by
- 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
All patches must include doctests, especially new functions.