feat: add stats/incr/nanvariance function#5755
Conversation
Signed-off-by: Gautam sharma <gautamkrishnasharma1@gmail.com>
Signed-off-by: Gautam sharma <gautamkrishnasharma1@gmail.com>
stdlib-bot
added
Statistics
Coverage Report
The above coverage report was generated for the changes in this PR. |
Signed-off-by: Gautam sharma <gautamkrishnasharma1@gmail.com>
Signed-off-by: Gautam sharma <gautamkrishnasharma1@gmail.com>
Krishna-Sharma-g
changed the title
stats/incr/nanvariance #5626 stats/incr/nanvariance function
|
@kgryte please review once it has some linting issues that i will fix soon but please review the main implementation of it and suggest the changes i can do in it |
|
hello @anandkaranubc please review |
| @@ -0,0 +1,57 @@ | |||
| /* | |||
There was a problem hiding this comment.
This entire file is incorrect. Please follow our repl text style guide and avoid using LLMs to generate files.
There was a problem hiding this comment.
it was showing warning in the linting years so i added that licence code in that but now i have resolved it please check is that exactly you wanted
|
|
||
| // The compiler throws an error if the function is provided invalid arguments... | ||
| { | ||
| incrnanvariance( '5' as any ); // $ExpectError |
There was a problem hiding this comment.
Remove all of these as any casts. The entire point of these lines is to test that the compiler errors when provided various non-any input values.
There was a problem hiding this comment.
i did it because it was showing some ecmacript error in it but i have changed it now and removed the as any from there please review
| * v = accumulator(); | ||
| * // returns 33.7796 | ||
| */ | ||
| function incrnanvariance( mean ) { |
There was a problem hiding this comment.
This implementation is not what is desired. As mentioned in the OP,
In particular notice how nansum is a thin wrapper around sum. In most cases, this is what we are looking for.
You're essentially duplicating the incr/variance implementation. That is not what we want. Create a thin wrapper similar to nansum.
There was a problem hiding this comment.
i got confused between both i have changed please check
kgryte
left a comment
kgryte
left a comment
There was a problem hiding this comment.
Left an initial round of comments. This PR needs a fair amount of refactoring and clean-up before it can be merged.
kgryte
added
Needs Changes
|
okay @kgryte working on it |
|
@kgryte i did the changes in this you wanted me to change and i have fixed the linting issues also please review and i am creating the readme for nanvariance also |
|
@kgryte please review !! |
|
@kgryte please review ! |
|
@Krishna-Sharma-g This package is missing a README file. |
stdlib-bot
added
the
Potential Duplicate
stdlib-bot
removed
the
Potential Duplicate
kgryte
added
autoclose: Git History
|
Thank you for working on this pull request. However, we cannot accept your contribution due to Git history issues. Some common issues include:
We recommend opening a new pull request with only the intended changes. Thank you for your interest in stdlib, and we look forward to your future contributions. |
3 participants
Resolves #{{TODO: add issue number}}.
#5626
This pull request:
Related Issues
This pull request:
stats/incr/nanvariance#5626Questions
No.
Other
all three test commands passes
No.
Checklist
@stdlib-js/reviewers