#8765: Huffman Encoding
-----------------------------+----------------------------------------------
Reporter: ncohen | Owner: wdj
Type: enhancement | Status: needs_work
Priority: major | Milestone: sage-4.4.1
Component: coding theory | Keywords:
Author: Nathann Cohen | Upstream: N/A
Reviewer: | Merged:
Work_issues: |
-----------------------------+----------------------------------------------
Changes (by leif):
* status: needs_review => needs_work
Comment:
Replying to [comment:13 wdj]:
> This applies to 4.4.rc0 fine and passes sage -testall.
> I have not checked if the documentation builds okay.
We ''should'' do that before giving a positive review, I guess... :)
> Are the other reviewers satisfied with the changes in this last patch?
There are still typos in the description:
* s/occurence/occurrence/
* s/frquency/frequency/
* s/its its/its/ (two times)
* s/occurencies/occurrences/ (two times)
* s/eah/each/
{{{encoding_table()}}}:
s/each letter/each trained letter/
{{{__init__}}}:
It's not tested if '''both''' {{{string}}} and {{{frequencies}}} are
given (=> error).
And as I said before I'd prefer type checks on the parameters (rather than
relying on ''automatically'' raised exceptions later in the code).
I also prefer having the return type documented at the top rather than
(more or less) implicitly in the examples (e.g. in
{{{frequency_table()}}}; {{{tree()}}} actually returns a {{{DiGraph}}}
which happens to also be a tree.)
Doctests?
(Still haven't tested the code, just read the patches, sorry.)
-Leif
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/8765#comment:14>
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.
