kylejusticemagnuson / pyti

Python library of various financial technical indicators
MIT License
646 stars 169 forks source link

ADX #12

Closed grich4 closed 6 years ago

grich4 commented 6 years ago

When the ADX function is called and run it provides unreasonably high numbers, cuts out on data gaps and never comes back, as opposed to NaN and then going back to evaluated numbers. Not sure what is going on, but something is not right. Perhaps the issue is with another one of the functions ADX uses

kylejusticemagnuson commented 6 years ago

Hi @grich4,

Thanks for reporting the issue, I'll look into this when I have some free time. Could you let me know which python version you are using?

grich4 commented 6 years ago

3.6

grich4 commented 6 years ago

I've gone through your code and do not understand what is causing the issues I'm experiencing. I've attached the data i've been using. Processing the data via this package shows there are no instances where ADX is under 25, when that occurrence should be a bit more frequent, and high values occur around 400, which doesn't make sense. 40 does make sense, but the formulas I checked this against all matched.

five_min.txt

vincentropy commented 6 years ago

I have run into this problem as well. I've done a little bit of digging and found a few issues: Most directly, some parenthesis are missing from the equation for ADX leading to erroneous values.

Since ADX depends on ATR and SMMA, I looked at those functions as well. ATR is defined recursively which does not play nice with nan values (one nan value in the middle of a dataset can cause all subsequent values to be nan).

The values for smma are not the same as what I get using other packages such as pandas.

I would be happy to work on fixing these issues if you are open to a pull-request.

kylejusticemagnuson commented 6 years ago

Always open to PRs and any suggestions.

vincentropy commented 6 years ago

@kylejusticemagnuson I noticed that the pyti package only depends on numpy and not pandas, is this by design or is it ok to include pandas as an additional dependency? I ask because there is a nice exponential weighted moving average function in pandas that might be useful when computing smma.

kylejusticemagnuson commented 6 years ago

Adding Pandas as an additional dependency is fine.

I did leave it out on purpose when I first made this since I was using this in a project running on AWS Lambda and there were compatibility issues with using Pandas. There are solutions out there now to using Pandas on Lambda since I ran into that issue if I, or anyone else, wants to use pyti in a lambda project.

vincentropy commented 6 years ago

@kylejusticemagnuson Thanks for the quick response! I'll let you know once I have some progress to show.

kylejusticemagnuson commented 6 years ago

Thanks a lot!

I have been busy recently and haven't been able to deep dive into any issues/improvements

kylejusticemagnuson commented 6 years ago

New version released v0.2.2 with this fix