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

random_vertex and random_edge functions #7569

Closed
nathanncohen mannequin opened this issue Dec 1, 2009 · 15 comments
Closed

random_vertex and random_edge functions #7569

nathanncohen mannequin opened this issue Dec 1, 2009 · 15 comments

Comments

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Dec 1, 2009

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

Here it is !

Nathann

CC: @sagetrac-abmasse

Component: graph theory

Author: Nathann Cohen

Reviewer: Alexandre Blondin Massé

Merged: sage-4.4.alpha0

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

@nathanncohen nathanncohen mannequin added this to the sage-4.4 milestone Dec 1, 2009
@nathanncohen nathanncohen mannequin assigned rlmill Dec 1, 2009
@nathanncohen

This comment has been minimized.

@rlmill
Copy link
Mannequin

rlmill mannequin commented Mar 2, 2010

comment:5

All patches must include doctests, especially new functions.

@rlmill rlmill mannequin added s: needs work and removed s: needs review labels Mar 2, 2010
@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Mar 2, 2010

comment:6

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

Nathann

@rlmill
Copy link
Mannequin

rlmill mannequin commented Mar 4, 2010

comment:7

Replying to @nathanncohen:

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)
(...,...,...)

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Mar 4, 2010

comment:8

Got it !

Here is the new version :-)

Nathann

@sagetrac-abmasse
Copy link
Mannequin

sagetrac-abmasse mannequin commented Mar 21, 2010

comment:9

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.

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Mar 22, 2010

comment:11

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

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Mar 22, 2010

Attachment: trac_7569.patch.gz

@sagetrac-abmasse
Copy link
Mannequin

sagetrac-abmasse mannequin commented Mar 22, 2010

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

@sagetrac-abmasse
Copy link
Mannequin

sagetrac-abmasse mannequin commented Mar 22, 2010

Author: Nathann Cohen

@sagetrac-abmasse
Copy link
Mannequin

sagetrac-abmasse mannequin commented Mar 22, 2010

Reviewer: Alexandre Blondin Massé

@sagetrac-abmasse
Copy link
Mannequin

sagetrac-abmasse mannequin commented Mar 22, 2010

comment:12

Attachment: trac_7569_review-abm.patch.gz

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.

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Mar 22, 2010

comment:13

Thank you very much again :-)

Nathann

@jhpalmieri
Copy link
Member

Merged: sage-4.4.alpha0

@jhpalmieri
Copy link
Member

comment:14

Merged in 4.4.alpha0:

  • trac_7569.patch
  • trac_7569_review-abm.patch

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

1 participant