pop() function not functioning right in a for loop

Question

Here's the code:

vowels = ['a','e','i','o','u']

def anti_vowel(text):
    tlength = len(text)
    words = []
    result = ""
    for word in range(len(text)):
        words.append(text[word])
        print words
    for index, word in enumerate(words):
        if word.lower() in vowels:
            words.pop(index)
    for old_word in words:
        result += str(old_word)
    return result

print anti_vowel("Hey look words!")

Expected result: "Hy lk wrds!" Apparent result: "Hy lok words!"

I'm not able to figure out why is the loop skipping the 'o' on index 5 in the list words. I know I could do this another way by appending non-vowel words to a list and combining them but I want to know how to get the desired result for the above code.

@PeterWood I was supposed to add a raw_input() instead of hard coding something into the statement that calls the function. — Areeb, Dec 26 '15 at 16:31
old_word was a string before the conversion too.. excuse that mistake — Areeb, Dec 26 '15 at 16:32
How could an uppercase character be in a list of lowercase characters? — Padraic Cunningham, Dec 26 '15 at 16:54

Daniel · Accepted Answer · 2015-12-26T16:36:25.380

4

You are deleting characters, but your index is counting on. So the vowels are not at the index anymore, you expect.

Better don't change a list in a for loop, but create a new one:

def anti_vowel(text):
    words = []
    for character in text:
        if not (character.lower() in vowels or character.upper() in vowels):
            words.append(character)
    return ''.join(words)

or with generator expression:

def anti_vowel(text):
    return ''.join(c for c in text if c.lower() not in vowels)

When you want to delete single elements, you have to use a while loop:

def anti_vowel(text):
    text = list(text)
    index = 0
    while index < len(text):
        if text[index].lower() in vowels:
            del text[index]
        else:
            index += 1
    return ''.join(text)

edited Dec 26 '15 at 16:36

answered Dec 26 '15 at 16:23

Daniel

42,087
4
55
81

perfect and simple :) – Nikhil Parmar Dec 26 '15 at 16:25
2

A generator is less efficient than a list comprehension in `str.join`. See [When is not a good time to use python generators?](http://stackoverflow.com/questions/245792/when-is-not-a-good-time-to-use-python-generators) – Peter Wood Dec 26 '15 at 16:29
Thank you for your answer Daniel, yes I already mentioned I know that way. So basically you mean that when I'm deleting / popping out stuff from list the indexing continues and the popping process is like uh a slow process and cannot sync the index at the 'o' next to the previous one? And there's no way to correct this? (I'm bad at explaining stuff.. excuse me for that please) – Areeb Dec 26 '15 at 16:29
2

So why want a complicated an error prone way, if you know the correct and elegant solution? – Daniel Dec 26 '15 at 16:31
Just asking stuff out of curiosity .. I came up with that way first while solving a practice problem. Anyway it seems like building up the new sentence would be a neat an non-buggy way to accomplish the task.. Thanks for your reply. – Areeb Dec 26 '15 at 16:35
@PeterWood: even if `join` creates a list internally, it's never less efficient than doing this explicitly. (more efficient implementations would create character buffer on the way) – Daniel Dec 26 '15 at 16:47
2

@Daniel It is less efficient as the generator must be created. See [this answer](http://stackoverflow.com/a/9061024/1084416) to a related question. – Peter Wood Dec 26 '15 at 17:12

pop() function not functioning right in a for loop

1 Answers1