Почему не удаляются все элементы списка?

Question

[Дмитрий Васильев]

Пытаю решить задачу на checkio. Надо удалить все уникальные элементы. Почему то при вводе списка со всеми уникальными элементами получается такой вывод.

def checkio(data):
    for i in data:
        if data.count(i) < 2:
            data.remove(i)
        return data


checkio([1, 2, 3, 4, 5])            #   >>>   [2, 4]

Comments

[Влад]

Нельзя одновременно итерироваться по списку и удалять из него элементы.

[angru]

d1skort: вам стоило ваш комментарий оформить как ответ, т.к. он единственный корректный из всех, что были даны.

Answer 1

[Andrey K]

def checkio(data):
    return [i for i in data if data.count(i)>1]

Comments

[Дмитрий Васильев]

Можете пояснить как работает данный код?))

[Andrey K]

Он формирует новый список из неуникальных элементов, вместо того чтобы изменять существующий.

[Дмитрий Васильев]

Andrey Kot: Спасибо. Теперь всё понятно.

Answer 2

[psvworks]

Отвечу на вопрос, а то все предложили свои варианты, но никто не ответил почему программа не работает. В свое время я тоже написал решение в стиле ТСа, но, естественно, она работала почему-то неправильно. Причина в следующем: если мы перебираем элементы и удаляем какой-либо, то сам элемент удалится, произойдет смещение всех элементов влево (в результате чего их индексы уменьшатся на 1) и при этом итерируемый индекс еще и увеличится на 1 (!). Т.е. каждый раз, когда удаляется уникальный элемент, будет рассматриваться не следующий за ним элемент, а через него. Такие дела.

P.S. Могу посоветовать производить перебор элементов списка через их индексы из range(len(data)), и после каждого удаления уникального элемента производить корректировку индекса i = i - 1 (но это будет не питоновский стиль ;) )

Comments

[Дмитрий Васильев]

Спасибо огромное)

[ADRian]

тут While надо, а не for...
i = 0
while i < len(data):
if data.count(data[i]) > 2:
del data[i]
else:
i += 1

но это очень долго. проще создать новый масив.

Answer 3

[Антон Федорян]

import copy

def checkio(data):
	xdata = copy.deepcopy(data)
	for i in data:
		if (xdata.count(i) < 2):
			xdata.remove(i)
	return xdata

Answer 4

[nirvimel]

С точки зрения производительности так будет быстрее:

from collections import Counter

def checkio(data):
    c = Counter(data)
    return [i for i in data if c[i] > 1]

Comments

[Артём Клименко]

Я знаком с исходной задачей на checkio, там именно необходимо удалить из списка не уникальные элементы, порядок элементов внутри списка должен сохранится:
[1, 7, 2, 3, 4, 3, 4, 6, 7] -> [7, 3, 4, 3, 4, 7]

[angru]

мог ошибиться при замерах, но как-то так:

import timeit


ex1 = """data = [1, 2, 2, 3, 3, 3, 4, 4, 4, 5]
res = [k for k, v in Counter(data).items() if v > 1]"""


ex2 = """data = [1, 2, 2, 3, 3, 3, 4, 4, 4, 5]
res = [i for i in data if data.count(i)>1]"""


if __name__ == '__main__':
    print('ex1', timeit.timeit(ex1, setup='from collections import Counter'))
    print('ex2', timeit.timeit(ex2))

>>> ex1: 5.086754029998701
>>> ex2: 3.0053565109992633

[nirvimel]

angru: А что, если так?

from collections import Counter
import random
import timeit


data = [random.randint(0, 9) for _ in range(1000)]


def ex1():
    return [k for k, v in Counter(data).items() if v > 1]


def ex2():
    return [i for i in data if data.count(i) > 1]


if __name__ == '__main__':
    print('ex1', timeit.timeit(ex1, number=1000))
    print('ex2', timeit.timeit(ex2, number=1000))

>>> ('ex1', 0.26424193382263184)
>>> ('ex2', 7.536624908447266)

[nirvimel]

Артём Клименко: Поправил ответ с учетом этого.
angru: В прежнем и нынешнем вариантах разница больше чем на порядок (на списке из 1000 элементов) сохраняется.

[angru]

nirvimel: ну так-то да, у вас каждый вызов функции вместо 1000 делает 0-9 итераций )) смысла сравнивать скорости конечно не было, просто я только в процессе замеров понял, что ваше решение не удовлетворяет условию.

[nirvimel]

angru:

from collections import Counter
import random
import timeit


data = [random.randint(0, 9) for _ in range(1000)]


def ex1():
    c = Counter(data)
    return [i for i in data if c[i] > 1]


def ex2():
    return [i for i in data if data.count(i) > 1]


if __name__ == '__main__':
    print('ex1', timeit.timeit(ex1, number=1000))
    print('ex2', timeit.timeit(ex2, number=1000))

Вот замер для обновленного варианта. Тут честные 1000 итераций.

('ex1', 0.3047502040863037)
('ex2', 7.602175951004028)

[angru]

nirvimel: все разобрался, вот теперь вариант со счетчиком действительно намного оптимальнее.

[nirvimel]

angru: https://ideone.com/u93NvY - Пришлось сильно урезать количество итераций, чтобы уложиться во время.

[angru]

nirvimel: да не это я затупил, использовал свой набор с данными [1, 2, 2, 3, 3, 3, 4, 4, 4, 5], потому так быстро было.

p.s.: https://ideone.com/uazKur ))

[angru]

тут, конечно, нет ничего удивительного, там куча проверок при создании Counter'а

Answer 5

[Evgeniy _]

def checkio(data):
    for i in data[-1::-1]:
        if data.count(i) < 2:
            data.remove(i)
    return data

checkio([1, 2, 3, 4, 5, ])             #   >>>   []
checkio([1, 2, 3, 4, 5, 4])             #   >>>   [4,4]

Comments

[abs0lut]

как непитонично...

[Evgeniy _]

abs0lut: Только учусь.

def checkio(data):
return filter(lambda x: data.count(x) > 1, data)

так чуть по-питонистей?)

[abs0lut]

Evgeniy _: да, немного лучше)
по-мне, так самый питоничный вариант:

def checkio(data):
    return [i for i in data if data.count(i)>1]