Skip to content
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

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

pppp606
Copy link

@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
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
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.

1 participant