Skip to content

feat: add C ndarray and refactor for stats/base/dmeanpn #4726

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 19 commits into
base: develop
Choose a base branch
from
Open

feat: add C ndarray and refactor for stats/base/dmeanpn #4726

wants to merge 19 commits into from

Conversation

DhruvArvindSingh
Copy link
Contributor

Resolves None.

Description

What is the purpose of this pull request?

This pull request:

  • adds C ndarray implementation for stats/base/dmeanpn
  • Updates README.md

Related Issues

Does this pull request have any related issues?

This pull request:

  • resolves None.

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

@DhruvArvindSingh
feat: add C ndarray implementation for stats/base/dmeanpn
2887967 
---
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: passed
  - 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: passed
  - task: lint_c_examples
    status: na
  - task: lint_c_benchmarks
    status: passed
  - 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
---

---
type: pre_push_report
description: Results of running various checks prior to pushing changes.
report:
  - task: run_javascript_examples
    status: na
  - task: run_c_examples
    status: na
  - task: run_cpp_examples
    status: na
  - task: run_javascript_readme_examples
    status: na
  - task: run_c_benchmarks
    status: na
  - task: run_cpp_benchmarks
    status: na
  - task: run_fortran_benchmarks
    status: na
  - task: run_javascript_benchmarks
    status: na
  - task: run_julia_benchmarks
    status: na
  - task: run_python_benchmarks
    status: na
  - task: run_r_benchmarks
    status: na
  - task: run_javascript_tests
    status: na
---
@stdlib-bot stdlib-bot added Statistics Issue or pull request related to statistical functionality. Needs Review A pull request which needs code review. labels Jan 12, 2025
@DhruvArvindSingh
Copy link
Contributor Author

/stdlib update-copyright-years

@stdlib-bot stdlib-bot added bot: In Progress Pull request is currently awaiting automation. and removed bot: In Progress Pull request is currently awaiting automation. labels Jan 12, 2025
@stdlib-bot
Copy link
Contributor

stdlib-bot commented Jan 12, 2025 •
edited
Loading

Coverage Report

Package Statements Branches Functions Lines
stats/base/dmeanpn $\color{green}383/383$
$\color{green}+100.00\%$ 		$\color{green}18/18$
$\color{green}+100.00\%$ 		$\color{green}4/4$
$\color{green}+100.00\%$ 		$\color{green}383/383$
$\color{green}+100.00\%$

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

@DhruvArvindSingh DhruvArvindSingh changed the title feat: add C ndarray implementation for stats/base/dmeanpn feat: add C ndarray and refcator for stats/base/dmeanpn Jan 12, 2025
@DhruvArvindSingh DhruvArvindSingh changed the title feat: add C ndarray and refcator for stats/base/dmeanpn feat: add C ndarray and refactor for stats/base/dmeanpn Jan 12, 2025
@Neerajpathak07
Copy link
Contributor

Neerajpathak07 commented Jan 13, 2025 •
edited
Loading

@DhruvArvindSingh
Hey, left a bunch of comments do look into it also you might need to add some changes in all the test files replacing N with 4 and removing N = floor/2
The examples.c will also require changes setting a const array and replacing stride with strideX

@Neerajpathak07
Copy link
Contributor

Neerajpathak07 commented Jan 13, 2025 •
edited
Loading

I would suggest you to make the suggested changes in your other PR's as well.
It makes it difficult for the maintainers while reviewing the code.
Had similar issues with my PR's as well which is why thought about bringing this up.

@DhruvArvindSingh
Copy link
Contributor Author

Thanks for pointing out ... I have made the necessary changes

DhruvArvindSingh and others added 3 commits January 13, 2025 15:58
@DhruvArvindSingh
Merge branch 'stdlib-js:develop' into feat/ndarray_dmeanpn
ffa1357
@DhruvArvindSingh
feat: add C ndarray implementation for stats/base/dmeanpn
ee78bc0 
---
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: passed
  - 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: passed
  - task: lint_c_examples
    status: na
  - task: lint_c_benchmarks
    status: passed
  - 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
---

---
type: pre_push_report
description: Results of running various checks prior to pushing changes.
report:
  - task: run_javascript_examples
    status: na
  - task: run_c_examples
    status: na
  - task: run_cpp_examples
    status: na
  - task: run_javascript_readme_examples
    status: na
  - task: run_c_benchmarks
    status: na
  - task: run_cpp_benchmarks
    status: na
  - task: run_fortran_benchmarks
    status: na
  - task: run_javascript_benchmarks
    status: na
  - task: run_julia_benchmarks
    status: na
  - task: run_python_benchmarks
    status: na
  - task: run_r_benchmarks
    status: na
  - task: run_javascript_tests
    status: na
---

---
type: pre_push_report
description: Results of running various checks prior to pushing changes.
report:
  - task: run_javascript_examples
    status: na
  - task: run_c_examples
    status: passed
  - task: run_cpp_examples
    status: na
  - task: run_javascript_readme_examples
    status: na
  - task: run_c_benchmarks
    status: na
  - task: run_cpp_benchmarks
    status: na
  - task: run_fortran_benchmarks
    status: na
  - task: run_javascript_benchmarks
    status: na
  - task: run_julia_benchmarks
    status: na
  - task: run_python_benchmarks
    status: na
  - task: run_r_benchmarks
    status: na
  - task: run_javascript_tests
    status: passed
---

