Python simple moving average class based

Question

Ok so I'm writing a class that will calculate a simple moving average on a list of prices. It calculates the average every N number of prices with out calculating the first N-1 days. This is what I have:

class Simplemovingaverage():
    def __init__(self, Nday, list_of_prices):
        self._Nday = Nday
        self._list_of_prices = list_of_prices

    def calculate(self):
        for i in range(len(self._list_of_prices)):
            if i < self._Nday:
                average = 0
            elif i == self._Nday:
                average = sum(self._list_of_prices[:self._Nday])/self._Nday
            else:
                average = sum(self._list_of_prices[i-self._Nday:i])/self._Nday
            print(average)

I tested it by making a class object on the shell 'x = Simplemovingaverage(3, [1,2,3,4,5,6,7,8,9,10])' and then doing the calculate method by 'x.calculate' the output I got was:

0
0
0
2.0
3.0
4.0
5.0
6.0
7.0
8.0

So from my list of numbers its only calculating up to 7,8,9 the last number should be 9 because that's the average of 8,9,10 and also there should only be 3 zeros since N is 3. This is the output I'm looking for:

0
0
2.0
3.0
4.0
5.0
6.0
7.0
8.0
9.0

Answer 1

OK, here's a cleanup:

class Simplemovingaverage():
    def __init__(self, navg, items):
        self.navg = navg
        self.items = items

    def calculate(self):
        av = []
        for i in range(len(self.items)):
            if i+1 < self.navg:
                av.append(0)
            else:
                av.append(sum(self.items[i+1-self.navg:i+1])/self.navg)
        return av

First, you need to use i+1 everywhere, since range gives 0...navg-1 rather than 1...navg (you could also do range(1, n+1)).

Second, you don't need to special-case i+1==self.navg (:m is the same as 0:m).

Third, it makes more sense to return a list rather than printing the results (although I like the other respondent's idea of using yield to make this a generator!)

Fourth, there's no real reason to hide the number and the list, so I've removed the underscores (python isn't java or c++!).

Finally, this is more general than a "list of prices" averaged over a certain number of "days", so I've renamed the parameters for more generality.

Answer 2

def sma_calc(prices_list, window_size):
    return sum(prices_list[-window_size:]) / window_size

Answer 3

from __future__ import division
from itertools import islice, tee

def moving_average(n, iterable):
    # leading 0s
    for i in range(1, n):
        yield 0.

    # actual averages
    head, tail = tee(iterable)
    sum_ = float(sum(islice(head, n)))
    while True:
        yield sum_ / n
        sum_ += next(head) - next(tail)

When run as

list(moving_average(3, [1,2,3,4,5,6,7,8,9,10]))

returns

[0.0, 0.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0]

(starts with N-1 leading 0s, meaning the output list has the same cardinality as the input list, which I think is what you actually wanted).

Answer 4

You've got an out by one error. Try this:

  for i in range(len(self._list_of_prices) + 1):

So this might illustrate what's going on a bit

>>> _list_of_prices = [1,2,3,4,5,6,7,8,9,10]
>>> len(_list_of_prices)
10
>>> range(len(_list_of_prices))
[0, 1, 2, 3, 4, 5, 6, 7, 8, 9]
>>> i = 9
>>> _list_of_prices[i-3:i]
[7, 8, 9]

So the problem is that the python's list slice operator returns up to i in the example above but not i, (in this example it returns i-3, i-2 and i-1) e.g.

>>> word = "helpa"
>>> word[0:2]
'he'

See here http://docs.python.org/2/tutorial/introduction.html

Answer 5

You've mistaken the index:

In [186]: class Simplemovingaverage():
     ...:     def __init__(self, Nday, list_of_prices):
     ...:         self._Nday = Nday
     ...:         self._list_of_prices = list_of_prices
     ...: 
     ...:     def calculate(self):
     ...:         for i in range(len(self._list_of_prices)):
     ...:             if i < self._Nday-1:
              #------------------------^^--------------------------
     ...:                 average = 0
     ...:             elif i == self._Nday-1:
              #---------------------------^^--------------------------
     ...:                 average = sum(self._list_of_prices[:self._Nday])/self._Nday
     ...:             else:
     ...:                 average = sum(self._list_of_prices[\
                                i+1-self._Nday:i+1])/self._Nday
              #-----------------^^^-------------^^^--------------------------                  

     ...:             print(average)

In [187]: x = Simplemovingaverage(3, [1,2,3,4,5,6,7,8,9,10])

In [188]: x.calculate()
0
0
2
3
4
5
6
7
8
9