-
-
Notifications
You must be signed in to change notification settings - Fork 804
feat: add C implementation for stats/base/dists/erlang/kurtosis
#4289
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
feat: add C implementation for stats/base/dists/erlang/kurtosis
#4289
Conversation
Coverage Report
The above coverage report was generated for the changes in this PR. |
/stdlib update-copyright-years |
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.
Thanks for your PR, Neeraj!
For the Erlang packages, it would be good to type the k
parameter as a int32_t
given the parameter has to be an integer.. This will require some slight refactoring, but hopefully won't be too burdensome.
If people need the larger range afforded by double
, they can use the gamma distribution instead.
@Planeshifter Sure let me get back to you with the refactored code. |
lib/node_modules/@stdlib/stats/base/dists/erlang/kurtosis/test/test.native.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/stats/base/dists/erlang/kurtosis/src/main.c
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/stats/base/dists/erlang/kurtosis/src/main.c
Outdated
Show resolved
Hide resolved
...s/@stdlib/stats/base/dists/erlang/kurtosis/include/stdlib/stats/base/dists/erlang/kurtosis.h
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/stats/base/dists/erlang/kurtosis/examples/c/example.c
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/stats/base/dists/erlang/kurtosis/README.md
Outdated
Show resolved
Hide resolved
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.
Looks very good overall; left a few comments that should be addressed. However, I am not sure about the failing test cases right now.
Any ideas on what is going on here, @kgryte?
/stdlib help |
@Planeshifter, available slash commands include:
|
lib/node_modules/@stdlib/stats/base/dists/erlang/kurtosis/test/test.native.js
Show resolved
Hide resolved
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.
Thanks for your PR and getting it into great shape, @Neerajpathak07!
As time permits, it would be great to similarly refactor the other Erlang PRs to have an integer-typed k
parameter.
PR Commit Message
Please review the above commit message and make any necessary adjustments. |
@Planeshifter Will wrap all of them by tonight. little heads up you'll be pretty attested with some load on reviewing them but dw I'll make sure to bring them in great shape. |
Resolves #3570 .
Description
This pull request:
stats/base/dists/erlang/kurtosis
Related Issues
This pull request:
@stdlib/stats/base/dists/erlang/kurtosis
#3570Questions
No.
Other
No.
Checklist
@stdlib-js/reviewers