1

I have a function that takes a list and removes (one instance of) the smallest and largest elements, then takes the average of the remaining elements. Running it doesnt bring up any errors, although on checking the results I realised they were incorrect. Here's the program:

def centered_average(x):
    x.sort()
    y = 0
    for i in x:
        if x.index(i) == 0 or x.index(i) == (len(x)-1):
            print(i, "is being removed")
            x.remove(i)
            i +=1
        else:
            y += i
            print(i, "is being added")
    return (y / len(x))

def average(x):
    return sum(x)/len(x)

(the print functions were put in for checking)

On putting through a list of

x = [1,2,3,4,5]

the result was:

1 is being removed
3 is being added
4 is being added
5 is being removed
2.3333333333333335

therefore, we can assume x[1] is not being used in the function, and I would like to know why.

Thanks in advance for any help.

Harrison
  • 5,095
  • 7
  • 40
  • 60
incapable moron
  • 13
  • 2
  • What is the i +=1 supposed to do ? (I mean what do you think it should do) – Julien Aug 26 '16 at 14:30
  • Your question is clearly lacking a central thing: what is it that you *want* to do? There's a couple of issues that you should fix with your code, but it's no use explaining them without knowing what you wanted to do in the first place. – Marcus Müller Aug 26 '16 at 14:32
  • Im not sure why i put it in, but removing it returns the same result – incapable moron Aug 26 '16 at 14:33
  • Questions seeking debugging help ("why isn't this code working?") must include the **desired behavior**, a **specific problem** or error and the shortest code necessary to reproduce it in the question itself. Questions without a clear problem statement are not useful to other readers. See: How to create a Minimal, Complete, and Verifiable example. – Marcus Müller Aug 26 '16 at 14:33
  • 3
    Your indentation is all off as well. – Harrison Aug 26 '16 at 14:33
  • 1
    I think you're misunderstanding what `for i in x:` does. – Marcus Müller Aug 26 '16 at 14:34
  • Marcus Muller I want it to recognize the second list element and print "[second element] is being added" – incapable moron Aug 26 '16 at 14:35
  • the indentation is just due to me incorrectly typing it in to stack overflow. sorry (i guess youre referring to the return statement, which is correctly indented on my file) – incapable moron Aug 26 '16 at 14:37
  • 2
    it doesn't matter when your indentation got damaged, @incapablemoron , you need to repair it for display, since wrong indentation makes it impossible to say what your program actually does in python – Marcus Müller Aug 26 '16 at 14:38
  • No, the entire body of `centered_average(x)` is improperly indented, not just the `return` statement. My answer below should hopefully help you understand why you're seeing the second element in your **original** list be skipped – dblclik Aug 26 '16 at 14:39
  • He did say just to remove 1 element of the Min() and Max(), so he's really just talking about a list that is `x[1:-1]` – dblclik Aug 26 '16 at 14:40
  • Possible duplicate of [strange result when removing item from a list](http://stackoverflow.com/questions/6260089/strange-result-when-removing-item-from-a-list) – Two-Bit Alchemist Aug 26 '16 at 14:42
  • 1
    @dblclik and what if the last element is in there twice? `[1,2,3,4,4]`; `x.index(4) == 3`!! – Marcus Müller Aug 26 '16 at 14:42
  • sorry about the overall indentation errors. yes, i realise that the entire function block needs to be indented. I'm sorry it is not – incapable moron Aug 26 '16 at 14:43
  • @MarcusMüller, direct quote: _"I have a function that takes a list and removes (one instance of) the smallest and largest elements"_, they just wants the first and last element of the sorted list removed. I don't think it's analytically "good", but it's what was asked for – dblclik Aug 26 '16 at 14:44
  • @dblclik nope, doesn't work; if you check for the index of `4` in my example, it will never be `len(x)-1`, because `list.index` always returns the *first* occurrence – Marcus Müller Aug 26 '16 at 14:45
  • @MarcusMüller, you're right, but it's the wrong way to solve the problem of removing one element of both the `min()` and `max()`. Plus, if you used your solution it could still work by creating `x_new = [x[i] for i in range(len(x)) if i not in [x.index(min(x)),x.index(max(x))]]` – dblclik Aug 26 '16 at 14:48
  • Marcus Muller, to be clear, although I am taking the highest and lowest values, I am taking only one of each. Without a loop with a count etc., sorting the list and then removing the first and last values achives the same result, although it causes the error that I am here asking about. Thanks for your input though. – incapable moron Aug 26 '16 at 14:48
  • @incapablemoron see my comments above, your method falls flat if the last element appears twice. – Marcus Müller Aug 26 '16 at 14:50

2 Answers2

3

You need to take special care when removing elements of a list that you are already iterating over in Python, as you are doing in your function. Python is looking at the indices of the list and using those, but when you remove the first element and the list is updated in place, the previous second element (x[1]) is now the first (x[0]) and will thus be skipped.

There is an easier way to do this, though, that doesn't require such a loop and the extra conditionals, if all you want to do is take the average of the elements that aren't the first or last:

def centered_average(x):
    x.sort()
    if len(x) <= 2:
        print "Cannot run with 2 or fewer elements..."
        return 0
    else:
        return sum(x[1:-1])/(len(x)-2.00)

There are other ways to do this, but this one should be fast enough and allow for any case you provide as long as x is a list. Hope this helps.

dblclik
  • 406
  • 2
  • 8
  • oh ok that makes sense. I was advised my error was to do with the remove() part, but I wasnt sure how. Thanks a lot! – incapable moron Aug 26 '16 at 14:39
  • No problem, if this works for your code please Accept the answer so the question shows as resolved and does not continue to draw discussion if you're finished with it. – dblclik Aug 26 '16 at 14:41
1

The issue comes from the for i in x:. When you remove the first element, the second element (2) becomes the first element of the list. This means that on the next iteration, you will look for the second element, and find 3, because the list is now [2, 3, 4, 5].

Instead, you could start by removing the first and last element with x=x[1:-1]. If you've never seen the [i:-j] syntax before, this tells Python to return a list starting at index i, and ending j indices from the end. In other words, this will produce [2, 3, 4] in your example. Afterwards, you could return sum(x)/len(x).

Note that neither this code, or your original approach, will work on lists of less than 3 elements. When dividing by len(x) in either solution, you will end up dividing by 0.

ScottWe
  • 190
  • 1
  • 1
  • 10