#18594: Add additional mutation options for clusters
-------------------------------------+-------------------------------------
       Reporter:  aram.dermenjian    |        Owner:
           Type:  enhancement        |       Status:  needs_review
       Priority:  major              |    Milestone:  sage-6.8
      Component:  combinatorics      |   Resolution:
       Keywords:  SageDays64.5,      |    Merged in:
  clusters, mutation, seeds, quiver  |    Reviewers:  Viviane Pons,
        Authors:  Aram Dermenjian    |  Christian Stump
Report Upstream:  N/A                |  Work issues:
         Branch:                     |       Commit:
  u/aram.dermenjian/add_additional_mutation_option_for_clusters|  
fd588d4d2e8d45b0335ed196dc8b496397d42656
   Dependencies:                     |     Stopgaps:
-------------------------------------+-------------------------------------
Changes (by stumpc5):

 * reviewer:  Viviane Pons => Viviane Pons, Christian Stump


Comment:

 Some more comments:

 * Looking at the diff's

   {{{git diff develop
 src/sage/combinat/cluster_algebra_quiver/quiver.py}}}

   I see that you introduce tons of trailing whitespaces (both in
 {{{quiver.py}}} and in {{{cluster_seed.py}}})

 * {{{quiver.py}}} has changed without acknowledgements in the author
 fields.

 * all tests pass, great!

 * 100% coverage, great!

 * How do you ensure that {{{first_sink}}} and {{{first_source}}} actually
 find the *first*, and what is the **first** (since the vertices are not
 necessarily labelled by something in a total order)? Do you mean to use
 {{{a_sink}}} and {{{a_source}}}?

 I don't have the time these days to actually play with the code, but I
 hope (and expect) that you provided well explained and good doctests!
 (That at least seems to be the case in the few examples I looked at.) So
 from my side, this is ready to go.

 On the other hand, I would like to have Salvatore and/or Dylan to also
 look at it again, since they will be modifying the mutation methods
 afterwards...

 Cheers, Christian

--
Ticket URL: <http://trac.sagemath.org/ticket/18594#comment:23>
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.

Reply via email to