3

I want a function that takes a list and returns that list with any elements less than 0 or greater than "upper" removed.

I figured out how to do it with a list comprehension, but I can't figure out why this didn't work:

dim = 4

def ensure_values(l, upper=dim**2):
    for i in l:
        if i < 0 or i >= upper:
            l.remove(i)
    return l

l = [0,2,-3,5]

ensure_values(l)
[0,2,-3,5]

I expected [0,2,5].

capybaralet
  • 1,757
  • 3
  • 21
  • 31
  • 3
    The list is changing while you are modifying it, causing the iteration to skip values. List comprehension is better in this case, but if you want, you can build up a list of values to remove, then remove those values later. – swstephe Mar 26 '14 at 22:35
  • It does work. Start a new session, copy paste your code, it works. Plus what @swstephe said: use a copy `l2=l[:]` for instance, and remove from l2, and return l2. – fredtantini Mar 26 '14 at 22:36
  • It works as long as you only have one out-of-range value in the list. – kojiro Mar 26 '14 at 22:36
  • fredtantini - you are right. In ipython, however, it does in fact output [0,2,-3,5], which is bizarre. – capybaralet Mar 26 '14 at 22:58
  • swstephe - that's sort of what I figured must be going on, but the syntax is very suggestive: it reads as iteration over items, not over indices. So I figured, removing an item that has already been through the iteration shouldn't affect the rest, but I guess that's not how it works. – capybaralet Mar 26 '14 at 22:58
  • Modifying an item while looping over the item plunges you into implementation-specific details. You really can't be sure what will happen. It's simplest and best simply to create a new list that contains just what you want it to contain. – steveha Mar 26 '14 at 23:12

3 Answers3

3

Best practice in Python would be for your function to return a new list that contains only the desired elements. This is best done with a list comprehension.

dim = 4

def ensure_values(lst, upper=dim**2):
    return [n for n in lst if 0 < n <= upper]

lst = [0,2,-3,5]

assert ensure_values(lst) == [2, 5]

Note that I was able to use the expression 0 < n <= upper. That's not legal in C or most other languages, but in Python it is legal and does what one would expect it to do.

Also, I recommend against using l as a variable name. It looks almost exactly like a 1 or an I in many fonts. Recommended practice is to use lst or L for a short name for a list.

The use of l as a variable name is specifically dis-recommended in the PEP 8 coding style guidelines for Python. Here's a link:

http://legacy.python.org/dev/peps/pep-0008/#id29

steveha
  • 74,789
  • 21
  • 92
  • 117
1

You could hange your loop so it iterates over a copy of the list, which avoids modifying the list you're iterating over, because that won't work in many cases.

dim = 4

def ensure_values(l, upper=dim**2):
    for i in l[:]:  # iterate over copy of list
        if i < 0 or i >= upper:
            l.remove(i)
    return l

l = [0, 2, -3, 20, 5]

print ensure_values(l)  # -> [0, 2, 5]

A more "Pythonic" way to do it — because it's shorter and doesn't need a copy of the list — would be to use a list comprehension. Note the condition had to be reversed because it's now being used to determine when to keep elements.

l = [i for i in l if i >= 0 and i < dim**2]

print l  # -> [0, 2, 5]
martineau
  • 119,623
  • 25
  • 170
  • 301
  • This will work but it's not best practice. It would be better to make a new list with only the desired elements, rather than mutating the original list in place. And in Python it is preferred to return `None` from any function that mutates an argument, because of "Command-Query Separation". http://stackoverflow.com/questions/9777122/why-does-sort-cause-the-list-to-be-none-in-python – steveha Mar 26 '14 at 23:03
  • @steveha: While I agree, that's not what the OP asked for. IMHO, the best way is to not use a function at all and instead use a list-comp as shown in my updated answer. – martineau Mar 26 '14 at 23:15
  • @steveha: P.S. One argument against returning `None` instead of the mutated argument is that it will mean the result can't be chained with another operation -- so there may be other considerations. – martineau Mar 26 '14 at 23:19
  • I am well aware that returning `None` means you can't chain with the result. That is one of the reasons why best practice is to return a new list containing only the desired stuff. In Python, all the built-in stuff obeys Command-Query Separation and it is very strongly disrecommended to break this. – steveha Mar 26 '14 at 23:25
  • @steveha: Oh goodness, let us hope no one ever fails to follow a **recommendation**... – martineau Mar 26 '14 at 23:34
  • It is better to follow a recommendation than not, *unless* there is a compelling benefit to be gained. What compelling benefit is there to writing this function to work differently than every built-in function in Python that mutates an object? I know that when I get an object back from a function, I expect that the function didn't mutate its argument... what benefit do you derive from breaking my expectation? I am not the boss of you, so you may do as you wish and it doesn't matter to me, but when I'm giving advice here I try to give Pythonic answers. – steveha Mar 27 '14 at 02:56
0

Why not use a filter?

def ensure_values(l, upper=dim**2):
   return filter(lambda x: x>0 or x<upper, l)
fmatheis
  • 241
  • 3
  • 8