-3

This is my function. I want it to return all of the uppercase characters in the string (x) and if there are no upper case letters, then just return ' '.

def upper(x):
    for c in x:
        if c.isupper():
            return(c)
        elif x.islower():
            return('')

I'm looking for it to give me something like this when I run it:

upper("aBBa")
'BB'
upper("abba")
''

However, it's only giving me the first uppercase letter of the string, instead of all of the uppercase letters. I thought using the 'for' loop would resolve this, but apparently not.

upper("aBBa")
'B'
upper("ABC")
'A'

Any suggestions are appreciated.

Morgan Thrapp
  • 9,748
  • 3
  • 46
  • 67
Brian
  • 47
  • 3
  • 9
  • 2
    You can only `return` *once* from a function; once you do that, it's finished. Try adding uppercase characters to a list, rather than returning straight away. – jonrsharpe Jan 29 '15 at 08:47
  • It's being downvoted because it's an extremely trivial issue that shows that you have no idea what you're doing, and didn't invest enough time into learning the python basics (or just programming basics in general, in this case). SO is for _professional and enthusiast programmers_, and thus we expect askers to have at least a basic understanding of the topics of their questions. Furthermore, questions should be useful to a general programmer audience, not just to you. This is not a forum or tutoring resource; think of it as similar to wikipedia - and you don't meet our quality requirements. – l4mpi Jan 29 '15 at 09:24
  • @l4mpi Thank you for the clarification. None of this was made evident to me before, so I apologize. The only reason I post these questions is because there really isn't any other way to look up a specific problem like this, and I wouldn't be here at all if I had a competent python professor. He doesn't stick to the book, and doesn't explain half of what he does, so it can be difficult at times. – Brian Jan 29 '15 at 09:35
  • While mostly agreeing with l4mpi's comment, I'd prefer to say "think of it as similar to Wikipedia, and **your question** doesn't meet our quality requirements." You can significantly reduce the odds of getting downvotes on your questions by following the instructions at http://stackoverflow.com/help/how-to-ask and the pages it links to; in fact, it's a Good Idea™ to become familiar with the contents of everything at http://stackoverflow.com/help/asking ; and when you have the time try to read everything on the [Help Tour](http://stackoverflow.com/tour). – PM 2Ring Jan 29 '15 at 09:47

7 Answers7

0

In your code, when you use return, you can just return the first uppercase char and the function will end. So you need to sum all uppercase chars and then return :

def upper(x):
  return ''.join([i for i in x if i.isupper()])

print upper('ABcc')
print upper('aaabb')

Or change the list comprehension:

def upper1(x):
  return ''.join((i for i in x if i.isupper()))

Time test:

import timeit
print timeit.timeit("''.join(i for i in 'ABCDefg' if i.isupper())", number=1000000)
print timeit.timeit("''.join(i for i in 'efg' if i.isupper())", number=1000000)
print timeit.timeit("''.join([i for i in 'ABCDefg' if i.isupper()])", number=1000000)
print timeit.timeit("''.join([i for i in 'efg' if i.isupper()])", number=1000000)

Output:

2.93963984339
2.16251670139
1.55889267962
0.729011582306

list comprehension is faster...

lqhcpsgbl
  • 3,694
  • 3
  • 21
  • 30
  • Better yet: Change the list comprehension into a generator expression by removing the `[]`s. – Tim Pietzcker Jan 29 '15 at 09:03
  • 2
    When using `str.join` list comprehensions are actually better than generator expressions. See [this answer](http://stackoverflow.com/a/9061024/3005188) for more details. – Ffisegydd Jan 29 '15 at 09:06
  • @Tim actually - `''.join(...)` given a list is actually more efficient - as internally it has to build a `list` from the generator so it can allocate for the resulting string - if given an existing `list` - it doesn't need to do that - so it ends up faster... – Jon Clements Jan 29 '15 at 09:06
  • @JonClements Yes, list comprehension is faster.Thanks to tell me that . – lqhcpsgbl Jan 29 '15 at 09:16
  • 2
    @zoosuck just to note that you've got redundant `()`'s around your generator - they're only needed for a function when it's not the only argument – Jon Clements Jan 29 '15 at 09:17
0

You need to add it to a list since there is more then 1 option, just check if it's upper and append it, at the end return the string according to the size of the list, the join will take the list and print the elements as string joined together.

def upper(x):
    upper_s = []
    for c in x:
        if c.isupper():
            upper_s.append(c)

    return "".join(upper_s)

print upper("aBBa")
print upper("aa")

Output:

BB
Kobi K
  • 7,743
  • 6
  • 42
  • 86
0

By normal string method

def upper(x):
    result = ""
    for c in x:
        if c.isupper():
            result += c

    return result

print "result:-", upper("aBBa")
print "result:-", upper("abba")

Output

$ python test1.py
result:- BB
result:-
Vivek Sable
  • 9,938
  • 3
  • 40
  • 56
  • Note that using a list (or list comprehension) to hold partial results and then calling `''.join()` on the resulting list is _significantly_ more efficient than building up a result string using string concatenation (`result += c`). That's because each time you concatenate to a string a new chunk of memory to hold the result has to be allocated and the old chunk holding the former string gets thrown away. So the interpreter wastes a lot of time allocating & freeing all these little chunks of memory. Appending to a list works a lot more efficiently in that regard. – PM 2Ring Jan 29 '15 at 09:53
0
import re
str1 = "aBBa"
t = re.search("[A-Z].",str1).group()
print (t)

Basically will find all the uppercases. if return processed one time, the process is end. That's why your function returns one letter.

In function;

import re
def upper(x):
    t = re.search("[A-B].",x).group()
    return t

print (upper("aBBa"))
GLHF
  • 3,835
  • 10
  • 38
  • 83
-1

Your upper function is only returning a single value.ie once it finds an upper case letter, it returns the single letter (and thereby, terminating the function)

Instead you should store it in a list and return the list as a whole.

def upper(x):
    capitals=[]
    for c in x:
            if c.isupper():
                    capitals.append(c)

    return capitals

Ofcourse, appropriate 'if' statements could be added in the main() func, to print in whatever way you want it to.

viv1
  • 99
  • 2
  • 10
-1

Try this:

>>> def upper(x):
...     r = ""
...     for c in x:
...             if c.isupper():
...                     r += c
...     return r
...
>>> upper("aBBa")
'BB'
>>> upper("abba")
''
>>> upper("ABC")
'ABC'
>>>
zhangjiangtao
  • 371
  • 1
  • 6
  • 23
-2

The program exits the function when it encounters a "return" statement. So, it will return only the first upper case character of your example. For example in upper("aBBa") the characters which are traversed are "aB", when it sees B, it returns, the rest of string("Ba") is not traversed. Consider using Python's "Yield" statement, it is specifically designed for these cases. https://freepythontips.wordpress.com/2013/09/29/the-python-yield-keyword-explained/

Enigma
  • 1
  • 1
  • If you dont want to use a Yield statement, then just attach append the characters to a string inside the "if" and return the same. – Enigma Jan 29 '15 at 09:09
  • I agree that `yield` is a wonderful thing, but it's not appropriate here, which is probably why you got some downvotes. (FWIW, I didn't downvote you; I generally give explanatory warnings before I downvote answers). And as for the suggestion in your comment, please see my comment to Vivek Sable's answer. – PM 2Ring Jan 29 '15 at 10:11