#6828: [with patch, needs review] Random Bipartite Graph
--------------------------+-------------------------------------------------
Reporter: ncohen | Owner: rlm
Type: enhancement | Status: new
Priority: minor | Milestone: sage-4.1.2
Component: graph theory | Keywords:
Reviewer: | Author:
Merged: |
--------------------------+-------------------------------------------------
Comment(by rbeezer):
Hi Nathann,
I like the new names for the vertices, and it looks much better for graphs
with low edge probability the way you are doing it now. Some more
comments:
1. Checking for errors usually involves raising an error, rather than
using an assert. Poke around in the code and I think you will see more
often a style like:
{{{
if not((p>=0 and p<=1):
raise ValueError, "Parameter p is a probability, and so should be a real
value between 0 and 1"
}}}
I'd place them after the imports, but that's just me.
2. You need to paste in the output of your test. Right now it is
failing. You really should make sure all your tests pass before someone
stops in to do a review.
3. You should also test the two checks on the input - in a section called
{{{TESTS}}}
4. Your EXAMPLE section needs to have two semicolons to create the
verbatim section, so at least you need a {{{::}}} on a line by itself
prior to each test. (Maybe two semicolons right after EXAMPLE will work
as well - I'm having trouble checking this right now with my setup.) This
ensures you get the right formatting in the reference manual. Again, take
a look at what is done elsewhere in the source and compare with the
output, then make sure your output also works properly before seeking a
review.
I think most of the above applies to #6823, which I haven't commented on.
Adding the Odd graphs there is great, though.
Rob
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/6828#comment:3>
Sage <http://sagemath.org/>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to
[email protected]
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en
-~----------~----~----~----~------~----~------~--~---
