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

Refactors AverageDirectionalIndex and AverageDirectionalMovementIndexRating #2667

Conversation

AlexCatarino
Copy link
Member

@AlexCatarino AlexCatarino commented Nov 1, 2018

Description

  • Fixes TrueRange computation: it was not using the H-L range;
  • Fixes SmoothedDirectionalMovementMinus that used a constant value instead of the defined period;
  • Use a WilderMovingAverage to compute ADX based on DX.
  • Fixes AverageDirectionalMovementIndexRating only be ready when there is enough past values.
  • Removes external data for AverageDirectionalMovementIndexRating and points to column in AverageDirectionalMovementIndex external data.

Related Issue

Closes #2666

Motivation and Context

Fix bugs in indicators.

Requires Documentation Change

No.

How Has This Been Tested?

Unit and regression tests.
Update unit tests using data from: https://stockcharts.com/school/doku.php?id=chart_school:technical_indicators:average_directional_index_adx

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Non-functional change (xml comments/documentation/etc)

Checklist:

  • My code follows the code style of this project.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • My branch follows the naming convention bug-<issue#>-<description or feature-<issue#>-<description>

…Rating

 - Fixes `TrueRange` computation: it was not using the H-L range;
- Fixes `SmoothedDirectionalMovementMinus` that used a constant value instead of the defined period;
- Use a `WilderMovingAverage` to compute ADX based on DX.
- Fixes `AverageDirectionalMovementIndexRating` only be ready when there is enough past values.
- Removes external data for `AverageDirectionalMovementIndexRating` and points to column in `AverageDirectionalMovementIndex` external data.

Closes QuantConnect#2666
@AlexCatarino AlexCatarino force-pushed the bug-2666-refactors-average-directional-index branch from f09d1af to bb99123 Compare November 2, 2018 22:53
@AlexCatarino AlexCatarino changed the title Refactors AverageDirectionalIndex Refactors AverageDirectionalIndex and AverageDirectionalMovementIndexRating Nov 2, 2018
Copy link
Contributor

@mchandschuh mchandschuh left a comment

Choose a reason for hiding this comment

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

Awesome stuff, might also want to include the source of the external data in the commit message (minimally in the PR description). This would allow someone else to independently verify that the data source is reputable and that the data contained herein is correct.

@AlexCatarino
Copy link
Member Author

@mchandschuh, I have added the source of the external data in the indicator class file and have updated the PR description to include it too.

@jaredbroad jaredbroad merged commit 81e6f1f into QuantConnect:master Nov 6, 2018
@AlexCatarino AlexCatarino deleted the bug-2666-refactors-average-directional-index branch November 6, 2018 11: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.

Review AverageDirectionalIndex Indicator
3 participants