Skip to content

feat: refactor and add native addon for complex/float64/conj #7301

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
Merged
merged 11 commits into from
Jun 21, 2025
Merged

feat: refactor and add native addon for complex/float64/conj #7301

merged 11 commits into from
Jun 21, 2025

Conversation

@ShabiShett07
Copy link
Contributor

@ShabiShett07 ShabiShett07 commented Jun 10, 2025

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: passed
  • task: lint_javascript_benchmarks status: passed
  • 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 ---

none

Description

What is the purpose of this pull request?

This pull request:

  • Adds native addon and refactor the implementation for complex/float64/conj

Related Issues

Does this pull request have any related issues?

This pull request:

  • 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

@ShabiShett07
feat: refactor and native addons for complex/float64/conj
8965325 
---
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: passed
  - task: lint_javascript_benchmarks
    status: passed
  - 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
---
@stdlib-bot stdlib-bot added the Needs Review A pull request which needs code review. label Jun 10, 2025
@ShabiShett07
chore: clean-up
9c9007e 
---
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
---
@stdlib-bot
Copy link
Contributor

stdlib-bot commented Jun 10, 2025 •
edited
Loading

Coverage Report

Package Statements Branches Functions Lines
complex/float64/conj $\color{green}143/143$
$\color{green}+100.00\%$ 		$\color{green}5/5$
$\color{green}+100.00\%$ 		$\color{green}2/2$
$\color{green}+100.00\%$ 		$\color{green}143/143$
$\color{green}+100.00\%$

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

@ShabiShett07
Copy link
Contributor Author

ShabiShett07 commented Jun 10, 2025

/stdlib update-copyright-years

@stdlib-bot stdlib-bot added the bot: In Progress Pull request is currently awaiting automation. label Jun 10, 2025
@stdlib-bot stdlib-bot removed the bot: In Progress Pull request is currently awaiting automation. label Jun 10, 2025
@ShabiShett07
Copy link
Contributor Author

ShabiShett07 commented Jun 10, 2025

/stdlib merge

@ShabiShett07 ShabiShett07 requested a review from kgryte June 10, 2025 06:06
@stdlib-bot stdlib-bot added the bot: In Progress Pull request is currently awaiting automation. label Jun 10, 2025
@stdlib-bot stdlib-bot removed the bot: In Progress Pull request is currently awaiting automation. label Jun 10, 2025
@ShabiShett07 ShabiShett07 added GSoC Google Summer of Code. gsoc: 2025 Google Summer of Code (2025). labels Jun 15, 2025
@ShabiShett07 ShabiShett07 mentioned this pull request Jun 16, 2025
Closed
kgryte
kgryte reviewed Jun 20, 2025
View reviewed changes
kgryte
kgryte reviewed Jun 20, 2025
View reviewed changes
lib/node_modules/@stdlib/complex/float64/conj/lib/main.js
*/
function conj( z ) {
return new z.constructor( z.re, -z.im );
return new Complex128( real( z ), -imag( z ) );
Copy link
Member

@kgryte kgryte Jun 20, 2025

Choose a reason for hiding this comment

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

Yes, I think this makes sense now. There is some history here in why z.constructor was used. Namely, when this package was complex/conj, we wanted to preserve input instance type. However, now that this package is in complex/float64, I think we are fine always returning a Complex128 instance.

kgryte
kgryte reviewed Jun 20, 2025
View reviewed changes
kgryte
kgryte reviewed Jun 20, 2025
View reviewed changes
kgryte
kgryte previously approved these changes Jun 20, 2025
View reviewed changes
@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 Jun 20, 2025
@kgryte
Copy link
Member

kgryte commented Jun 20, 2025

@ShabiShett07 In order for the native add-on to compile, you need to add the *.gyp files. This was why the addon.c bug was not caught.

@kgryte
Copy link
Member

kgryte commented Jun 20, 2025

...once those files are added and the tests pass, this PR can be merged.

@ShabiShett07
chore: add gyp files
dd0c8d2 
---
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: na
  - task: lint_typescript_tests
    status: na
  - task: lint_license_headers
    status: passed
---
@ShabiShett07
Copy link
Contributor Author

ShabiShett07 commented Jun 21, 2025

/stdlib update-copyright-years

@stdlib-bot stdlib-bot added the bot: In Progress Pull request is currently awaiting automation. label Jun 21, 2025
@stdlib-bot stdlib-bot removed the bot: In Progress Pull request is currently awaiting automation. label Jun 21, 2025
@ShabiShett07
chore: update package.json
eb0d1ae 
---
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: passed
  - 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: na
  - task: lint_typescript_tests
    status: na
  - task: lint_license_headers
    status: passed
---
@ShabiShett07 ShabiShett07 requested a review from kgryte June 21, 2025 17:19
@stdlib-bot stdlib-bot added the Needs Review A pull request which needs code review. label Jun 21, 2025
@ShabiShett07
Copy link
Contributor Author

ShabiShett07 commented Jun 21, 2025

Added gyp files to this, just needed confirmation for the structure and got it from @kgryte

kgryte
kgryte reviewed Jun 21, 2025
View reviewed changes
@kgryte
chore: remove field
b83b1aa 
Signed-off-by: Athan <[email protected]>
@kgryte kgryte changed the title feat: refactor and native addons for complex/float64/conj feat: refactor and add native addon for complex/float64/conj Jun 21, 2025
@kgryte kgryte added C Issue involves or relates to C. and removed Needs Review A pull request which needs code review. Needs Changes Pull request which needs changes before being merged. labels Jun 21, 2025
@kgryte kgryte merged commit 40373c5 into stdlib-js:develop Jun 21, 2025
33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kgryte kgryte kgryte approved these changes

Assignees

No one assigned

Labels

C Issue involves or relates to C. GSoC Google Summer of Code. gsoc: 2025 Google Summer of Code (2025).

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ShabiShett07 @stdlib-bot @kgryte