twopirllc / pandas-ta

Technical Analysis Indicators - Pandas TA is an easy to use Python 3 Pandas Extension with 150+ Indicators
https://twopirllc.github.io/pandas-ta/
MIT License
5.02k stars 986 forks source link

bug in overlap pivots #690

Open smhoma opened 1 year ago

smhoma commented 1 year ago

Which version are you running? The lastest version is on Github. Pip is for major releases. I am using python 3.10 and pandas_ta 0.3.91b0.

Do you have TA Lib also installed in your environment? yes

*Describe the bug I was working with forex daily data and I figured out that the pivots indicator has issue in calculating pivot points of Mondays based on Fridays and it returns NaN for pivots of Mondays. I think that with the default 'D' anchor, for Mondays, the code is looking for Sunday data which does not exist since its weekend in Forex market, instead of using data of Friday, which is the last trading day in Forex market. I used anchor 'B' too, but i got this error:

> KeyError                                  Traceback (most recent call last)
> File [c:\Users\MAHDI\Miniconda3\envs\torch310\lib\site-packages\pandas\_libs\tslibs\timedeltas.pyx:719](file:///C:/Users/MAHDI/Miniconda3/envs/torch310/lib/site-packages/pandas/_libs/tslibs/timedeltas.pyx:719), in pandas._libs.tslibs.timedeltas.parse_timedelta_unit()
> 
> KeyError: 'b'
> 
> During handling of the above exception, another exception occurred:
> 
> ValueError                                Traceback (most recent call last)
> Cell In[30], line 2
>       1 df_test = df_ta.copy()
> ----> 2 df_test.ta.pivots(anchor= "B")
> 
> File [c:\Users\MAHDI\Miniconda3\envs\torch310\lib\site-packages\pandas_ta\core.py:1263](file:///C:/Users/MAHDI/Miniconda3/envs/torch310/lib/site-packages/pandas_ta/core.py:1263), in AnalysisIndicators.pivots(self, method, anchor, **kwargs)
>    1261 low = self._get_column(kwargs.pop("low", "low"))
>    1262 close = self._get_column(kwargs.pop("close", "close"))
> -> 1263 result = pivots(
>    1264     open_=open_, high=high, low=low, close=close,
>    1265     method=method, anchor=anchor, **kwargs,
>    1266 )
>    1267 return self._post_process(result, **kwargs)
> 
> File [c:\Users\MAHDI\Miniconda3\envs\torch310\lib\site-packages\pandas_ta\overlap\pivots.py:243](file:///C:/Users/MAHDI/Miniconda3/envs/torch310/lib/site-packages/pandas_ta/overlap/pivots.py:243), in pivots(open_, high, low, close, method, anchor, **kwargs)
>     240 df[f"{_props}_R1"], df[f"{_props}_R2"] = r1, r2
>     241 df[f"{_props}_R3"], df[f"{_props}_R4"] = r3, r4
> --> 243 df.index = df.index + Timedelta(1, anchor.lower())
>     244 if freq is not anchor:
>     245     df = df.reindex(dt_index, method="ffill")
> 
> File [c:\Users\MAHDI\Miniconda3\envs\torch310\lib\site-packages\pandas\_libs\tslibs\timedeltas.pyx:1694](file:///C:/Users/MAHDI/Miniconda3/envs/torch310/lib/site-packages/pandas/_libs/tslibs/timedeltas.pyx:1694), in pandas._libs.tslibs.timedeltas.Timedelta.__new__()
> 
> File [c:\Users\MAHDI\Miniconda3\envs\torch310\lib\site-packages\pandas\_libs\tslibs\timedeltas.pyx:721](file:///C:/Users/MAHDI/Miniconda3/envs/torch310/lib/site-packages/pandas/_libs/tslibs/timedeltas.pyx:721), in pandas._libs.tslibs.timedeltas.parse_timedelta_unit()
> 
> ValueError: invalid unit abbreviation: b

Screenshots 1

Thanks for using Pandas TA!

smhoma commented 1 year ago

ok I realized that the infer_freq(dt_index) in line 175 of the pivots.py file can not infer my data frequency and it returns freq=None. so the resample function creates those weekend dates and all those NaNs are from here. i added df.dropna(inplace=True) in the if part after the resamplings in the source code and the problem in my case was resolved. I don't if it can cause any side effects in other usages and timeframes. so i didn't make a pull request.

therefore lines 183-192 of pivots.py is like this:

if freq is not anchor:
    df = DataFrame(
            data={
                "open": open_.resample(anchor).first(),
                "high": high.resample(anchor).max(),
                "low": low.resample(anchor).min(),
                "close": close.resample(anchor).last()
            }
        )
        df.dropna(inplace=True)
twopirllc commented 1 year ago

Hi @smhoma,

i added df.dropna(inplace=True) in the if part after the resamplings in the source code and the problem in my case was resolved. I don't if it can cause any side effects in other usages and timeframes. so i didn't make a pull request.

Awesome you were able to isolate and solve the issue for your use case! 😎 Understandable if you did not want to make a PR so as not to make a breaking change. I'll do it and take the hit. 🤣

If it does cause issues, we can designate a keyword argument name to run df.dropna(inplace=True) when True. Do you have any good ideas for a keyword argument name to use? I have yet to think of a decent keyword argument name since I believe it would also apply for crypto and futures data as well. 😑

Kind Regards, KJ