-3

The program is supposed to use quick select and return the median of a set of integer values.

Question: When I run the program, it tells me that k is not defined. How should I define k to get the median?

def quickSelect(lines,k):
    if len(lines)!=0:
        pivot=lines[(len(lines)//2)]
        smallerlist=[]
        for i in lines:
            if i<pivot:
                smallerlist.append(i)
        largerlist=[]
        for i in lines:
            if i>pivot:
                largerlist.append(i)
        m=len(smallerlist)
        count=len(lines)-len(smallerlist)-len(largerlist)
        if k >= m and k<m + count:
            return pivot
        elif m > k:
            return quickSelect(smallerList,k)
        else:
            return quickSelect(largerList, k - m - count)
Hugh Bothwell
  • 55,315
  • 8
  • 84
  • 99
Learner
  • 57
  • 2
  • 6
  • 3
    Please post the trace of any error you're seeing. And please test the code in your question to make sure it reproduces the error you describe in your question. – Bi Rico Mar 04 '14 at 22:45
  • @bvidal: _don't fix the error in the question text!_ - it makes it impossible for anyone else to see what the problem was. – Hugh Bothwell Mar 05 '14 at 02:54
  • @HughBothwell: the typo in the variable names wasn't the error referenced in the post. Having the real error from the OP would actually help. – El Bert Mar 05 '14 at 14:13

3 Answers3

0

The code seems to be working fine, after a minor correction. smallerlist and largerlist had a typo.

        elif m > k:
            return quickSelect(smallerList,k)
        else:
            return quickSelect(largerList, k - m - count)

should be changed by

        elif m > k:
            return quickSelect(smallerlist,k)
        else:
            return quickSelect(largerlist, k - m - count)

This is the final corrected code, which runs just fine.

def quickSelect(lines,k):
    if len(lines)!=0:
        pivot=lines[(len(lines)//2)]
        smallerlist=[]
        for i in lines:
            if i<pivot:
                smallerlist.append(i)
        largerlist=[]
        for i in lines:
            if i>pivot:
                largerlist.append(i)
        m=len(smallerlist)
        count=len(lines)-len(smallerlist)-len(largerlist)
        if k >= m and k<m + count:
            return pivot
        elif m > k:
            return quickSelect(smallerlist,k)
        else:
            return quickSelect(largerlist, k - m - count)

Hope it helps

mrcl
  • 2,130
  • 14
  • 29
  • As I sad there was a typo in your code. smallerlist was written as smalleList (with list written with capital L) at the parts I have indicated. The same applies to largerlist. After changing those typos the code ran just fine. – mrcl Mar 04 '14 at 23:11
  • Ok I was trying to help, the only problem in your code is the typo. I don't think it deserves to be voted down for that. – mrcl Mar 04 '14 at 23:20
  • Anyway, by running quickSelect([1,2,3,4,5],3) I get no errors, just the answer 4, as I said previously. The variable k is defined properly. No mistakes there! I pointed out that I couldn't reproduce your error because it did not exist, since k is defined at the functions arguments. The only error was simply the typo and I just pointed out. But whatever. – mrcl Mar 04 '14 at 23:39
-1

You need to initializing k for the first time into the function. It should be the position of the item you are looking for (if the list was sorted). Default it to half the list length, for median. Call it like so:

k = len(lines) // 2
x = quickSelect(lines, k)

or if you only ever want the median, you could fix the function so you don't have to provide the index of the item you want

def quickSelect(lines, k=None):
    if k is None:
        k = len(lines)//2

As Hugh pointed out, this function will only select an element of the list. For a median of an even number of elements, the median should actually be the mean of the middle two elements.

cmd
  • 5,754
  • 16
  • 30
  • `k` is not the problem. – Hugh Bothwell Mar 05 '14 at 03:01
  • @HughBothwell The OP's question was about `k`. Sure there are other problems, that he will find once he knows how to call it. – cmd Mar 05 '14 at 15:26
  • the OP _mistakenly said_ the error was about k. If you run the code, it _actually_ complains about smallerList. – Hugh Bothwell Mar 05 '14 at 15:29
  • @HughBothwell you are assuming that he was actually able to call it? I *have* run the code and realize the problems Hugh, but that was *not* the question. He probably is reporting the error correctly if he was calling `quickSelect(lines, k)` – cmd Mar 05 '14 at 15:33
  • (banging head on table) after re-reading: you are right - he wants to know which k to use to get median (and he's going to have a rough time if he has an even number of lines). – Hugh Bothwell Mar 05 '14 at 15:35
-1

You misreported the error; it doesn't complain about k, it complains about smallerList because you defined smallerlist (lower-case-l) and then tried to call it with an uppercase-L. Python variables are case-sensitive, ie smallerlist is smallerList == False.

Looking at your code:

def quickSelect(lines, k):
    if len(lines) != 0:

len(lst) != 0 is non-idiomatic; PEP-8 says it should just be lst, as in if lst:. Also, camelCase is unPythonic; your function name should be quick_select. lines implies you can only operate on text, but your function should work just as well on any orderable data type, so items would be more accurate. You should have a docstring so the next person to come along has some idea what it does, and we're going to call len(items) again, so we may as well do it once and store the result. Finally, what if k > len(items)?

def quick_select(items, k):
    """
    Return the k-from-smallest item in items

        Assumes 0 <= k < len(items)
    """
    num_items = len(items)
    if 0 <= k < num_items:
        pivot = items[num_items // 2]

continuing:

        smallerlist = []
        for i in lines:
            if i<pivot:
                smallerlist.append(i)
        largerlist=[]
        for i in lines:
            if i>pivot:
                largerlist.append(i)

You've iterated through lines twice; you could combine this into a single pass. Also, better variable names:

        smaller, larger = [], []
        for item in items:
            if item < pivot:
                smaller.append(item)
            elif item > pivot:
                larger.append(item)

continuing with better variable names,

        num_smaller = len(smaller)
        num_pivot = num_items - num_smaller - len(larger)

then your ifs are out of order; they are easier to read in order, so

        if k < num_smaller:
            return quick_select(smaller, k)
        elif k < num_smaller + num_pivot
            return pivot
        else:
            return quick_select(larger, k - num_smaller - num_pivot)

then what if k < 0 or k >= num_items?:

    else:
        raise ValueError("k={} is out of range".format(k))

Finally, because this function is tail-recursive, it is easy to convert to an iterative function instead:

        while True:
            pivot = items[num_items // 2]

            # ...

            if k < num_smaller:
                items = smaller
                num_items = num_smaller
            elif k < num_smaller + num_pivot
                return pivot
            else:
                items = larger
                num_items = num_larger
                k -= num_smaller + num_pivot

... some assembly required, hope that helps!

Hugh Bothwell
  • 55,315
  • 8
  • 84
  • 99