---
type: pre_push_report
description: Results of running various checks prior to pushing changes.
report:
  - task: run_javascript_examples
    status: na
  - task: run_c_examples
    status: passed
  - task: run_cpp_examples
    status: na
  - task: run_javascript_readme_examples
    status: na
  - task: run_c_benchmarks
    status: na
  - task: run_cpp_benchmarks
    status: na
  - task: run_fortran_benchmarks
    status: na
  - task: run_javascript_benchmarks
    status: na
  - task: run_julia_benchmarks
    status: na
  - task: run_python_benchmarks
    status: na
  - task: run_r_benchmarks
    status: na
  - task: run_javascript_tests
    status: passed
---
@DhruvArvindSingh
fix: fixed minor error
6d3d7df 
---
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: passed
  - 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: passed
  - task: lint_c_examples
    status: passed
  - 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
---

---
type: pre_push_report
description: Results of running various checks prior to pushing changes.
report:
  - task: run_javascript_examples
    status: na
  - task: run_c_examples
    status: passed
  - task: run_cpp_examples
    status: na
  - task: run_javascript_readme_examples
    status: na
  - task: run_c_benchmarks
    status: na
  - task: run_cpp_benchmarks
    status: na
  - task: run_fortran_benchmarks
    status: na
  - task: run_javascript_benchmarks
    status: na
  - task: run_julia_benchmarks
    status: na
  - task: run_python_benchmarks
    status: na
  - task: run_r_benchmarks
    status: na
  - task: run_javascript_tests
    status: passed
---
@Neerajpathak07
Copy link
Contributor

You will also need to add changes to repl.txt and index.d.ts for instance you can refer this PR:- #4251 and how the refactoring is done over there.

@DhruvArvindSingh
fix: fixed test files
3db0802 
---
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
---

---
type: pre_push_report
description: Results of running various checks prior to pushing changes.
report:
  - task: run_javascript_examples
    status: na
  - task: run_c_examples
    status: na
  - task: run_cpp_examples
    status: na
  - task: run_javascript_readme_examples
    status: na
  - task: run_c_benchmarks
    status: na
  - task: run_cpp_benchmarks
    status: na
  - task: run_fortran_benchmarks
    status: na
  - task: run_javascript_benchmarks
    status: na
  - task: run_julia_benchmarks
    status: na
  - task: run_python_benchmarks
    status: na
  - task: run_r_benchmarks
    status: na
  - task: run_javascript_tests
    status: passed
---
@DhruvArvindSingh
Copy link
Contributor Author

Thanks @Neerajpathak07 for pointing out my mistakes.

@Neerajpathak07
Copy link
Contributor

@DhruvArvindSingh
My pleasure! and thanks for bearing with me XD.

@DhruvArvindSingh
Copy link
Contributor Author

@Neerajpathak07 since you have approved my PR you have to comment lgtm on this PR inorder to approve it

@Neerajpathak07
Copy link
Contributor

@DhruvArvindSingh
Hey, I didn't have time to have a look at the the entire PR which is why I didn't mention it in the first place. Will take a look once I get free from college!!

DhruvArvindSingh and others added 5 commits January 16, 2025 00:15
@DhruvArvindSingh @Neerajpathak07
fix: fixing mistakes
3899530 
Co-authored-by: Neeraj Pathak <[email protected]>
Signed-off-by: Dhruv/ <[email protected]>
@DhruvArvindSingh
fix: Update README.md
33c4d14 
Signed-off-by: Dhruv/ <[email protected]>
@DhruvArvindSingh
fix: Update README.md
0c32ffc 
Signed-off-by: Dhruv/ <[email protected]>
@DhruvArvindSingh
fix: Update example.c
0a7b50f 
Signed-off-by: Dhruv/ <[email protected]>
@DhruvArvindSingh
fix: update example.c
1e95b9b 
Signed-off-by: Dhruv/ <[email protected]>
@Neerajpathak07
Copy link
Contributor

@DhruvArvindSingh
Just a few comments other than that everything else looks good!!

@Neerajpathak07
Copy link
Contributor

Also the changes done in the index.js should also be done in the markdown file.

@Neerajpathak07
Copy link
Contributor

Hey @DhruvArvindSingh Hope you're doing great!
Could you a look at this PR will be needing this for refactoring dmean.

@DhruvArvindSingh
Copy link
Contributor Author

Hey @DhruvArvindSingh Hope you're doing great! Could you a look at this PR will be needing this for refactoring dmean.

Sorry for the late reply, i was actually busy with my exams but now i am free..... I will resolve this by tomorrow morning

@Neerajpathak07
Copy link
Contributor

Great, you'll also need to update it to handle NaN elements you can refer to some of the merged Pr's or for instance take a look at this #4496

@DhruvArvindSingh DhruvArvindSingh mentioned this pull request Apr 7, 2025
Open
7 tasks
ShabiShett07
ShabiShett07 reviewed May 5, 2025
View reviewed changes
lib/node_modules/@stdlib/stats/base/dmeanpn/README.md
Comment on lines +251 to +253
const int N = 4;

// Specify the stride length:
Copy link
Contributor

@ShabiShett07 ShabiShett07 May 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
const int N = 4;
// Specify the stride length:
const int N = 4;
// Specify the stride length:

ShabiShett07
ShabiShett07 reviewed May 5, 2025
View reviewed changes
lib/node_modules/@stdlib/stats/base/dmeanpn/README.md
Comment on lines +254 to +256
const int strideX = 2;

// Compute the arithmetic mean:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const int strideX = 2;
// Compute the arithmetic mean:
const int strideX = 2;
// Compute the arithmetic mean:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ShabiShett07 ShabiShett07 ShabiShett07 left review comments

@Neerajpathak07 Neerajpathak07 Neerajpathak07 left review comments

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

Assignees
No one assigned
Labels
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.
4 participants
@DhruvArvindSingh @stdlib-bot @Neerajpathak07 @ShabiShett07