#10266: Add sum of digits of a sequence to the word constructor
-------------------------------+--------------------------------------------
Reporter: slabbe | Owner: slabbe
Type: enhancement | Status: needs_work
Priority: major | Milestone: sage-4.6.1
Component: combinatorics | Keywords:
Author: Sébastien Labbé | Upstream: N/A
Reviewer: | Merged:
Work_issues: |
-------------------------------+--------------------------------------------
Changes (by abmasse):
* status: needs_review => needs_work
Comment:
Hi Sébastien!
Some remarks
1. I'm not sure that this generator should be placed here. It sounds a
lot like the `delta` and `partial_sums` functions for the infinite words
class. Shouldn't it be placed there along with these functions? Another
argument in that direction: to be more coherent with the object-oriented
approach, it seems to me that the parameter `seq` should be `self`, i.e.
the function should belong to the word/sequence `seq`.
1. If you don't agree with the first comment, I suggest to name the
function `SumDigitsWord`.
1. One of the parameter is `m`, I suggest replacing it with `mod` which
is more significant.
1. Is there any reason why the `m` parameter allows one to relabel the
word with new letters? I don't think the function `SumDigits` should be
taking care of that translation.
1. For the Thue-Morse word example, I would add a line that checks if
the first, say 500 letters, are indeed the same.
1. Very minor remark, but in the first sentence of the docstring, I
would replace "mod" with "modulo".
That's it for now!
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/10266#comment:2>
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.
