Delete item from list in Python while iterating over it

Question

I'm writing a round robin algorithm for a tournament app.

When the number of players is odd, I add 'DELETE' to the list of players, but later, when I want to delete all items from schedule list that contain 'DELETE', I can't -- one is always left. Please take a look at the code -- the problem is simple and I suppose it's about lists; I just i can't see it.

"""
Round-robin tournament:
1, 2, 3, 4, | 5, 6, 7, 8    =>  1, 2, 3, 4  => rotate all but 1 =>  1, 5, 2, 3  => repeat =>    1, 6, 5, 2  ...
                    5, 6, 7, 8              6, 7, 8, 4          7, 8, 4, 3
in every round pick l1[0] and l2[0] as first couple, after that l1[1] and l2[1]...
"""

import math

lst = []
schedule = []
delLater = False

for i in range(3):                  #make list of numbers
    lst.append(i+1)

if len(lst) % 2 != 0:               #if num of items is odd, add 'DELETE'
    lst.append('DELETE')
    delLater = True


while len(schedule) < math.factorial(len(lst))/(2*math.factorial(len(lst) - 2)): #!(n)/!(n-k)

    mid = len(lst)/2

    l1 = lst[:mid]
    l2 = lst[mid:]

    for i in range(len(l1)):            
        schedule.append((l1[i], l2[i]))         #add lst items in schedule

    l1.insert(1, l2[0])             #rotate lst
    l2.append(l1[-1])
    lst = l1[:-1] + l2[1:]


if delLater == True:                #PROBLEM!!! One DELETE always left in list
    for x in schedule:
        if 'DELETE' in x:
            schedule.remove(x)

i = 1
for x in schedule:
    print i, x
    i+=1

score 7 · Answer 1 · edited May 23 '17 at 11:45

7

schedule[:] = [x for x in schedule if 'DELETE' not in x]

See the other questions about deleting from a list while iterating over it.

edited May 23 '17 at 11:45

Community

1
1

answered Sep 27 '11 at 17:20

Jochen Ritzel

104,512
31
200
194

1

+1 for using [:] to just modify the same list object. – Macke Sep 27 '11 at 17:24
3

Why use in-place assignment? @Macke there is likely no benefit to it. It briefly saves a bit of memory, and it's a bit slower. Most likely neither matters. – agf Sep 27 '11 at 17:25
@agf: Just to fix the existing code with as little change as possible. Personally, I'd just use generators everywhere or write a algorithm that doesn't need the bogus entries, but that's not the question. – Jochen Ritzel Sep 27 '11 at 17:34
@agf Readability. Introducing another list object introduces another variable to remember when reading the code. – Velociraptors Sep 27 '11 at 17:36
1

@Velociraptors What does object identity matter? You can still use the same name, you don't need a new variable: `schedule = [x for x in schedule if 'DELETE' not in x]` – agf Sep 27 '11 at 17:39
@agf: use in-place assignment when it's important to keep all the names bound to the list synchronized. – Ethan Furman Sep 27 '11 at 18:09

Ethan Furman · Answer 2 · 2011-09-27T18:40:08.210

4

To remove elements from a list while iterating over it, you need to go backwards:

if delLater == True:
    for x in schedule[-1::-1]):
        if 'DELETE' in x:
            schedule.remove(x)

A better option is to use a list-comprehension:

schedule[:] = [item for item in schedule if item != 'DELETE']

Now, you could just do schedule = instead of schedule[:] = -- what's the difference? One example is this:

schedule = [some list stuff here]  # first time creating
modify_schedule(schedule, new_players='...') # does validation, etc.

def modify_schedule(sched, new_players):
    # validate, validate, hallucinate, illustrate...
    sched = [changes here]

At this point, all the changes that modify_schedule made have been lost. Why? Because instead of modifying the list object in place, it re-bound the name sched to a new list, leaving the original list still bound to the name schedule in the caller.

