0

I've been trying to convert some of my functions where I use a for loop by using list comprehension. Here is my first version of the function,

def adstocked_advertising(data, adstock_rate):
    '''
    Transforming data with applying Adstock transformations
    data - > The dataframe that is being used to create Adstock variables
    adstock_rate -> The rate at of the adstock

    ex. data['Channel_Adstock'] = adstocked_advertising(data['Channel'], 0.5)
    '''
    adstocked_advertising = []
    for i in range(len(data)):
        if i == 0: 
            adstocked_advertising.append(data[i])
        else:
            adstocked_advertising.append(data[i] + adstock_rate * adstocked_advertising[i-1])            
    return adstocked_advertising

I want to convert it to this,

def adstocked_advertising_list(data, adstock_rate):
    adstocked_advertising = [data[i] if i == 0 else data[i] + adstock_rate * data[i-1] for i in range(len(data))]

    return adstocked_advertising

However, when viewing the df after running both functions I get two different values.

data['TV_adstock'] = adstocked_advertising_list(data['TV'], 0.5)
data['TV_adstock_2'] = adstocked_advertising(data['TV'], 0.5)

here is output,

data.head()

enter image description here

data.tail()

enter image description here

I am not too sure why the first two rows are the same and then from there the numbers are all different. I am new to list comprehension so I may be missing something here.

  • 1
    Is there a specific reason you want to convert it to a list comprehension? Sometimes the loop is more legible. I am not aware of significant computational advantages to list comprehension, see here: https://stackoverflow.com/a/22108640/8793975 – enumaris Nov 20 '18 at 19:24
  • 1
    In the function version `adstocked_advertising_list()`, you reference `i` but i is not defined as a parameter, it's just dangling, presumably a dangling non-zero final value from the for-loop version, hence in your function the `data[i] if i == 0` clause is never used. So just fix that and it should work. – smci Nov 20 '18 at 19:28
  • @enumaris , the main reason for this is to learn how to use list comprehension. – Timothy Mcwilliams Nov 20 '18 at 19:29

2 Answers2

2

You need to refer to the previously generated element in the list, and list comprehensions are not well suited to this type of problem. They work well for operations that only need to look at a single element at once. This question goes into more detail.

In your initial example, you use adstock_rate * adstocked_advertising[i-1]. The list comprehension version uses adstock_rate * data[i-1], which is why you are getting different results.

A standard for loop works just fine for your use case. You could switch to using enumerate, as for i in range(len(data)) is discouraged.

if data:
    res = [data[0]]
for index, item in enumerate(data[1:]):
        results.append(item + rate * data[index-1])
Wieschie
  • 479
  • 1
  • 7
  • 15
  • 1
    Thanks for the suggestion. I guess there are specific use cases where list compression is used over a standard for loop and vise versa. – Timothy Mcwilliams Nov 20 '18 at 19:36
  • 2
    `if index == 0` will only be true once, at the beginning of the loop. Append the first element, and then iterate unconditionally. – ggorlen Nov 20 '18 at 19:38
  • Your translation of the algorithm is now incorrect because the first element will be appended twice. – ggorlen Nov 20 '18 at 20:16
1

You've changed your logic in the list comp version. Originally, your else formula looked like:

data[i] + adstock_rate * adstocked_advertising[i-1]

But the list comprehension version looks like:

data[i] + adstock_rate * data[i-1]

The first version accesses the i-1th element of the result list, while the second version accesses the i-1th element of the input list.

index == 0 is only true once at the beginning of the list. Why not eliminate the conditional:

def adstocked_advertising(data, adstock_rate):
    if data:
        res = [data[0]]

        for i in range(1, len(data)):
            res.append(data[i] + adstock_rate * res[i-1])

        return res
ggorlen
  • 44,755
  • 7
  • 76
  • 106