finta icon indicating copy to clipboard operation
finta copied to clipboard

added supertrend indicator

Open kitt-th opened this issue 5 years ago • 10 comments

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!

kitt-th avatar Dec 04 '20 09:12 kitt-th

Thanks for the contribution. Unfortunately I will not be able to review before Monday.

peerchemist avatar Dec 04 '20 15:12 peerchemist

no problem!

On Fri, 4 Dec 2020 at 16:25, peerchemist [email protected] 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 https://github.com/peerchemist/finta/pull/104#issuecomment-738844465, or unsubscribe https://github.com/notifications/unsubscribe-auth/AJWB6LVMI5SG4KUZYMWENTDSTD5MXANCNFSM4UNFIBJQ .

kitt-th avatar Dec 05 '20 09:12 kitt-th

Please add unit test.

peerchemist avatar Dec 06 '20 17:12 peerchemist

NP will get to that. and submit.

kitt-th avatar Dec 07 '20 10:12 kitt-th

@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.

kitt-th avatar Dec 14 '20 22:12 kitt-th

Hi, you are trying to push some weird dotfiles in the tests directory.

tests/data/.DS_Store

peerchemist avatar Dec 16 '20 16:12 peerchemist

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.

kitt-th avatar Dec 16 '20 20:12 kitt-th

Hi, @peerchemist It seems the PR has been cleaned by @kitt-th I would like to test this indicator, so it would be nice if you can merge it to the master. By the way I would be very intrested by the 'megatrend' indicator. Unfortunately I couldn't find the or source/formula for it...

ph4z avatar Feb 01 '21 22:02 ph4z

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.

peerchemist avatar Feb 05 '21 16:02 peerchemist

Did it not work for you with results not being as expected or was there some other error? @peerchemist

kitt-th avatar Feb 05 '21 19:02 kitt-th