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

Add renko chart support #47

Merged
merged 18 commits into from
Mar 12, 2020
Merged

Add renko chart support #47

merged 18 commits into from
Mar 12, 2020

Conversation

coffincw
Copy link
Collaborator

@coffincw coffincw commented Mar 7, 2020

Adds the ability to create Renko charts by passing in type='renko' and optional renko_params dictionary to specify brick_size and atr_length to the plot() function. Also includes examples in the README and customization_and_styles.ipynb

@coffincw coffincw mentioned this pull request Mar 8, 2020
Copy link
Collaborator

@DanielGoldfarb DanielGoldfarb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! Thank you Caleb. Really nice job implementing Renko charts.

I've added some comments to various places in the code below, where things can be changed to protect the code better, and/or make it cleaner (overall it's fantastic though!). And there is the one issue regarding mav which I am not sure exactly how it should be handled, but I think what we are doing there is not necessarily what a Renko chartist wants to see. All the best. And thank you! --Daniel

src/mplfinance/_utils.py Outdated Show resolved Hide resolved
src/mplfinance/plotting.py Outdated Show resolved Hide resolved
src/mplfinance/plotting.py Show resolved Hide resolved
src/mplfinance/plotting.py Show resolved Hide resolved
src/mplfinance/_utils.py Show resolved Hide resolved
src/mplfinance/_utils.py Outdated Show resolved Hide resolved
@coffincw
Copy link
Collaborator Author

Thanks for leaving feedback! I'll check it out and let you know what I think later today!

Copy link
Collaborator

@DanielGoldfarb DanielGoldfarb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see comments inline. Thank you!

src/mplfinance/_utils.py Outdated Show resolved Hide resolved
src/mplfinance/_utils.py Show resolved Hide resolved
src/mplfinance/_utils.py Outdated Show resolved Hide resolved
src/mplfinance/_utils.py Outdated Show resolved Hide resolved
src/mplfinance/_utils.py Outdated Show resolved Hide resolved
src/mplfinance/_utils.py Outdated Show resolved Hide resolved
src/mplfinance/plotting.py Show resolved Hide resolved
src/mplfinance/plotting.py Outdated Show resolved Hide resolved
src/mplfinance/_utils.py Outdated Show resolved Hide resolved
@coffincw
Copy link
Collaborator Author

Thank you so much for the feedback, I really appreciate the comments you left! Let me know if you have any additional fixes/comments!

@DanielGoldfarb DanielGoldfarb merged commit e04066b into matplotlib:master Mar 12, 2020
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