Skip to content

sign of zero fraction should return 0/1 instead of 1/1#3513

Merged
josdejong merged 3 commits into
josdejong:developfrom
kyle-compute:develop
Jul 24, 2025
Merged

sign of zero fraction should return 0/1 instead of 1/1#3513
josdejong merged 3 commits into
josdejong:developfrom
kyle-compute:develop

Conversation

@kyle-compute
Copy link
Copy Markdown

@kyle-compute kyle-compute commented Jul 23, 2025

Original Issue: #3512

math.sign(0) // gives 0 ✅
math.sign(math.complex(0,0)) // gives 0+0i ✅
math.sign(math.fraction(0)) // gives 1/1 ❌, it should give 0/1 

Fixed:

issue where math.sign(math.fraction(0)) incorrectly returned 1/1 instead of 0/1. Changed fraction sign detection from checking x.s to checking x.n === 0n (numerator is

Modified:

/function/arithmetic/sign.js:55 - Changed from return new Fraction(x.s) to return x.n === 0n ? new Fraction(0) : new Fraction(x.s)

Added:

test/unit-tests/function/arithmetic/sign.test.js:108-110 - Added test case for math.sign(math.fraction(0))

npm run lint and npm test had no issues :)

Copy link
Copy Markdown
Owner

@josdejong josdejong left a comment

Choose a reason for hiding this comment

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

Thanks Kyle! I made one small inline remark, can you have a look at that?

Comment thread test/unit-tests/function/arithmetic/sign.test.js Outdated
@kyle-compute
Copy link
Copy Markdown
Author

All fixed :D

Moved testcase into correct function block

@josdejong josdejong merged commit aedbee5 into josdejong:develop Jul 24, 2025
8 checks passed
@josdejong
Copy link
Copy Markdown
Owner

Thanks again!

@josdejong
Copy link
Copy Markdown
Owner

josdejong commented Jul 25, 2025

The fix is published now in v14.6.0 🎉 .

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