nardew / talipp

talipp - incremental technical analysis library for python
https://nardew.github.io/talipp
MIT License
367 stars 59 forks source link

but (division by 0 in ADX) #123

Closed popek777 closed 5 months ago

popek777 commented 5 months ago

hi, division by 0 exception in ADX:

self.dx.append(100.0 * float(abs(self.pdi[-1] - self.mdi[-1])) / **(self.pdi[-1] + self.mdi[-1]**))

self.adx3 = ADX(4, 4, ohlcv)

File "/home/pprod/.pyenv/versions/3.10.4/lib/python3.10/site-packages/talipp/indicators/ADX.py", line 62, in init self.initialize(input_values, input_indicator) File "/home/pprod/.pyenv/versions/3.10.4/lib/python3.10/site-packages/talipp/indicators/Indicator.py", line 51, in initialize self.add(value) File "/home/pprod/.pyenv/versions/3.10.4/lib/python3.10/site-packages/talipp/indicators/Indicator.py", line 79, in add new_value = self._calculate_new_value() File "/home/pprod/.pyenv/versions/3.10.4/lib/python3.10/site-packages/talipp/indicators/ADX.py", line 93, in _calculate_new_value self.dx.append(100.0 * float(abs(self.pdi[-1] - self.mdi[-1])) / (self.pdi[-1] + self.mdi[-1])) ZeroDivisionError: float division by zero

popek777 commented 5 months ago

talib takes previous value when denominator is zero:

https://github.com/TA-Lib/ta-lib/blob/main/src/ta_func/ta_ADX.c#L780

popek777 commented 5 months ago

proposed fix:

      dx_denom = (self.pdi[-1] + self.mdi[-1])
      if dx_denom != 0:
          self.dx.append(100.0 * float(abs(self.pdi[-1] - self.mdi[-1])) / dx_denom)
      else:
          self.dx.append(self.dx[-1])
nardew commented 5 months ago

Thanks for spotting it (and even more so for proposing a fix), I will look into it asap.

popek777 commented 5 months ago

@nardew below full fix proposal (in case there is nothing in dx yet):

popek777 commented 5 months ago
     dx_denom = (self.pdi[-1] + self.mdi[-1])
      if dx_denom != 0:
          self.dx.append(100.0 * float(abs(self.pdi[-1] - self.mdi[-1])) / dx_denom)
      elif len(self.dx) > 0:
          self.dx.append(self.dx[-1])
      else:
          self.dx.append(0) # might be that None would be better choice
nardew commented 5 months ago

@popek777 I delivered a fix in #124. I opt for returning 0 instead of None at your last line because the subsequent code cannot work with Nones (e.g. summing of a list) and it would require too much special handling. If there is noone who really complains about it, I prefer to go this way.

nardew commented 5 months ago

Delivered in 2.1.1.

popek777 commented 5 months ago

excelent! thantks!