So whether or not you use list_object[:] = depends on if you want any other names bound to your list to see the changes.

edited Sep 27 '11 at 18:40

answered Sep 27 '11 at 18:07

Ethan Furman

63,992
20
159
237

In a correct design, `def` returns `sched` which is assigned back to `schedule`, or in a considered-bad-form-but-logical design, you don't pass the schedule and you use `global schedule`. The example design you have above is simply wrong, whether or not you use in-place assignment, because it isn't clear that `sched` and `schedule` refer to the same thing at the point where you're changing `schedule`. – agf Sep 27 '11 at 18:13
Also, did you try your loop version? I'm not sure how it's supposed to work, but it's got at least a couple of errors. You're trying to remove something that you don't know is in the list (the index from `enumerate`) and your values for slicing don't appear to make sense. – agf Sep 27 '11 at 18:17
1

That is not the correct way to use slices to reverse a list, try `schedule[-1::-1]`, or better yet `reversed(schedule)`, since the slice will construct a new list anyway. – Andrew Clark Sep 27 '11 at 18:24
@F.J something like `for _ in range(sum('DELETE' in x for x in schedule)): schedule.remove(x)` works just as well -- since `remove` does a search anyway, you're really just using the _number of matches_ not their location, so just de-sychronize the two operations and there is no need to act in reverse, even if you want to use a loop. – agf Sep 27 '11 at 18:27
@agf: No, unfortunately -- my main point was refuting your claim that `[:]` was unnecessary and a waste of time. Loop version fixed, thanks. – Ethan Furman Sep 27 '11 at 18:43
@agf: so change the call site to `schedule[:] = modify_schedule(schedule, ...)` -- my point still stands about multiple names being bound to the one list object, and the decision about keeping them in sync (or not). – Ethan Furman Sep 27 '11 at 18:48
code works just fine with schedule[:] = [x for x in schedule if 'DELETE' not in x]. im new at python and programming in general so there are mistakes, wish i know more so i can discuss about them. thanks to all for help. – Rodic Sep 27 '11 at 20:03
agree with @AndrewClark , schedule[-1::-1] same as schedule[:] constructs a copy. – iperov Mar 14 '22 at 15:18
@iperov: Then you missed the point twice: 1) `schedule[-1::-1]` makes a reverse copy, while `schedule[:]` makes a normal copy; 2) my answer addresses the question of why you would want to assign using `schedule[:] = ...` instead of `schedule = ...` . – Ethan Furman Mar 14 '22 at 17:26

score 3 · Accepted Answer · edited Oct 11 '19 at 22:18

3

You shouldn't modify the list while iterating over it:

for x in schedule:
    if 'DELETE' in x:
        schedule.remove(x)

Instead, try:

schedule[:] = [x for x in schedule if 'DELETE' not in x]

For more info, see How to remove items from a list while iterating?

edited Oct 11 '19 at 22:18

Karl Knechtel

62,466
11
102
153

answered Sep 27 '11 at 17:20

NPE

486,780
108
951
1,012

2

There was no need to copy the in-place assignment from the other answers, it's not better. – agf Sep 27 '11 at 17:26
thanks for the answer and info. to be honest i still don't know how to ask question about python properly or where to look for the answer. i didn't know that removing items during iteration is different. – Rodic Sep 27 '11 at 17:39
@Rodic: My pleasure. This problem is something every Python coder runs into sooner or later. – NPE Sep 27 '11 at 17:41
1

@agf: if not using `[:]` creates bugs, then using it *is* better. See my answer for details. – Ethan Furman Sep 27 '11 at 18:08

score 2 · Answer 4 · answered Sep 27 '11 at 17:21

2

Don't modify a sequence being iterated over.

schedule[:] = [x for x in schedule if 'DELETE' not in x]

answered Sep 27 '11 at 17:21

Ignacio Vazquez-Abrams

776,304
153
1,341
1,358

Delete item from list in Python while iterating over it

4 Answers4

Linked