#19160: Add a new class SnakeGraphs and SnakeGraph in
combinat/cluster_algebra_quiver
-------------------------------------+-------------------------------------
Reporter: egunawan | Owner:
Type: enhancement | Status: needs_work
Priority: major | Milestone: sage-6.9
Component: combinatorics | Resolution:
Keywords: snake graph, | Merged in:
cluster algebra, days69 | Reviewers: Travis Scrimshaw,
Authors: egunawan | mlapointe
Report Upstream: N/A | Work issues:
Branch: u/egunawan/19160 | Commit:
Dependencies: | dd7241b32660b018c8575e80d31e5fbf6d1cf922
| Stopgaps:
-------------------------------------+-------------------------------------
Changes (by tscrim):
* status: needs_review => needs_work
* reviewer: tscrim, mlapointe => Travis Scrimshaw, mlapointe
Comment:
Thank you for your work on this.
Some things that need to be addressed from a quick glance:
- Full doctest coverage (`shape`).
- `print 'print an error message here TODO'` in `_element_constructor_`
needs to raise an actual error.
- `The snake graphs` -> `Snake graphs`.
- You should include more information in the `SnakeGraphs`'s class level
docstring.
- Remove the module variables `RIGHT` and `UP`; just use the python
strings (I've never noticed a speed decrease; <technobabble>in fact,
python probably keeps string instances in a pool and/or has special
compiler behavior for hardcoded strings</technobabble>).
- You might consider using `ClonableArray` instead of `Element` since the
former gives you list-like behavior for free (and by handling the internal
storage in python, will be faster) and is a subclass of `Element`.
- You should explain the association between your internal data structure
and the behavior of the snake graph (i.e., what integer makes the graph go
in which direction?). This can be in python comments, not necessarily in
the docstrings.
Looks pretty, and I like the ascii art.
For the ticket on trac, you should use real names for author and reviewer.
--
Ticket URL: <http://trac.sagemath.org/ticket/19160#comment:4>
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 unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/d/optout.