-
-
Notifications
You must be signed in to change notification settings - Fork 41
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
Fix ichimoku indicators calculation (#452) #455
Conversation
ccbeffb
to
5f811e4
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #455 +/- ##
==========================================
+ Coverage 65.58% 72.37% +6.79%
==========================================
Files 94 102 +8
Lines 1537 1665 +128
Branches 181 254 +73
==========================================
+ Hits 1008 1205 +197
+ Misses 517 448 -69
Partials 12 12 ☔ 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.
I wonder if we should keep a single naming. Do you know if the Senkou Span or Leading Span is much better known? Perhaps then we can name both the object fields and the method names the same.
That's a good point. Let's go with your suggestion.
…On Wed, Apr 17, 2024 at 12:46 PM Thomas Heniart ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/indicator/momentum/ichimokuCloud.ts
<#455 (comment)>:
>
- return {
- conversion,
- base,
- leadingSpanA,
- leadingSpanB,
- laggingSpan,
- };
+ return {
+ conversion: tenkanSen,
+ base: kijunSen,
+ leadingSpanA: calculateSenkouSpanA({tenkanSen, kijunSen, medium}),
Tenkan, Kijun, SSA, SSB, and LagginSpan terms are widely used among people
working with Ichimoku.
I didn't want to introduce a "breaking change" in the code base, that's
why I didn't touch IchimokuCloudResult
If you think it's to changeIchimokuCloudResult interface then it could be
export interface IchimokuCloudResult {
kijun: number[];
tenkan: number[];
ssa: number[];
ssb: number[];
laggingSpan: number[];}
—
Reply to this email directly, view it on GitHub
<#455 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AANMH3CDUT3BYRTZZF6RYTDY53GS3AVCNFSM6AAAAABGKH25TCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAMBWHE4TQOBWGQ>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
e309122
to
4920830
Compare
Done @cinar, I think it's ready for review 👍 |
Thank you very much! I am merging it! |
Fix #452