Skip to content

Extend multiply and divide functions to accept scalar values for both arguments#14

Merged
grimmerk merged 4 commits intogrimmerk:masterfrom
pppp606:fix/multiply_and_divide_argument_types
Mar 7, 2026
Merged

Extend multiply and divide functions to accept scalar values for both arguments#14
grimmerk merged 4 commits intogrimmerk:masterfrom
pppp606:fix/multiply_and_divide_argument_types

Conversation

@pppp606
Copy link
Contributor

@pppp606 pppp606 commented Sep 4, 2023

Summary

This PR aims to extend the functionality of the multiply and divide functions in the numjs library by allowing them to accept scalar numbers for both a and b arguments.

Changes

Updated the type signatures for multiply and divide functions to accept number as a possible type for both a and b parameters.

Why this change is needed

The current implementation of multiply and divide functions allows for a combination of NdArray and scalar numbers, but they don't allow two scalar numbers. By allowing scalar numbers for both arguments, we increase the versatility and user-friendliness of these functions without introducing any breaking changes.

Examples

Before

multiply([1, 2], 3);  // Works
multiply(3, [1, 2]);  // Works
multiply(3, 4);       // Doesn't work

After

multiply([1, 2], 3);  // Works
multiply(3, [1, 2]);  // Works
multiply(3, 4);       // Works

@pppp606
Copy link
Contributor Author

pppp606 commented Sep 4, 2023

@grimmer0125
Hello grimmer0125,

First of all, thank you for your hard work on this fork of numjs, it's a valuable resource.

I've noticed that the multiply and divide functions currently don't allow scalar values for both a and b arguments, and thought extending this functionality could be useful.

I've attached a pull request that makes these changes. Would you mind taking a look? I hope it doesn't create any inconvenience for you.

Thank you for considering my contribution. I look forward to your feedback.

Best regards 😇

@pppp606 pppp606 marked this pull request as ready for review September 4, 2023 06:14
@grimmerk
Copy link
Owner

grimmerk commented Mar 7, 2026

Hi @pppp606,

I sincerely apologize for the extremely late response — this PR has been sitting here for over two years, which is unacceptable. I haven't been actively maintaining this project, but that's no excuse for leaving your contribution unreviewed for so long.

I've now reviewed your changes, and they look great. The fix is correct — multiply and divide were the only two functions missing | number on the a parameter, while add, subtract, power, mod, etc. already accepted it. Your PR closes that gap cleanly, and the tests confirm it works as expected.

Merging now via squash merge. Thank you for taking the time to contribute, and again, sorry for the delay.

@grimmerk grimmerk merged commit 6743062 into grimmerk:master Mar 7, 2026
@grimmerk grimmerk mentioned this pull request Mar 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants