-
Notifications
You must be signed in to change notification settings - Fork 158
✨ Allow config difference core trend timeframe #156
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did some improvements to the code already as I was setting up a development environment for this PR.
I'll get to more thorough debugging of the code soon & will see if all checks out and if I can find a way to do this with a bit less code duplication.
@taina0407 already thank you for this PR! 🙏
Feel free to also try to find a solution for this with less code duplication 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also tested with PyCharms debugger and the implementation looks good to me! 😄
I took the liberty of also removing some double/unused dataframe columns which I noticed during my debugging, this should result in less memory usage & perhaps a faster HyperOpt too 🙂
However I still need to thoroughly test both of our changes now before I get to merging.
Feel free to already append some test results:
- From before the PR implementation as comparison
- Some of the best results yielded with the new feature implemented
So we can be sure & document it in this PR that it does indeed bring improvements
@taina0407 {
"timeframe": "30m",
"trend_timeframe": "1d",
"backtest_timeframe": "5m"
}
|
TODO: