search

backtrader2 / backtrader

Python Backtesting library for trading strategies
https://www.backtrader.com
GNU General Public License v3.0
221 stars 52 forks source link

Addition of KeltnerChannel indicator #54

Closed FGU1 closed 3 years ago

FGU1 commented 3 years ago

Addition of keltner.py (KeltnerChannel indicator) in indicators/contrib and its associated test file test_ind_keltner.py in tests Modification of contrib/init for addition of keltner indicator.

neilsmurphy commented 3 years ago

This might be part of a bigger discussion. The intention originally when branching out from Backtrader was to have a place where fixes, repairs and updates could be implemented in a timely manner. It's my understanding that we are not keen on growing the code base. Perhaps the @backtrader2/admins could chime in?

neilsmurphy commented 3 years ago

@vladisld What do you think about this? Should we start adding in enhancements?

vladisld commented 3 years ago

Neil, you're right that our original intention was to focus only on bug fixing, while the reasoning was that we are not fully understand nor master the core of the framework. While being true for the framework core itself - I'm not sure adding new indicator (something that many know not only how to develop but also how to test) qualify as a framework core component.

Another reason for not adding new features was that we have not a lot of dedicated full/partial time developers to support even existing code, so support new half-backed features could be even a bigger problem. Still, not sure it applies to the indicator which is relatively simple and well tested.

So IMHO there is no hard objection to adding new indicator unless you have another view on this of cause.

Another question could be whether or not we would like to maintain all the new indicators withing the current backtrader repository or in the external one.

The agruments in favor keeping the indicators in the original backtrader repository:

neilsmurphy commented 3 years ago

I have mixed feelings about this as well. Nothing would make me happier than to have a vibrant community building out backtrader with new developments and the like. But in my view, we would seriously have to make a break with the original fork. As it currently stands, Daniel has moved on and I think he's only been back once since we started Backtrader2 to confirm pull requests into the original repo. Since he wishes to keep that repo in lock down, we may have to consider moving this fork forward with new development if that's something we want to do. That said, I don't think introducing one indicator is the way to start that.

On top of all the issues you mention above, we would also need to open up new documentation and evolve that with the new growth.

So my mixed emotions. I feel like I would love to see backtrader evolve, but at the same time, I don't think there seems to be the momentum for it.

I would vote to pass on this pull request for the time being until we are ready to expand the code base in new directions. Sorry about that @FGU1 we appreciate the work you put into this.

For the time being, I think Backtrader2 is still a bug fix area.

vladisld commented 3 years ago

@neilsmurphy with all my previous comments I both agree and respect your position on this.

Jens1989 commented 3 years ago

would having add-ons not motivate people to build and create and will then further attract people? I get your reasoning but just wondering if small additions which are safe to add could still be viable. Alternatively there could be some sort of add-ons folder which would have the additions the core system doesn't have in order to make a differentiation? Then in the future, these could be added to the main folders. Also, can you actually modify the documentation or is this also locked? I'd like to contribute to the core system but my level of skill is not good enough at the moment.

neilsmurphy commented 3 years ago

@Jens1989 Your points are valid and we've discussed these before. In fact, I just deleted a library for addons because there are so few people with limited time to work on this project. (Myself included and guilty). I would be open to building up a community that is vibrant and building this project, I'm just not sure adding in one offs is the way to do it.

Also, you your other question, formal documentation and the origianl codebase are in lock down. So we would need to fork everything and start fresh, which has problems. For example, all backtrader sites would still lead to the original.

neilsmurphy commented 3 years ago

I think we should close this pull request but move some of the comments and make a pinned issue for ongoing discussion. There is valuable discussion here and it should probably be kept ongoing. Thoughs?

neilsmurphy commented 3 years ago

The commentary about making additions has been moved to issue #58 B2: Creating new additions/enhancements vs. soley bug fixes. Future direction. #58 This pull request will be closed until the bigger questions of enhancements in B2 is resolved.

FGU1 commented 3 years ago

@neilsmurphy, I understand your point of view and it make sense. I will try to come soon with a proposal that fits original goals and desires for addons despite not enough people getting involved for these.