Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reopening of PR 363 #380

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

jbdelmundo
Copy link
Collaborator

Reopening from PR 363

Description
Added the following changes:

  • Docstrings in BaseStrategy
  • Added a new logging module instead of simply doing print() inside the BaseStrategy
  • Logging inside the strategy can be accessed using self.logging.log()
  • Separated the function of adding periodic cash
  • buy_signal() and sell_signal() can now return (signal, buy/sell_prop) instead of just boolean (still backwards compatible)
  • Added trade_history
  • Set default signal to False (no action)

Checklist
[ X] I am making a pull request from a branch other than master
[ X] I have read the CONTRIBUTING.md
[ X] I have added/edited documentation in a relevant docs/docusaurus markdown file
[X ] (For new docs, check if not applicable) I have added the id of a new docs md to the docs/docusaurus/sidebars.js file

@mikeejazmines
Copy link
Collaborator

hi @jbdelmundo! Sorry for the delay, am reviewing your PR now but could you fix all the merge conflicts? I agree with the implementation and can merge this once the merge conflicts are fixed and when I'm done reviewing and testing everything else :)

Thanks so much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants