# Making a nested loop structure more pythonic

I'm looking for a review on my code. I am also looking for ways to transform this function into something more Pythonic. I'm fairly new to Python, so any advice would be appreciated.

def get_rsi(self, period):
rsi = []

for i in xrange(1, len(self.hist_d) - period + 1):
gains = 0.0
losses = 0.0

for j in xrange(i, i + period):
diff = self.hist_d[j][2] - self.hist_d[j - 1][2]

if diff > 0:
gains += diff
elif diff < 0:
losses += abs(diff)

rsi.append(round(100 - (100 / (1 + gains / losses)), 2))

return rsi
-

There is a bug in your code. If no losses were reported in that window then losses == 0.0 which when you append to rsi will throw a ZeroDivisionError.

There are two suggestions I would make:

1. Use zip (or izip if possible)

The zip function takes two iterables and combines them. It essentially returns:

>>>zip([1,2,3], ['a','b','c'])
[(1, 'a'), (2, 'b'), (3, 'c')]

We can use this instead of directly indexing through j and j-1.

2. If-Else

Currently your if-statement looks like this:

if diff > 0:
gains += diff
elif diff < 0:
losses += abs(diff)

I would recommend making the elif into a simple else. This is because anything that the elif won't catch is if diff == 0 and since losses == losses + 0, it won't effect losses. Also, you remove a comparision using a simple else.

With these suggestions taken into account (plus the simple bug fix), we can refactor your code a little bit:

def get_rsi(self, period):
rsi=[]
for i in range(len(self.hist_d)-period):
gains = 0.0
losses = 0.0

window = self.hist_d[i:i+period+1]
for year_one, year_two in zip(window, window[1:]):
diff = year_two - year_one

if diff > 0:
gains += diff
else:
losses += abs(diff)

# Check if losses is zero. If so, 100/(1 + RS) will practically be 0.
if not losses:
rsi.append(100.00)
else:
rsi.append(round(100 - (100 / (1 + gains / losses)), 2))

return rsi
-

It looks like you want a moving window of length period over self.hist_d (and then a moving window of length 2 over each of those windows, to get pairs of consecutive years). An efficient way of doing that is provided in the old version of the itertools documentation:

from itertools import islice, izip

def window(seq, n=2):
"Returns a sliding window (of width n) over data from the iterable"
"   s -> (s0,s1,...s[n-1]), (s1,s2,...,sn), ...                   "
it = iter(seq)
result = tuple(islice(it, n))
if len(result) == n:
yield result
for elem in it:
result = result[1:] + (elem,)
yield result

I heard of this via this SO question, where there are also other options for the same task.

You can then use this to process your results:

def get_rsi(self, period):
for series in window(self.hist_d, period):
gains = losses = 0.0
for year1, year2 in window(series): # or 'izip(series, series[1:])'
diff = year2[2] - year1[2]
...
-
He doesn't want a static second window of size two. Its of size period. –  BeetDemGuise May 22 '14 at 14:36
@DarinDouglass the inner, length-2 window in my code is for diff = self.hist_d[j][2] - self.hist_d[j - 1][2] - it iterates over the whole series. I have edited slightly to hopefully clarify this. –  jonrsharpe May 22 '14 at 14:40
Oh ok, I get what you meant now. It probably would be more clear as 'the difference of each pair of successive years' or something like that :P –  BeetDemGuise May 22 '14 at 14:56
@DarinDouglass edited - is that clearer? –  jonrsharpe May 22 '14 at 14:59
Yeah that looks fine :D –  BeetDemGuise May 22 '14 at 15:06