Negative Volume Index Strategy is added. #237
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #237 +/- ##
==========================================
+ Coverage 92.87% 92.93% +0.05%
==========================================
Files 168 169 +1
Lines 4353 4416 +63
==========================================
+ Hits 4043 4104 +61
- Misses 251 253 +2
Partials 59 59 ☔ View full report in Codecov by Sentry. |
WalkthroughThe pull request introduces significant updates to the Indicator Go module, primarily reflected in the Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (4)
strategy/volume/negative_volume_index_strategy_test.go (2)
17-37: LGTM: Well-structured test function with a minor suggestion.The test function is well-organized, using separate CSV files for input data and expected results. Error handling is properly implemented, and helper functions are utilized effectively.
Consider adding a comment explaining the choice of 12 as the parameter for NewNegativeVolumeIndexStrategyWith. This would provide context for future maintainers:
+ // Initialize strategy with a period of 12, which is a common choice for NVI nvis := volume.NewNegativeVolumeIndexStrategyWith(12)
39-55: LGTM: Well-structured test function with suggestions for improvement.The test function for the strategy report is well-organized and follows similar patterns to the previous test, which is good for consistency. The use of defer for cleanup is a good practice.
Consider the following improvements:
- Add a check to verify the content of the generated HTML file:
content, err := os.ReadFile(fileName) if err != nil { t.Fatal(err) } if len(content) == 0 { t.Fatal("Generated report is empty") }
- Use a temporary directory for the output file to avoid potential conflicts:
tempDir, err := os.MkdirTemp("", "test") if err != nil { t.Fatal(err) } defer os.RemoveAll(tempDir) fileName := filepath.Join(tempDir, "negative_volume_index_strategy.html")
- Add a comment explaining why the strategy is initialized without parameters in this test, as it differs from the previous test:
// Initialize strategy without parameters to test the default configuration nvis := volume.NewNegativeVolumeIndexStrategy()strategy/volume/README.md (1)
219-223: Minor grammatical fix neededThere's a small grammatical issue in the description of the NewNegativeVolumeIndexStrategy function.
Please update the description as follows:
-NewNegativeVolumeIndexStrategy function initializes a new Negative Volume Index strategy instance with the default parameters. +The NewNegativeVolumeIndexStrategy function initializes a new Negative Volume Index strategy instance with the default parameters.🧰 Tools
🪛 LanguageTool
[uncategorized] ~222-~222: Possible missing article found.
Context: ...egy() *NegativeVolumeIndexStrategy ``` NewNegativeVolumeIndexStrategy function initializes a new Negative Vol...(AI_HYDRA_LEO_MISSING_THE)
README.md (1)
144-144: Great addition, but please fix the indentation.The addition of the Negative Volume Index Strategy is a valuable contribution to the list of Volume Strategies. However, there's a minor formatting issue:
Please replace the hard tab with spaces to maintain consistency with the rest of the file. You can apply this diff:
- [Negative Volume Index Strategy](strategy/volume/README.md#type-negativevolumeindexstrategy) + [Negative Volume Index Strategy](strategy/volume/README.md#type-negativevolumeindexstrategy)This change will ensure that the indentation is consistent with the other items in the list.
🧰 Tools
🪛 Markdownlint
144-144: Column: 2
Hard tabs(MD010, no-hard-tabs)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
strategy/volume/testdata/negative_volume_index_strategy.csvis excluded by!**/*.csv
📒 Files selected for processing (5)
- README.md (1 hunks)
- strategy/volume/README.md (3 hunks)
- strategy/volume/negative_volume_index_strategy.go (1 hunks)
- strategy/volume/negative_volume_index_strategy_test.go (1 hunks)
- strategy/volume/volume.go (1 hunks)
🧰 Additional context used
🪛 Markdownlint
README.md
144-144: Column: 2
Hard tabs(MD010, no-hard-tabs)
🪛 LanguageTool
strategy/volume/README.md
[uncategorized] ~222-~222: Possible missing article found.
Context: ...egy() *NegativeVolumeIndexStrategy ``` NewNegativeVolumeIndexStrategy function initializes a new Negative Vol...(AI_HYDRA_LEO_MISSING_THE)
🔇 Additional comments (4)
strategy/volume/negative_volume_index_strategy_test.go (1)
7-15: LGTM: Appropriate imports for testing.The import statements are well-organized and include all necessary packages for testing the NegativeVolumeIndexStrategy. Using the versioned import
github.com/cinar/indicator/v2is a good practice for maintaining compatibility.strategy/volume/README.md (3)
41-46: LGTM: Index section updated correctlyThe new entries for NegativeVolumeIndexStrategy have been added to the index section, following the established pattern for other strategies. This ensures consistency and makes it easy for users to find information about the new strategy.
63-70: LGTM: New constant properly defined and documentedThe new constant
DefaultNegativeVolumeIndexStrategyEmaPeriodis correctly defined with a value of 255 and is accompanied by an appropriate comment explaining its purpose. This follows the established pattern for constant definitions in the file.
201-260: LGTM: NegativeVolumeIndexStrategy documentation addedThe documentation for NegativeVolumeIndexStrategy and its methods has been added successfully. The structure and content are consistent with the documentation of other strategies in the file, providing clear information about the new strategy's purpose and usage.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~222-~222: Possible missing article found.
Context: ...egy() *NegativeVolumeIndexStrategy ``` NewNegativeVolumeIndexStrategy function initializes a new Negative Vol...(AI_HYDRA_LEO_MISSING_THE)
Describe Request
Negative Volume Index Stratey is added.
Change Type
New strategy.
Summary by CodeRabbit
Release Notes
New Features
Negative Volume Index Strategyfor enhanced trading decisions.Bug Fixes
Documentation