-
-
Notifications
You must be signed in to change notification settings - Fork 717
added supertrend indicator #104
base: master
Are you sure you want to change the base?
Conversation
|
Thanks for the contribution. Unfortunately I will not be able to review before Monday. |
|
no problem!
…On Fri, 4 Dec 2020 at 16:25, peerchemist ***@***.***> wrote:
Thanks for the contribution. Unfortunately I will not be able to review
before Monday.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#104 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AJWB6LVMI5SG4KUZYMWENTDSTD5MXANCNFSM4UNFIBJQ>
.
|
|
Please add unit test. |
|
NP will get to that. and submit. |
|
@arul67800 - thanks for pointing that out. The range should start at 'period +1' as supertrend calculations reference the value of the previous time series/candle. Updated commit coming next. |
|
Hi, you are trying to push some weird dotfiles in the
|
|
Hi, sorry about that - my first pull requests so I will check all the files again, clean up and commit without those extra things. Thanks for your patience. I must have made a mistake when adding files. |
… that the finder adds. Used gitignore to filter them out
|
Hi, |
|
Unfortunately this indicator did not work for me in my tests, and I did not have time in a good while to play and hack on it. |
|
Did it not work for you with results not being as expected or was there some other error? @peerchemist |
Hi @peerchemist , I added the supertrend indicator. I cross checked results with the original pinescript indicator linked in the indicator description.
black also reformatted some of the comments/lines in finta.py
new code is on lines 1408-1466
this is my first git pull request, so I hope I did it right. let me know what you think!