Skip to content

refactor: update stats/base/snanvariancepn native addon from C++ to C #4515

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
Closed
wants to merge 4 commits into from
Closed

refactor: update stats/base/snanvariancepn native addon from C++ to C #4515

wants to merge 4 commits into from

Conversation

DeshDeepakKant
Copy link
Contributor

@DeshDeepakKant DeshDeepakKant commented Jan 3, 2025 •
edited
Loading

Resolves None

Description

adds C addons to stats/base/snanvariancepn instead of c++ addons

This pull request:

Related Issues

This pull request:

Questions

No.

Other

No.

Checklist

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

@stdlib-js/reviewers

DeshDeepakKant and others added 2 commits January 3, 2025 20:56
@DeshDeepakKant
test
8e814da
refactor: update stats/base/snanvariancepn native addon from C++ to C
5b5b413 
---
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: missing_dependencies
  - 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: na
---
@stdlib-bot stdlib-bot added the Needs Review A pull request which needs code review. label Jan 3, 2025
@stdlib-bot
Copy link
Contributor

Hello! Thank you for your contribution to stdlib.

We noticed that the contributing guidelines acknowledgment is missing from your pull request. Here's what you need to do:

  1. Please read our contributing guidelines.

  2. Update your pull request description to include this checked box:

    - [x] Read, understood, and followed the [contributing guidelines](https://github.com/stdlib-js/stdlib/blob/develop/CONTRIBUTING.md)

This acknowledgment confirms that you've read the guidelines, which include:

  • The developer's certificate of origin
  • Your agreement to license your contributions under the project's terms

We can't review or accept contributions without this acknowledgment.

Thank you for your understanding and cooperation. We look forward to reviewing your contribution!

@DeshDeepakKant
Update addon.c
c14069c 
Signed-off-by: Desh Deepak Kant <[email protected]>
@kgryte kgryte added Needs Changes Pull request which needs changes before being merged. and removed Needs Review A pull request which needs code review. labels Jan 3, 2025
kgryte
kgryte reviewed Jan 3, 2025
View reviewed changes
@kgryte
chore: remove file
53259ae 
Signed-off-by: Athan <[email protected]>
@stdlib-bot stdlib-bot added the Statistics Issue or pull request related to statistical functionality. label Jan 3, 2025
@kgryte
Copy link
Member

kgryte commented Jan 3, 2025

@DeshDeepakKant Note that you need to actually check the box in the OP, as indicated in the message above.

kgryte
kgryte reviewed Jan 8, 2025
View reviewed changes
lib/node_modules/@stdlib/stats/base/snanvariancepn/manifest.json
],
"libpath": [],
"dependencies": [
"@stdlib/stats/base/snanvariancepn",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"@stdlib/stats/base/snanvariancepn",

This introduces a circular dependency.

kgryte
kgryte reviewed Jan 8, 2025
View reviewed changes
lib/node_modules/@stdlib/stats/base/snanvariancepn/src/addon.c
STDLIB_NAPI_ARGV_DOUBLE( env, mean, argv, 1 );
STDLIB_NAPI_ARGV_DOUBLE( env, correction, argv, 2 );
STDLIB_NAPI_ARGV_STRIDED_FLOAT64ARRAY( env, X, N, stride, argv, 3 );
STDLIB_NAPI_CREATE_DOUBLE( env, stdlib_strided_dvarmpn( N, mean, correction, X, stride ), v );
Copy link
Member

Choose a reason for hiding this comment

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

This is very clearly incorrect. The function is not the right function.

Copy link
Member

Choose a reason for hiding this comment

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

Please study the original addon.cpp file.

@0PrashantYadav0
Copy link
Member

@DeshDeepakKant Are you still working on this issue?

@DeshDeepakKant
Copy link
Contributor Author

@DeshDeepakKant Are you still working on this issue?
No, you can work on it 🙌

@0PrashantYadav0
Copy link
Member

@DeshDeepakKant Please close this PR.

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

Reviewers

@kgryte kgryte kgryte left review comments

Assignees

No one assigned

Labels

Needs Changes Pull request which needs changes before being merged. 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

@DeshDeepakKant @stdlib-bot @kgryte @0PrashantYadav0