Skip to content

feat: add stats/incr/nanmrss #6197

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 18 commits into
base: develop
Choose a base branch
from
Open

feat: add stats/incr/nanmrss #6197

wants to merge 18 commits into from

Conversation

jalajk3004
Copy link
Contributor

Resolves #5607

Description

What is the purpose of this pull request?

This pull request:

  • {{TODO: add description describing what this pull request does}}

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. Good First PR A pull request resolving a Good First Issue. Needs Review A pull request which needs code review. labels Mar 19, 2025
@stdlib-bot
Copy link
Contributor

Hello! 👋

We've noticed that you've been opening a number of PRs addressing good first issues. Thank you for your interest and enthusiasm!

Now that you've made a few contributions, we suggest no longer working on good first issues. Instead, we encourage you to prioritize cleaning up any PRs which have yet to be merged and then proceed to work on more involved tasks.

Not only does this ensure that other new contributors can work on things and get ramped up on all things stdlib, it also ensures that you can spend your time on more challenging problems. 🚀

For ideas for future PRs, feel free to search the codebase for TODOs and FIXMEs and be sure to check out other open issues on the issue tracker. Cheers!

@stdlib-bot
Copy link
Contributor

stdlib-bot commented Mar 19, 2025 •
edited
Loading

Coverage Report

Package Statements Branches Functions Lines
stats/incr/nanmrss $\color{green}155/155$
$\color{green}+100.00\%$ 		$\color{red}10/12$
$\color{green}+83.33\%$ 		$\color{green}2/2$
$\color{green}+100.00\%$ 		$\color{green}155/155$
$\color{green}+100.00\%$

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

@jalajk3004 jalajk3004 changed the title feat added stats/incr/nanmrss feat: add stats/incr/nanmrss Mar 19, 2025
hrshya
hrshya reviewed Mar 19, 2025
View reviewed changes
Copy link
Contributor

@hrshya hrshya left a comment

Choose a reason for hiding this comment

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

The img directory is also missing in docs.

lib/node_modules/@stdlib/stats/incr/nanmrss/README.md Outdated

#### incrnanmrss( window )

Returns an accumulator `function` which incrementally computes a moving [residual sum of squares][residual-sum-of-squares]. The `window` parameter defines the number of values over which to compute the moving [residual sum of squares][residual-sum-of-squares].
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs to add , ignoring NaN values. at the end.

lib/node_modules/@stdlib/stats/incr/nanmrss/docs/types/index.d.ts Outdated
/**
* If provided arguments, returns an updated residual sum of squares; otherwise, returns the current residual sum of squares, ignoring `NaN` values.
*
* ## Notes
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove from L26-29

lib/node_modules/@stdlib/stats/incr/nanmrss/lib/main.js Outdated
* @returns {(number|null)} residual sum of squares or null
*/
function accumulator( x, y ) {
if ( arguments.length === 0 ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of using two if statements. This can be done in a single if statement. (L79-84)

@jalajk3004
Copy link
Contributor Author

@hrshya you can review this PR again, i have resolved the errors

@hrshya
Copy link
Contributor

hrshya commented Mar 20, 2025

@hrshya you can review this PR again, i have resolved the errors

On it!

hrshya
hrshya reviewed Mar 20, 2025
View reviewed changes
Copy link
Contributor

@hrshya hrshya left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@jalajk3004 Can you also change the description to:

adds stats/incr/nanmrss .

@jalajk3004 jalajk3004 mentioned this pull request Mar 29, 2025
Closed
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@hrshya hrshya hrshya left review comments

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

Good First PR A pull request resolving a Good First Issue. Needs Review A pull request which needs code review. Statistics Issue or pull request related to statistical functionality.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[RFC]: add stats/incr/nanmrss

3 participants

@jalajk3004 @stdlib-bot @hrshya