#4859: [with patch, needs work] basic covering design module
---------------------------+------------------------------------------------
Reporter: dgordon | Owner: mhansen
Type: enhancement | Status: new
Priority: major | Milestone: sage-3.3
Component: combinatorics | Resolution:
Keywords: |
---------------------------+------------------------------------------------
Changes (by was):
* milestone: sage-3.4.1 => sage-3.3
Comment:
REFEREE REPORT:
The overall style of the code looks really good.
* This worries me:
{{{
82 bound = 1.0
83 for i in range(t-1,-1,-1):
84 bound = (bound*RDF(v-i)/RDF(k-i)).ceiling()
85
86 return bound
}}}
I'm worried about overflow. Maybe using interval arithmetic would be
better, i.e., the real interval field. You could make a comment about why
precision isn't an issue, but I sort of wonder.
* I think it might be nice to avoid confusion if you make all the
attributes private (i.e., put an underscore (or two) in the beginning of
their names), then have methods to access them, e.g.,
{{{
This is bad:
sage: C=CoveringDesign(7,3,2,7,range(7),[[0, 1, 2], [0, 3, 4], [0, 5, 6],
[1, 3, 5], [1, 4, 6], [2, 3, 6], [2, 4, 5]],0, 'Projective Plane')
sage: C.method
'Projective Plane'
sage: C.method = 'foo bar'
sage: C.method
'foo bar'
This is better:
sage: C.method()
'Projective Plane'
sage: C.method?
lots of nice documentation about what the C.__method *means* and how to
use it.
}}}
Do the same with all the other attributes. This is for consistency with
the rest of Sage, and because it is easier for users.
* You defined a {{{show}}} method. This is reserved for graphical
display. Instead call that method {{{_repr_}}}, so it gets automatically
picked up by the print and str(...) methods. Also {{{_repr_}}} should
return a string instead of using the print command.
* change {{{ requires internet, optional }}} to {{{optional -- requires
internet}}}. Doing that isn't documented anywhere, but it's the "new way"
now that I wrote an optional doctesting framework that really singles out
various components.
With the above issues addressed, this code should go into sage.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/4859#comment:4>
Sage <http://sagemath.org/>
Sage - Open Source Mathematical Software: Building the Car Instead of
Reinventing the Wheel
--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---
