2

I'm suppose remove all the vowels from the any string that would be entered. I'm trying to make the code as simple as possible.

Thank you for the help.

def anti_vowel(text):
    for i in text:
       i.strip(['i','o','a','u','e'])
       return i
cs95
  • 379,657
  • 97
  • 704
  • 746
STNDR
  • 25
  • 4
  • [Relevant question from code review.](https://codereview.stackexchange.com/questions/151716/remove-all-vowels-from-a-string-except-an-initial-character) – Max von Hippel May 10 '18 at 04:05

4 Answers4

8

So you call strip on each character.... and then what? You don't update the string, because strings are immutable and i.strip is not an inplace operation.

A naive improvement over your solution would filtering characters out inside a list comprehension and then doing a join on the result:

vowels = {'i','o','a','u','e'}
def anti_vowel(text):
    return ''.join([c for c in text if c not in vowels])

A small note: if your string contains mixed case, you may want to either

  1. Lowercase text, or
  2. Augment vowels to contain uppercase vowels: vowels = set('aeiouAEIOU'), or
  3. Use str.casefold (as per @Adam Smith's comment)—augmenting vowels is no longer needed in that case:

    return ''.join([c for c in text if c.casefold() not in vowels])
    

You can get even better with str.translate (this works on python-3.x):

mapping = str.maketrans(dict.fromkeys(vowels, '')) # create a global mapping once
def anti_vowel(text):
    return text.translate(mapping))
cs95
  • 379,657
  • 97
  • 704
  • 746
  • Don't need the `[]` in the join. – Stephen Rauch May 10 '18 at 01:21
  • 8
    @StephenRauch But it's faster _with_ it... let me find you the link. EDIT: here: https://stackoverflow.com/a/9061024/4909087 – cs95 May 10 '18 at 01:21
  • for mixed case -- `...if c.casefold() not in vowels])` is probably best, rather than `[c for c in text.casefold() ...` since then you don't lose the original case – Adam Smith May 10 '18 at 01:25
  • I'd probably create the translation table as a global, rather than rebuilding it in each `anti_vowel` call. – PM 2Ring May 10 '18 at 01:26
  • @AdamSmith Very nice, thank you. I put that in there. – cs95 May 10 '18 at 01:26
  • @PM2Ring Yes, thank you, another great improvement there. – cs95 May 10 '18 at 01:27
  • 2
    @StephenRauch Here's what core dev Raymond Hettinger has to say about `.join` on a gen exp vs list comp: https://stackoverflow.com/a/9061024/4014959 – PM 2Ring May 10 '18 at 01:29
  • @PM2Ring, thanks that is the link coldspeed had above. If you are paying attention, and hanging out in the right places, you can learn something new everyday... – Stephen Rauch May 10 '18 at 01:31
  • @StephenRauch Ah, right, I missed his edit. :) FWIW, if `.join` didn't do two passes it would be less efficient, since it would have to do concatenation in a loop, [which is bad](https://www.joelonsoftware.com/2001/12/11/back-to-basics/). – PM 2Ring May 10 '18 at 01:34
1

Two problems:

  1. strip is the wrong method; it only removes from the beginning and end of a string. Use .replace(something, '').
  2. Strings are immutable; a method cannot modify a string. strip returns the modified string.
Jonathon Reinhart
  • 132,704
  • 33
  • 254
  • 328
  • Very true, although #1 isn't relevant here, since the OP's code is calling `.strip` on single chars. – PM 2Ring May 10 '18 at 01:22
  • 1
    @STNDR Just FYI, you can only accept a single answer... when you accept one and then accept another, the first is automatically unmarked :-) – cs95 May 10 '18 at 02:26
  • 1
    Lol, I was surprised that mine was accepted over cᴏʟᴅsᴘᴇᴇᴅ's. You should probably consider accepting that one again. – Jonathon Reinhart May 10 '18 at 02:44
0

This code worked too! The dude who wrote the anwser deleted their post. Shame it worked really well.

liz = ("Totally not going to hitting the big bong at the event")
import re

def anti_vowel(text):
    print re.sub('[aeiou]', '', text)

anti_vowel(liz)

output: 
Ttlly nt gng t httng th bg bng t th vnt
STNDR
  • 25
  • 4
  • The deleted answer contains some incorrect information. But yes, they could've just edited that out. That regex code works, but I suspect that using the built-in `str.translate` is faster. – PM 2Ring May 10 '18 at 01:40
  • Feel free to do a timeit test, but I can guarantee this regex replacement is the slowest, followed by your loopy replace. The fastest and second fastest by far is translate and the list comp. – cs95 May 10 '18 at 02:30
0

str.strip is not an in-place operation and takes a string as an argument. Therefore, your code will not work as desired.

You can use bytearray to efficiently remove certain characters from your string. You may see an order of magnitude better performance for larger strings.

Character removal can be performed with bytearray.translate directly, without the need to use str.maketrans.

def anti_vowel_bytes(s):
    b = bytearray()
    b.extend(s.encode())
    b = b.translate(None, b'aeiou')
    return b.decode('utf-8') 

def anti_vowel_str(s, mapping):
    return s.translate(mapping)

vowels = set('aeiou')
mapping = str.maketrans(dict.fromkeys(vowels, ''))

test = 'alpha beta gamma delta epsilon zeta eta'*100000
res1 = anti_vowel_bytes(test)
res2 = anti_vowel_str(test, mapping)

assert res1 == res2

%timeit anti_vowel_bytes(test)            # 20 ms
%timeit anti_vowel_str(test, mapping)     # 398 ms
jpp
  • 159,742
  • 34
  • 281
  • 339
  • Slightly unfair as of now, because both vowel and mapping's definitions can be moved outside, otherwise they're timed as part of the timeit at each iteration. Otherwise this is a nice find. – cs95 May 10 '18 at 02:33
  • @cᴏʟᴅsᴘᴇᴇᴅ, Fair point; updated the benchmarking. Yep, it's amazing the new things you can learn from the official docs. – jpp May 10 '18 at 02:37