16
def get_digits(str1):
    c = ""

    for i in str1:
        if i.isdigit():
            c += i
            return c

Above is the code that I used and the problem is that it only returns only the first digit of strings. For this, I have to keep both for loop and return statement. Anyone knows how to fix?

Thanks.

2
  • Is this homework? If so, please tag it as such. Commented Aug 17, 2012 at 12:26
  • I added a benchmark testing on my answer. Commented Aug 18, 2012 at 13:49

6 Answers 6

29

As the others said, you have a semantic problem on your indentation, but you don't have to write such function to do that, a more pythonic way to do that is:

def get_digits(text):
    return filter(str.isdigit, text)

On the interpreter:

>>> filter(str.isdigit, "lol123")
'123'

Some advice

Always test things yourself when people shows 'faster' methods:

from timeit import Timer

def get_digits1(text):
    c = ""
    for i in text:
        if i.isdigit():
            c += i
    return c

def get_digits2(text):
    return filter(str.isdigit, text)

def get_digits3(text):
    return ''.join(c for c in text if c.isdigit())

if __name__ == '__main__':
    count = 5000000
    t = Timer("get_digits1('abcdef123456789ghijklmnopq123456789')", "from __main__ import get_digits1")
    print t.timeit(number=count)

    t = Timer("get_digits2('abcdef123456789ghijklmnopq123456789')", "from __main__ import get_digits2")
    print t.timeit(number=count)

    t = Timer("get_digits3('abcdef123456789ghijklmnopq123456789')", "from __main__ import get_digits3")
    print t.timeit(number=count)



~# python tit.py
19.990989106  # Your original solution
16.7035926379 # My solution
24.8638381019 # Accepted solution
Sign up to request clarification or add additional context in comments.

2 Comments

In Python 3 you need to do ''.join(filter(lambda x: x.isdigit(), "lol123")) , see stackoverflow.com/a/1450900/179014 .
FYI: you only need the lambda if you're calling isdigit() (note the parentheses). Using str.isdigit as shown above works as expected
11

Your indentation is a bit borked (indentation in Python is quite important). Better:

def get_digits(str1):
    c = ""
    for i in str1:
        if i.isdigit():
            c += i
    return c

A shorter and faster solution using generator expressions:

''.join(c for c in my_string if c.isdigit())

5 Comments

That's a generator expression, not a list comprehension, but +1 for it either way.
is there any better place than the actual PEP for documentation on generator expressions?
I've edited your answer to link to the relevant part of the Python Tutorial; there's also the language reference.
a more 'advanced' version would be to replace the concatenation with a yield statement. The filter method is more elegant though
Your solution isn't faster, it is slower, and there is no reason to use generator expressions here, you're returning the entire result and not yielding the digits, that doesn't makes sense, see my answer with a benchmark test between the solutions.
2

it's because your return statement is inside the for loop, so it returns after the first true if condition and stops.

  def get_digits(str1):
      c = ""
      for i in str1:
        if i.isdigit():
            c += i
      return c

Comments

1

Of course it returns only the first digit, you explicitly tell Python to return as soon as you have a digit.

Change the indentation of the return statement and it should work:

def get_digits(str1):
    c = ""

    for i in str1:
        if i.isdigit():
            c += i

    # Note the indentation here
    return c

Comments

1

there is an indentation problem which returns when it finds the first digit, as with current indentation, it is intepreted as a statement inside if statement, it needs to be parallel to the for statement to be considered outside for statement.

def get_digits(str1):
    c = ""

    for i in str1:
        if i.isdigit():
            c += i

    return c

digits = get_digits("abd1m4m3m22mmmbb4")
print(digits)

A curly braces equivalent of your incorrect code is :

def get_digits(str1){
    c = ""

    for i in str1 {
        if i.isdigit(){
            c += i
            return c    # Notice the error here
        }
    }
}

And when the code is corrected to move the return statement in alignment with for, the equivalent is :

def get_digits(str1){
        c = ""

        for i in str1 {
            if i.isdigit(){
                c += i               
            }
        }
        return c    # Correct as required
    }

Comments

1

Your code was almost ok, except the return statement needed to be moved to the level of your for-loop.

def get_digits(str1):
    c = ""
    for i in str1:
        if i.isdigit():
            c += i
    return c   ## <--- moved to correct level

so, now:

get_digits('this35ad77asd5')

yields:

'35775'

Explanation:

Previously, your function was returning only the first digit because when it found one the if statement was executed, as was the return (causing you to return from the function which meant you didn't continue looking through the string).

Whitespace/indentation really matters in Python as you can see (unlike many other languages).

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.