-
-
Notifications
You must be signed in to change notification settings - Fork 111
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
%B indicator is added. #264
Conversation
WalkthroughThis pull request introduces the %B indicator to the technical analysis library. The changes include adding a new Changes
Assessment against linked issues
Possibly related PRs
Poem
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 (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #264 +/- ##
==========================================
+ Coverage 93.63% 93.66% +0.02%
==========================================
Files 178 179 +1
Lines 6376 6404 +28
==========================================
+ Hits 5970 5998 +28
Misses 346 346
Partials 60 60 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
volatility/percent_b_test.go (1)
25-28
: Verify duplication logic.Using duplication to build separate sets (e.g., handling expected & actual data) is a neat trick. However, ensure that large input files won't significantly affect memory usage or runtime.
volatility/percent_b.go (1)
26-31
: Customizable period constructor.Enforcing custom periods is beneficial for advanced usage. Period validation (e.g., ensuring period ≥ 1) may be added to guard against invalid user input.
volatility/bollinger_bands.go (1)
39-42
: Enhance parameter checks if needed.Initializing Bollinger Bands with a configurable period fosters flexibility. For production-level usage, consider validating bounds (e.g., reject period < 2).
volatility/README.md (1)
657-717
: Enhance the %B indicator documentation for better clarity and consistency.The documentation for the %B indicator could be improved in several areas to match the quality and style of other indicators in the package:
- Add interpretation guidelines:
%B = (Close - Lower Band) / (Upper Band - Lower Band) + +Values above 1.0 indicate that price is above the upper band +Values below 0.0 indicate that price is below the lower band +Values between 0.0 and 1.0 indicate that price is within the bands
- Add a complete example:
// Example: percentB := volatility.NewPercentB[float64]() result := percentB.Compute(closings)
- Enhance field documentation:
type PercentB[T helper.Number] struct { - // BollingerBands + // BollingerBands is the underlying Bollinger Bands indicator used for calculations BollingerBands *BollingerBands[T] }🧰 Tools
🪛 Markdownlint (0.37.0)
662-662: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
volatility/testdata/percent_b.csv
is excluded by!**/*.csv
📒 Files selected for processing (6)
README.md
(1 hunks)trend/README.md
(2 hunks)volatility/README.md
(5 hunks)volatility/bollinger_bands.go
(1 hunks)volatility/percent_b.go
(1 hunks)volatility/percent_b_test.go
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
volatility/README.md
[uncategorized] ~402-~402: Possible missing article found.
Context: ...er](period int) *BollingerBands[T] ``` NewBollingerBandsWithPeriod function initializes a new Bollinger Ba...
(AI_HYDRA_LEO_MISSING_THE)
🪛 Markdownlint (0.37.0)
volatility/README.md
662-662: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (14)
volatility/percent_b_test.go (5)
14-19
: Well-structured test data type.
Defining a local struct for test data is clear and ensures type safety. The naming is also self-explanatory.
20-23
: Robust file handling approach.
Handling an error from the CSV reader with an immediate test fail is essential to avoid misleading test results if the file is missing or corrupted.
29-33
: Compute logic alignment.
The steps for computing %B appear consistent with the underlying indicator logic. Rounding results for easier test comparison is a good practice. Ensure the rounding does not mask borderline edge cases.
34-39
: Proactive idle period handling.
Skipping the elements in the expected slice by the idle period matches how the indicator ramps up. The final equality check ensures that actual and expected values are closely aligned.
42-49
: Meaningful string output test.
Verifying string output of the indicator for clarity helps ensure user-friendly logging and debugging. The test is straightforward and likely to catch any unintentional changes in the representation.
volatility/percent_b.go (5)
13-19
: Concise documentation and struct.
The docstring provides a clear formula for %B, and the struct design is minimal but sufficient, holding a reference to BollingerBands
.
21-24
: Streamlining default initialization.
Delegating to NewPercentBWithPeriod
avoids code duplication and ensures consistency between default and custom constructors.
33-50
: Valid concurrency design with channels.
The approach of duplicating channels, draining unused outputs, and computing in a separate goroutine is well-structured and consistent with Go’s concurrency patterns. The formula for %B
is correct.
52-56
: Consistent idle period logic.
Reusing the Bollinger Bands' idle period keeps logic centralized. This fosters maintainability.
58-60
: Clear representation string.
Utilizing fmt.Sprintf
and including the period is straightforward and user-friendly.
volatility/bollinger_bands.go (1)
36-37
: Reuse with parametric constructor.
Refractoring the default constructor to call NewBollingerBandsWithPeriod
is a clean solution for avoiding duplication. The code is succinct and consistent across the library.
README.md (1)
77-77
: New %B indicator addition looks good.
Adding %B to the list of volatility indicators expands the library’s capabilities. The link to the extended docs is consistent with other indicators, ensuring discoverability and uniform documentation style.
volatility/README.md (1)
396-404
: LGTM!
The documentation for NewBollingerBandsWithPeriod
follows the established pattern and provides clear information about the function's purpose.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~402-~402: Possible missing article found.
Context: ...er](period int) *BollingerBands[T] ``` NewBollingerBandsWithPeriod function initializes a new Bollinger Ba...
(AI_HYDRA_LEO_MISSING_THE)
trend/README.md (1)
1850-1850
: LGTM!
The method signature update improves code clarity by removing the unused receiver name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
volatility/percent_b_test.go (2)
15-35
: Enhance the example to be more illustrative.The example could be more comprehensive by showing multiple output values to better demonstrate how %B changes across different market conditions (e.g., overbought, oversold, and neutral scenarios).
Consider modifying the example to show more output values:
func ExamplePercentB() { // Closing prices closes := helper.SliceToChan([]float64{ 318.600006, 315.839996, 316.149994, 310.570007, 307.779999, 305.820007, 305.98999, 306.390015, 311.450012, 312.329987, 309.290009, 301.910004, 300, 300.029999, 302, 307.820007, 302.690002, 306.48999, 305.549988, 303.429993, + 325.600006, // Strong upward move + 290.839996, // Strong downward move }) // Initialize the %B indicator percentB := volatility.NewPercentB[float64]() // Compute %B result := percentB.Compute(closes) // Round digits result = helper.RoundDigits(result, 2) fmt.Println(helper.ChanToSlice(result)) - // Output: [0.3] + // Output: [0.3, 0.9, 0.1] // Shows normal, overbought, and oversold conditions }
65-72
: Consider adding test cases with different periods.While the test is good, it could be more comprehensive by testing the string representation with different periods.
Consider using table-driven tests:
func TestPercentBString(t *testing.T) { - expected := "%B(10)" - actual := volatility.NewPercentBWithPeriod[float64](10).String() - - if actual != expected { - t.Fatalf("actual %v expected %v", actual, expected) - } + tests := []struct { + period int + expected string + }{ + {10, "%B(10)"}, + {20, "%B(20)"}, + {50, "%B(50)"}, + } + + for _, tt := range tests { + t.Run(tt.expected, func(t *testing.T) { + actual := volatility.NewPercentBWithPeriod[float64](tt.period).String() + if actual != tt.expected { + t.Fatalf("period %d: actual %v expected %v", tt.period, actual, tt.expected) + } + }) + } }volatility/README.md (1)
681-706
: Replace hard tabs with spaces in the example code.The example code in the documentation contains hard tabs which should be replaced with spaces for consistent formatting across different editors and platforms.
Replace all hard tabs with appropriate number of spaces in the example code section.
🧰 Tools
🪛 Markdownlint (0.37.0)
682-682: Column: 1
Hard tabs(MD010, no-hard-tabs)
684-684: Column: 1
Hard tabs(MD010, no-hard-tabs)
685-685: Column: 1
Hard tabs(MD010, no-hard-tabs)
689-689: Column: 1
Hard tabs(MD010, no-hard-tabs)
690-690: Column: 1
Hard tabs(MD010, no-hard-tabs)
691-691: Column: 1
Hard tabs(MD010, no-hard-tabs)
692-692: Column: 1
Hard tabs(MD010, no-hard-tabs)
693-693: Column: 1
Hard tabs(MD010, no-hard-tabs)
694-694: Column: 1
Hard tabs(MD010, no-hard-tabs)
695-695: Column: 1
Hard tabs(MD010, no-hard-tabs)
697-697: Column: 1
Hard tabs(MD010, no-hard-tabs)
698-698: Column: 1
Hard tabs(MD010, no-hard-tabs)
700-700: Column: 1
Hard tabs(MD010, no-hard-tabs)
701-701: Column: 1
Hard tabs(MD010, no-hard-tabs)
703-703: Column: 1
Hard tabs(MD010, no-hard-tabs)
704-704: Column: 1
Hard tabs(MD010, no-hard-tabs)
706-706: Column: 1
Hard tabs(MD010, no-hard-tabs)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
volatility/README.md
(5 hunks)volatility/percent_b.go
(1 hunks)volatility/percent_b_test.go
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- volatility/percent_b.go
🧰 Additional context used
🪛 Markdownlint (0.37.0)
volatility/README.md
710-710: Expected: h3; Actual: h4
Heading levels should only increment by one level at a time
(MD001, heading-increment)
682-682: Column: 1
Hard tabs
(MD010, no-hard-tabs)
684-684: Column: 1
Hard tabs
(MD010, no-hard-tabs)
685-685: Column: 1
Hard tabs
(MD010, no-hard-tabs)
689-689: Column: 1
Hard tabs
(MD010, no-hard-tabs)
690-690: Column: 1
Hard tabs
(MD010, no-hard-tabs)
691-691: Column: 1
Hard tabs
(MD010, no-hard-tabs)
692-692: Column: 1
Hard tabs
(MD010, no-hard-tabs)
693-693: Column: 1
Hard tabs
(MD010, no-hard-tabs)
694-694: Column: 1
Hard tabs
(MD010, no-hard-tabs)
695-695: Column: 1
Hard tabs
(MD010, no-hard-tabs)
697-697: Column: 1
Hard tabs
(MD010, no-hard-tabs)
698-698: Column: 1
Hard tabs
(MD010, no-hard-tabs)
700-700: Column: 1
Hard tabs
(MD010, no-hard-tabs)
701-701: Column: 1
Hard tabs
(MD010, no-hard-tabs)
703-703: Column: 1
Hard tabs
(MD010, no-hard-tabs)
704-704: Column: 1
Hard tabs
(MD010, no-hard-tabs)
706-706: Column: 1
Hard tabs
(MD010, no-hard-tabs)
662-662: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
712-712: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (1)
volatility/percent_b_test.go (1)
37-63
: LGTM! Well-structured test implementation.
The test follows good practices:
- Data-driven approach using CSV file
- Proper error handling
- Clear separation of input and expected data
- Effective use of helper functions
Describe Request
%B indicator is added.
Fixed #254
Change Type
New indicator.
Summary by CodeRabbit
New Features
%B
, enhancing technical analysis capabilities.%B
indicator, includingNewPercentB
andNewPercentBWithPeriod
.NewBollingerBandsWithPeriod
.Bug Fixes
Tests
%B
indicator to validate functionality and string representation.