Skip to content
This repository has been archived by the owner on Feb 15, 2022. It is now read-only.

Set default value for min_periods in trade command #1738

Merged
merged 1 commit into from
Oct 8, 2018

Conversation

BAKFR
Copy link
Contributor

@BAKFR BAKFR commented Oct 8, 2018

Using paper mode, the min_periods option was used without being defined. It would lead to date calculation returning NaN, and thus provoking errors (at least on Binance exchange).

This commit fixes this.

Using paper mode, the `min_periods` option was used without being defined. It would lead to date calculation returning `NaN`, and thus provoking errors (at least on Binance exchange).

This commit fixes this.
Copy link
Owner

@DeviaVir DeviaVir left a comment

Choose a reason for hiding this comment

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

👍

@DeviaVir DeviaVir merged commit e49acc9 into DeviaVir:unstable Oct 8, 2018
@olexiyb
Copy link
Contributor

olexiyb commented Nov 19, 2018

This change actually impacting for example sar strategy and when we run paper trading it's failing with

C:\Users\olexi\WebstormProjects\zenbot-web\packages\zenbot-api\zenbot\extensions\strategies\sar\strategy.js:32
s.sar = Math.max(s.lookback[1].high, s.lookback[0].high)
^

TypeError: Cannot read property 'high' of undefined
at Object.calculate (C:\Users\olexi\WebstormProjects\zenbot-web\packages\zenbot-api\zenbot\extensions\strategies\sar\strategy.js:32:42)

Which strategy actually was failing?

@BAKFR
Copy link
Contributor Author

BAKFR commented Nov 19, 2018

@olexiyb I don't remember, but I've tested several strategies that was failing.

Basically, before this commit, min_periods was used several times (in trades.js) without it being initialized. This was causing an error to all strategies not requiring min_periods has a strategy option.

Can you try to call the sar strategy with the min_periods defined manually ? It's just a guess, but I thing it could be explained by the default value not being set at 52 (as requested in the sar strategy), thus creating only one period in s.lookback.

Edit: Feel free to mentions me in another issue instead of commenting in this one.

BAKFR added a commit to BAKFR/zenbot that referenced this pull request Jan 29, 2019
Using paper mode, the `min_periods` option was used without being defined. It would lead to date calculation returning `NaN`, and thus provoking errors (at least on Binance exchange).

This commit fixes this.
YarnSeemannsgarn pushed a commit to YarnSeemannsgarn/zenbot that referenced this pull request Apr 15, 2020
Using paper mode, the `min_periods` option was used without being defined. It would lead to date calculation returning `NaN`, and thus provoking errors (at least on Binance exchange).

This commit fixes this.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants