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

compute_interval must use total seconds #59

Merged
merged 4 commits into from
Jan 17, 2020

Conversation

xmatthias
Copy link
Member

@xmatthias xmatthias commented Jan 16, 2020

compute_interval must use total seconds, otherwise it's wrong for every timeframe > 1 day

Discovered while analyzing freqtrade/freqtrade-strategies#62

which uses 4h timeframe and resamples to 2d - so resampled_merge() returns always resampled_0_sma instead of resampled_2880_sma.

Documentation improvement

Also adds a proper example for resampling - since this continues to raise questions.

Copy link
Member

@hroff-1902 hroff-1902 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@hroff-1902 hroff-1902 merged commit dd62077 into master Jan 17, 2020
@xmatthias xmatthias deleted the fix/bug_in_compute_interval branch January 17, 2020 05:34
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