cinar / indicatorts

IndicatorTS - Stock technical indicators and strategies in TypeScript for browser and server programs.
MIT License
296 stars 53 forks source link

Attempted to fix the Projection Upper band and lower band calculation #329

Open abhishekmittal15 opened 1 year ago

abhishekmittal15 commented 1 year ago

Attempting to fix #218

Modified the projection Oscillator function by changing the calculation of the arrays pu and pl according to what I suspect the issue is, as I mentioned in the conversation of #218 Hi @cinar , Please have a look at the code which has LOOK HERE written, that is the changes made to the implementation mainly. Let me know what you think about it.

The file is messy right now, I will clean it after you have approved the changes in the logic part of the code.

I also needed advice on how I can test this indicator.

Thanks

codecov-commenter commented 1 year ago

Codecov Report

Merging #329 (e10fe1e) into main (306893d) will increase coverage by 1.36%. The diff coverage is 97.43%.

@@            Coverage Diff             @@
##             main     #329      +/-   ##
==========================================
+ Coverage   64.64%   66.00%   +1.36%     
==========================================
  Files          93       93              
  Lines        1533     1562      +29     
  Branches      183      186       +3     
==========================================
+ Hits          991     1031      +40     
+ Misses        529      518      -11     
  Partials       13       13              
Impacted Files Coverage Δ
src/indicator/volatility/projectionOscillator.ts 95.91% <97.43%> (+60.91%) :arrow_up:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

abhishekmittal15 commented 1 year ago

Hi @cinar , I was wondering if you got a chance to have a look at the previous comment and the code?

Regards, Abhishek Mittal

cinar commented 1 year ago

My sincere apologies, I totally missed your earlier message. Let me quickly take a look.

abhishekmittal15 commented 1 year ago

So @cinar , does the changes in the logic part make sense. Does the error in the previous version of the code and my fix make logical sense, or do you have any questions about it.

Yes I will clean up the file, I just wanted to first confirm if the logic is right 😅 .

Also is there some way to check if this implementation is actually giving the right values?

cinar commented 1 year ago

I think your fix definitely makes sense. What we are lacking is a test case here. If we can find a source of truth for this calculation that we can compare the results against, it will give us more confidence that this is the correct algorithm to go with. Not sure if there is already an example that we can use, or an other tool to compute and compare the results against?