Skip to content

Conversation

manvith2003
Copy link
Contributor

@manvith2003 manvith2003 commented Dec 17, 2024

Resolves #3783

Description

What is the purpose of this pull request?

This pull request:

  • adds C implementation for stats/base/dists/normal/stdev along with relevant tests, docs, examples and benchmarks.

Related Issues

Does this pull request have any related issues?

This pull request:

Questions

Any questions for reviewers of this pull request?

No.

Other

Any other information relevant to this pull request? This may include screenshots, references, and/or implementation notes.

No.

Checklist

Please ensure the following tasks are completed before submitting this pull request.


@stdlib-js/reviewers

@stdlib-bot stdlib-bot added Statistics Issue or pull request related to statistical functionality. Needs Review A pull request which needs code review. labels Dec 17, 2024
@stdlib-bot
Copy link
Contributor

stdlib-bot commented Dec 17, 2024

Coverage Report

Package Statements Branches Functions Lines
stats/base/dists/normal/stdev $\color{green}175/175$
$\color{green}+100.00\%$
$\color{green}9/9$
$\color{green}+100.00\%$
$\color{green}2/2$
$\color{green}+100.00\%$
$\color{green}175/175$
$\color{green}+100.00\%$

The above coverage report was generated for the changes in this PR.

@Planeshifter Planeshifter changed the title feat: add C implementation for stats/base/dists/normal/stdev feat: add C implementation for stats/base/dists/normal/stdev Dec 18, 2024
Comment on lines 29 to 32
double mu;
double sigma;
double y;
int i;
Copy link
Member

@Planeshifter Planeshifter Dec 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We typically order the variable declarations by length:

Suggested change
double mu;
double sigma;
double y;
int i;
double sigma;
double mu;
double y;
int i;

Copy link
Member

@Planeshifter Planeshifter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this looks good and should be close to landing, but there were a few issues that still need to be addressed.

@Planeshifter Planeshifter added Needs Changes Pull request which needs changes before being merged. and removed Needs Review A pull request which needs code review. labels Dec 18, 2024
Copy link
Member

@Planeshifter Planeshifter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the edits! Will land PR shortly.

@Planeshifter Planeshifter added the Ready To Merge A pull request which is ready to be merged. label Dec 18, 2024
@stdlib-bot stdlib-bot removed the Needs Changes Pull request which needs changes before being merged. label Dec 18, 2024
@stdlib-bot
Copy link
Contributor

PR Commit Message

feat: add C implementation for `stats/base/dists/normal/stdev`

PR-URL: https://github.com/stdlib-js/stdlib/pull/4003
Closes: https://github.com/stdlib-js/stdlib/issues/3783

Co-authored-by: Philipp Burckhardt <[email protected]>
Reviewed-by: Philipp Burckhardt <[email protected]>
Signed-off-by: Philipp Burckhardt <[email protected]>

Please review the above commit message and make any necessary adjustments.

@Planeshifter Planeshifter merged commit 7550aab into stdlib-js:develop Dec 19, 2024
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Ready To Merge A pull request which is ready to be merged. Statistics Issue or pull request related to statistical functionality.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[RFC]: Add C implementation for @stdlib/stats/base/dists/normal/stdev

3 participants