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

refactor: add support for ratios evaluating as infinity in math/base/tools/evalrational-compile-c #1970

Merged
merged 4 commits into from
Mar 20, 2024
Merged

Conversation

gunjjoshi
Copy link
Member

@gunjjoshi gunjjoshi commented Mar 20, 2024

Description

What is the purpose of this pull request?

This pull request:

Related Issues

Does this pull request have any related issues?

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

@stdlib-bot stdlib-bot added the Math Issue or pull request specific to math functionality. label Mar 20, 2024
@gunjjoshi
Copy link
Member Author

gunjjoshi commented Mar 20, 2024

I have used PINF and NINF to check whether x is infinite, rather than using is-infinite. The reason being, we need to set the string to 1.0 / 0.0 or -1.0 / 0.0 based on the case if the ratio is positive infinity or negative infinity.

Also, I have added 2 tests, which use string.includes() function, to check if the desired infinity value is contained in the generated string or not.

This PR is a pre-requisite for #1834.

Signed-off-by: Athan <kgryte@gmail.com>
Copy link
Member

@kgryte kgryte left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks, @gunjjoshi. You'll want to make a note of how I updated the tests. As done elsewhere in the tests, we need to test across all the different function types. In general, unit tests should be agnostic as possible regarding implementation details (i.e., they should test to API and expected behavior, not our knowledge of how things are implemented).

@kgryte kgryte changed the title refactor: modified math/base/tools/evalrational-compile-c refactor: add support for ratios evaluating as infinity in math/base/tools/evalrational-compile-c Mar 20, 2024
@kgryte kgryte merged commit f36b80d into stdlib-js:develop Mar 20, 2024
6 checks passed
@gunjjoshi
Copy link
Member Author

LGTM. Thanks, @gunjjoshi. You'll want to make a note of how I updated the tests. As done elsewhere in the tests, we need to test across all the different function types. In general, unit tests should be agnostic as possible regarding implementation details (i.e., they should test to API and expected behavior, not our knowledge of how things are implemented).

I will certainly make a note of that, thanks for sharing this !

@gunjjoshi gunjjoshi deleted the evalrational-compile-c branch March 21, 2024 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Math Issue or pull request specific to math functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants