Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug in overlap pivots #690

Open
smhoma opened this issue May 14, 2023 · 2 comments
Open

bug in overlap pivots #690

smhoma opened this issue May 14, 2023 · 2 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@smhoma
Copy link

smhoma commented May 14, 2023

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 smhoma added the bug Something isn't working label May 14, 2023
@twopirllc twopirllc removed their assignment May 15, 2023
@twopirllc twopirllc added the help wanted Extra attention is needed label May 15, 2023
@smhoma
Copy link
Author

smhoma commented May 31, 2023

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
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants