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

Fix varPPO strategy and TSI indicator #1799

Merged
merged 6 commits into from
Jan 30, 2018
Merged

Fix varPPO strategy and TSI indicator #1799

merged 6 commits into from
Jan 30, 2018

Conversation

Paulovms
Copy link
Contributor

  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
    Bug fix.

  • What is the current behavior? (You can also link to an open issue here)
    TSI values on varPPO and TSI strategies stay above 95 after a lot of candles.
    The first momentum value is the full close value and this put a big error on next EMAs.
    varPPO use the PPO value from this.indicators.ppo.ppo to calculate PPO histogram, but this value is not been updated and are always 0. In fact, since Gekko 0.5.11, the PPO indicator exposes the PPO histogram directly to the API on this.indicators.ppo.result.ppohist.

  • What is the new behavior (if this is a feature change)?
    TSI indicator calculate the first value after get the first previous close and varPPO get ppoHist directly from PPO indicator.

Copy link
Owner

@askmike askmike left a comment

Choose a reason for hiding this comment

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

see question, will merge after you can let us know :)

@@ -39,7 +39,7 @@ var retryForever = {
};

// Probably we need to update these string
var recoverableErrors = new RegExp(/(SOCKETTIMEDOUT|TIMEDOUT|CONNRESET|CONNREFUSED|NOTFOUND|StatusCodeError: 429|StatusCodeError: 5)/)
var recoverableErrors = new RegExp(/(SOCKETTIMEDOUT|TIMEDOUT|CONNRESET|CONNREFUSED|NOTFOUND|429|443|5\d\d)/g);
Copy link
Owner

Choose a reason for hiding this comment

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

Is this supposed to be part of this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I’m sorry, this will update the Bitfinex string. Maybe I’ll submit another PR.

Copy link
Owner

Choose a reason for hiding this comment

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

Maybe it needs to be updated, did you add this specifically because it caught errors better?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this string caught erros better.It can be committed.

@askmike
Copy link
Owner

askmike commented Jan 30, 2018

thanks!

@askmike askmike merged commit d703a91 into askmike:develop Jan 30, 2018
This was referenced Mar 21, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants