Iteration Stopping Short

Question

I'm trying to write a function definition named only_odd_numbers that takes in sequence of numbers and returns the odd numbers in a list.

I've tried this in the Python Shell, but it only returns the first odd number in a list of numbers:

def only_odd_numbers(s):
    for num in s:
        if num % 2 != 0:
            return num
        elif num % 2 == 0:
            return s[num]

Here are some assertions that I'm trying to make work:

assert only_odd_numbers([1, 2, 3]) == [1, 3]
assert only_odd_numbers([-5, -4, -3, -2, -1, 1, 2, 3, 4, 5]) == [-5, -3, -1, 1, 3, 5]
assert only_odd_numbers([-4, -3, 1]) == [-3, 1]

Answer 1

Your function design is almost right. As others have noted before me, the function you have created returns from the function call after a single odd number is found in the list. One way to get the behavior you desire is to build the list within the function and return that list after the loop over elements completes, as the other respondents have suggested. But there is a way to keep this function in nearly the same form and get the results you want:

Generators

In python, you can create functions that keep track of their internal state. This means you can create a function-like object that takes a list, like your only_odd_numbers function, but that returns the next odd number in the list every time you call it. These things are called "generators".

You create generators just like you create functions, but instead of "returning" a value, generators "yield" a value, like so:

def only_odd_numbers(s):
    for num in s:
        if num % 2 != 0:
            yield num

Now when you call only_odd_numbers(s), it will return a generator rather than a number or a list. You can iterate over that generator just like how you iterate over a list, meaning you can build your answer from a list constructor or list comprehension, like so:

# list constructor
assert list(only_odd_numbers([1, 2, 3])) == [1, 3]
# list comprehension
assert [x for x in only_odd_numbers([-5, -4, -3, -2, -1, 1, 2, 3, 4, 5])] == [-5, -3, -1, 1, 3, 5]
# plain old iteration
itr = only_odd_numbers([-4, -3, 1])
assert next(itr) == -3
assert next(itr) == 1
# no more!
try:
    next(itr)
    print("Still iterating")
except StopIteration:
    print("No more")

I encourage you to read https://stackoverflow.com/a/231855/5075720 to get a better understanding of how generators work and how useful they can be.

Answer 2

When the code hits the first return statement, it interrupts the loop. You want it to continue, and you also want to keep all the odd numbers. So, you should create a list and append all the odd numbers. Finally, tuen function should return the list.

def only_odd_numbers(s):
    odd_nums = list() # initiate an empty list
    for num in s:
        if num % 2 != 0:
            odd_nums.append(num) # add to list the odd numbers
    return odd_nums # return the list

assert only_odd_numbers([1, 2, 3]) == [1, 3] # it works

print(only_odd_numbers([1, 2, 3, 4, 5, 6, 7, 8, 9]))

[1, 3, 5, 7, 9]

Answer 3

Yes, that's because in your code you are only returning the first odd number you find and you are not actually making a list.

first you have to start a list:

list = []

then, do:

for num in s: 

and inside put:

if num % 2 != 0: list.append(num)

and you dont have to have an else sentence, since if it is not odd you do nothing

Finally, at the end, outside the for, do:

return list

Hope it helps!

Answer 4

You should create list inside only_odd_numbers and add all odd values to this list before you use return

def only_odd_numbers(s):
    result = []

    for num in s:
        if num % 2 != 0:
            result.append(num)

    return result

assert only_odd_numbers([1, 2, 3]) == [1, 3]

You can even use list comprehension

def only_odd_numbers(s):
    return [num for num in s if num % 2 != 0]

Answer 5

You can do all this with a list comprehension. It's clearer and shorter:

odd_numbers = [n for n in s if n % 2 != 0]

Anyway your code's issue is that return s[num] doesn't do an append operation.