-
-
Notifications
You must be signed in to change notification settings - Fork 804
feat: add new package to the stats/incr/* namespace: @stdlib/stats/incr/nanmpcorrdist #6614
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
base: develop
Are you sure you want to change the base?
feat: add new package to the stats/incr/* namespace: @stdlib/stats/incr/nanmpcorrdist #6614
Conversation
…cr/nanmpcorrdist --- type: pre_commit_static_analysis_report description: Results of running static analysis checks when committing changes. report: - task: lint_filenames status: passed - task: lint_editorconfig status: passed - task: lint_markdown status: passed - task: lint_package_json status: passed - task: lint_repl_help status: passed - task: lint_javascript_src status: passed - task: lint_javascript_cli status: na - task: lint_javascript_examples status: passed - task: lint_javascript_tests status: passed - task: lint_javascript_benchmarks status: passed - task: lint_python status: na - task: lint_r status: na - task: lint_c_src status: na - task: lint_c_examples status: na - task: lint_c_benchmarks status: na - task: lint_c_tests_fixtures status: na - task: lint_shell status: na - task: lint_typescript_declarations status: passed - task: lint_typescript_tests status: passed - task: lint_license_headers status: passed ---
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.
👋 Hi there! 👋
And thank you for opening your first pull request! We will review it shortly. 🏃 💨
…cr/nanmpcorrdist --- type: pre_commit_static_analysis_report description: Results of running static analysis checks when committing changes. report: - task: lint_filenames status: passed - task: lint_editorconfig status: passed - task: lint_markdown status: na - task: lint_package_json status: na - task: lint_repl_help status: na - task: lint_javascript_src status: na - task: lint_javascript_cli status: na - task: lint_javascript_examples status: na - task: lint_javascript_tests status: passed - task: lint_javascript_benchmarks status: na - task: lint_python status: na - task: lint_r status: na - task: lint_c_src status: na - task: lint_c_examples status: na - task: lint_c_benchmarks status: na - task: lint_c_tests_fixtures status: na - task: lint_shell status: na - task: lint_typescript_declarations status: na - task: lint_typescript_tests status: na - task: lint_license_headers status: passed ---
…cr/nanmpcorrdist --- type: pre_commit_static_analysis_report description: Results of running static analysis checks when committing changes. report: - task: lint_filenames status: passed - task: lint_editorconfig status: passed - task: lint_markdown status: na - task: lint_package_json status: na - task: lint_repl_help status: na - task: lint_javascript_src status: na - task: lint_javascript_cli status: na - task: lint_javascript_examples status: na - task: lint_javascript_tests status: passed - task: lint_javascript_benchmarks status: na - task: lint_python status: na - task: lint_r status: na - task: lint_c_src status: na - task: lint_c_examples status: na - task: lint_c_benchmarks status: na - task: lint_c_tests_fixtures status: na - task: lint_shell status: na - task: lint_typescript_declarations status: na - task: lint_typescript_tests status: na - task: lint_license_headers status: passed ---
e95582f
to
bd96b60
Compare
Coverage Report
The above coverage report was generated for the changes in this PR. |
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.
Hey @MohannedAhmed67, thanks for your work on this! I’ve added some comments on the PR.
lib/node_modules/@stdlib/stats/incr/nanmpcorrdist/docs/types/index.d.ts
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/stats/incr/nanmpcorrdist/docs/types/index.d.ts
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/stats/incr/nanmpcorrdist/docs/types/test.ts
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/stats/incr/nanmpcorrdist/docs/types/test.ts
Outdated
Show resolved
Hide resolved
…cr/nanmpcorrdist --- type: pre_commit_static_analysis_report description: Results of running static analysis checks when committing changes. report: - task: lint_filenames status: passed - task: lint_editorconfig status: passed - task: lint_markdown status: passed - task: lint_package_json status: na - task: lint_repl_help status: na - task: lint_javascript_src status: na - task: lint_javascript_cli status: na - task: lint_javascript_examples status: na - task: lint_javascript_tests status: na - task: lint_javascript_benchmarks status: passed - task: lint_python status: na - task: lint_r status: na - task: lint_c_src status: na - task: lint_c_examples status: na - task: lint_c_benchmarks status: na - task: lint_c_tests_fixtures status: na - task: lint_shell status: na - task: lint_typescript_declarations status: na - task: lint_typescript_tests status: na - task: lint_license_headers status: passed ---
…cr/nanmpcorrdist --- type: pre_commit_static_analysis_report description: Results of running static analysis checks when committing changes. report: - task: lint_filenames status: passed - task: lint_editorconfig status: passed - task: lint_markdown status: na - task: lint_package_json status: na - task: lint_repl_help status: na - task: lint_javascript_src status: na - task: lint_javascript_cli status: na - task: lint_javascript_examples status: na - task: lint_javascript_tests status: na - task: lint_javascript_benchmarks status: na - task: lint_python status: na - task: lint_r status: na - task: lint_c_src status: na - task: lint_c_examples status: na - task: lint_c_benchmarks status: na - task: lint_c_tests_fixtures status: na - task: lint_shell status: na - task: lint_typescript_declarations status: passed - task: lint_typescript_tests status: passed - task: lint_license_headers status: passed ---
@hrshya Thank you so much for your valuable comments on my PR !!! |
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.
Hey @MohannedAhmed67, Good work. Just need some minor updates.
…cr/nanmpcorrdist --- type: pre_commit_static_analysis_report description: Results of running static analysis checks when committing changes. report: - task: lint_filenames status: passed - task: lint_editorconfig status: passed - task: lint_markdown status: na - task: lint_package_json status: na - task: lint_repl_help status: na - task: lint_javascript_src status: na - task: lint_javascript_cli status: na - task: lint_javascript_examples status: na - task: lint_javascript_tests status: passed - task: lint_javascript_benchmarks status: na - task: lint_python status: na - task: lint_r status: na - task: lint_c_src status: na - task: lint_c_examples status: na - task: lint_c_benchmarks status: na - task: lint_c_tests_fixtures status: na - task: lint_shell status: na - task: lint_typescript_declarations status: na - task: lint_typescript_tests status: na - task: lint_license_headers status: passed ---
👋 Hi there! 👋
|
I have resolved all mentioned and gone through the file to fill all the similar minor issues. |
@hrshya Hello, have you checked the PR this time ? |
@kgryte Can I get a review of this PR ? |
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.
Just some minor changes. Rest, the PR looks good!
|
||
// For each simulated datum, update the moving sample correlation distance... | ||
for ( i = 0; i < 100; i++ ) { | ||
if ( randu() < 0.2 ) { |
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.
We don't need separate if-else statements for x and y each as the conditions are same so it can be done in a single if-else statement.
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.
The problem is that I want x to be NaN sometimes and y to be a value and vice versa, but if I included them in the the same if-else, x and y will be NaN at the same time, and I want to include all the cases of
(x, y)
(Number, Number)
(NaN, NaN)
(Number, NaN)
(NaN, Number)
// For each simulated datum, update the moving sample correlation distance... | ||
console.log( '\nx\ty\tCorrelation Distance\n' ); | ||
for ( i = 0; i < 100; i++ ) { | ||
if ( randu() < 0.2 ) { |
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.
Same comment as earlier.
<!-- <div class="equation" align="center" data-raw-text="d_{x,y} = 1 - r_{x,y} = 1 - \frac{\operatorname{cov_n(x,y)}}{\sigma_x \sigma_y}" data-equation="eq:pearson_distance"> | ||
<img src="https://cdn.jsdelivr.net/gh/stdlib-js/stdlib@49d8cabda84033d55d7b8069f19ee3dd8b8d1496/lib/node_modules/@stdlib/stats/incr/mpcorrdist/docs/img/equation_pearson_distance.svg" alt="Equation for the Pearson product-moment correlation distance."> | ||
<br> | ||
</div> --> |
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.
You can remove this, as this will be automatically populated.
|
||
#### incrnanmpcorrdist( window\[, mx, my] ) | ||
|
||
Returns an accumulator `function` which incrementally computes a moving [sample Pearson product-moment correlation distance][pearson-correlation] while ignoring `NaN` values. The `window` parameter defines the number of values over which to compute the moving [sample Pearson product-moment correlation distance][pearson-correlation]. |
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.
Returns an accumulator `function` which incrementally computes a moving [sample Pearson product-moment correlation distance][pearson-correlation] while ignoring `NaN` values. The `window` parameter defines the number of values over which to compute the moving [sample Pearson product-moment correlation distance][pearson-correlation]. | |
Returns an accumulator `function` which incrementally computes a moving [sample Pearson product-moment correlation distance][pearson-correlation], ignoring `NaN` values. The `window` parameter defines the number of values over which to compute the moving [sample Pearson product-moment correlation distance][pearson-correlation]. |
{ | ||
"name": "@stdlib/stats/incr/nanmpcorrdist", | ||
"version": "0.0.0", | ||
"description": "Compute a moving sample Pearson product-moment correlation distance incrementally while ignoring `NaN` values.", |
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.
"description": "Compute a moving sample Pearson product-moment correlation distance incrementally while ignoring `NaN` values.", | |
"description": "Compute a moving sample Pearson product-moment correlation distance incrementally, ignoring `NaN` values.", |
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.
Update this here and throughout the package to keep the consistency across the package.
…cr/nanmpcorrdist
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes. report:
Resolves #5602.
Description
This pull request:
Related Issues
This pull request:
Questions
No.
Other
No.
Checklist
@stdlib-js/reviewers