#5243: [with patch, needs work] non decreasing parking functions (depend now on
5255)
---------------------------+------------------------------------------------
 Reporter:  hivert         |       Owner:  hivert                        
     Type:  enhancement    |      Status:  assigned                      
 Priority:  minor          |   Milestone:  sage-combinat                 
Component:  combinatorics  |    Keywords:  parking functions / Dyck words
---------------------------+------------------------------------------------

Comment(by ddrake):

 I am working on reviewing this, and I have a couple questions and one
 larger concern. First, some questions. I'm just curious about these
 things:

   * what does the {...@classmethod}}} decorator do?
   * why does the {{{keys}}} method return the domain of the function? It
 seems strange to have a {{{keys}}} method for something list-based, and
 not dictionary-based.

 Here are my concerns:
   * Your code does not check to see if it gets a list of positive
 integers, so you can do {{{NonDecreasingParkingFunction[[0, .1, pi/3,
 sqrt(2)])}}}, yikes! Do we want to require lists of positive integers?
   * In {{{is_a()}}}, you have:
 {{{
    for i in range(len(x)-1):
         if x[i] > x[i+1] or x[i] > i+1:
             return False
 }}}
     Instead of iterating over indices and doing a list lookup every time,
 would it be faster to use Python's {{{enumerate}}} in something like this?
 {{{
    prev = 1
    for i, elt in enumerate(x):
        if prev > elt or elt > i+1:
            return False
        prev = elt
 }}}
     That would also let you finish the function with {{{return True}}},
 since the {{{enumerate}}} loop would check the final element.

   * Right now when you give the {{{NonDecreasingParkingFunction}}}
 constructor a bad list, I see a strange error message:
 {{{
 sage: NonDecreasingParkingFunction([1,1,4])
 ERROR: An unexpected error occurred while tokenizing input
 The following traceback may be corrupted or invalid
 The error message is: ('EOF in multi-line statement', (5, 0))

 ---------------------------------------------------------------------------
 AssertionError                            Traceback (most recent call
 last)

 /home/drake/.sage/temp/klee/10186/_home_drake__sage_init_sage_0.py in
 <module>()

 /var/tmp/sage-3.4.2/local/lib/python2.5/site-
 packages/sage/combinat/non_decreasing_parking_function.pyc in
 __init__(self, lst)
     383             [1, 1, 2, 2, 5, 6]
     384         """
 --> 385         assert(is_a(lst))
     386         CombinatorialObject.__init__(self, lst)
     387

 AssertionError:
 }}}
   * Also, related to {{{is_a}}}: the {{{assert}}} statement should give
 the user some idea of what has gone wrong. I suggest changing the assert
 line (line 409) to {{{assert is_a(lst), 'list is not a non-decreasing
 parking function.'}}}. Also note that {{{assert}}} is a statement, not a
 function -- just like {{{print}}} before Python 3.0.
   * My biggest concern is with the getitem stuff. You are effectively
 shifting the list indices by 1, which really bothers me. Perhaps other
 people don't feel this way, and perhaps this is the best decision, but
 whenever I am thinking of something as a list, I ''really'' want the
 indices to be zero-based, since that's what the rest of Sage/Python does.
 Right now, we have:
 {{{
 sage: f = NonDecreasingParkingFunction([1,1,2,3])
 sage: f[2]
 1
 sage: f[3]
 2
 }}}
     When I use square brackets, I'm thinking "list index", and I really
 want it zero-based. Perhaps we should make these objects callable, so we
 can treat them like functions? I would not mind having {{{f(2)}}} be 1 and
 {{{f(3)}}} be 2, since the round parentheses indicate a function call, and
 indeed the above object f is a function that sends 3 to 2.

 I'm marking this "needs work" because of the list index issue. I have a
 patch which fixes a bunch of tiny docstring bits which I'll upload once we
 have the rest of this sorted out.

-- 
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/5243#comment:9>
Sage <http://sagemath.org/>
Sage - Open Source Mathematical Software: Building the Car Instead of 
Reinventing the Wheel

--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---

Reply via email to