patternpythonMinor
Calculating relative strength index over a period
Viewed 0 times
periodstrengthcalculatingindexoverrelative
Problem
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 rsiSolution
Your code is already pretty Pythonic.
There is a bug in your code. If no losses were reported in that window then
There are two suggestions I would make:
-
Use
The
We can use this instead of directly indexing through
-
Currently your if-statement looks like this:
I would recommend making the
With these suggestions taken into account (plus the simple bug fix), we can refactor your code a little bit:
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:
-
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.-
If-ElseCurrently 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 rsiCode Snippets
>>>zip([1,2,3], ['a','b','c'])
[(1, 'a'), (2, 'b'), (3, 'c')]if diff > 0:
gains += diff
elif diff < 0:
losses += abs(diff)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 rsiContext
StackExchange Code Review Q#51406, answer score: 6
Revisions (0)
No revisions yet.