#10530: De Bruijn Sequence construction for combinat
---------------------------------+------------------------------------------
Reporter: eviatarbach | Owner: eviatarbach
Type: enhancement | Status: needs_review
Priority: major | Milestone: sage-4.7
Component: combinatorics | Keywords:
Work_issues: | Upstream: N/A
Reviewer: Nicolas M. Thiéry | Author: Eviatar Bach
Merged: | Dependencies:
---------------------------------+------------------------------------------
Comment(by ncohen):
Hello again !
* doctest coverage still isn't to 100% (even though only the __init__
is missing.
* The first line in the docstring of a method should be a description
of what it does. It is missing in many methods.
* It would be nice to add a formal definition of what a DeBruijn
sequence is. A "sequence that incorporates all substrings of a certain
length of an alphabet" is a bit vague. And even when it is made clear that
they are contiguous substrings, there is something to mention about the
fact that is actually does *not* contain them all, are one has to assume
there is a sequence of trailing zeroes.
{{{
sage: DeBruijnSequences(2,3).an_element()
[0, 0, 0, 1, 0, 1, 1, 1]
}}}
Here the sequences 110 and 100 do not appear, except when
considering trailing zeroes, and even though I think this is what is
called a de Bruijn sequence, it has to be made clear somewhere (perhaps
the best is in ``DeBruijnSequences?``).
* We usually push the TESTS section of each docstring to the end, as
it is usually long and the least relevant to the user.
* References (bibliography) are still wrong
* Couldn't the debruijn_sequence function (in Cython) be improved by
defining the recursive method outside, as a cdef function ? Here I fear
the recursive calls are made in Python.
* I remember Minh telling me that we should stick to
{{{
raise ValueError("message")
}}}
instead of
{{{
raise ValueError, "message"
}}}
It has something to do with the next version of Python that we may
have to use somewhere in the distant future.
* There is a warning when building documentation
{{{
/home/ncohen/.Sage/local/lib/python2.6/site-
packages/sage/combinat/debruijn_sequences.py:docstring of
sage.combinat.debruijn_sequences.DeBruijnSequences:4: (WARNING/2) Bullet
list ends without a blank line; unexpected unindent.
}}}
Nathann
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/10530#comment:19>
Sage <http://www.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.
