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

Fix toPrecision #16

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

amoshydra
Copy link

@amoshydra amoshydra commented Jan 17, 2025

Moved from jessealama/decimal128#128
commits from jessealama/decimal128#128 are rebased onto the main branch of this repository

Summary

  1. Add more tests for toPrecision, implement toPrecision according to specification
  2. Change to existing test:
    Given Before After
    new Decimal128("123.456").toPrecision({ digits: 2 }) "12e+2" "12e+1"
    new Decimal128("0.0").toPrecision({ digits: 1 }) "0.0" "0"
  3. implement toPrecision closer to specification.
    some variables are renamed and created to match the text in the spec

Related to

#15

How is this tested?

See updated tests for toPrecision

image

@amoshydra amoshydra force-pushed the fix-rounding-and-to-precision branch 5 times, most recently from 263cdbf to 0306510 Compare January 17, 2025 19:25
@amoshydra amoshydra changed the title Fix rounding and to precision Fix toprecision Jan 17, 2025
@amoshydra amoshydra force-pushed the fix-rounding-and-to-precision branch from 0306510 to 8f29b7d Compare January 17, 2025 19:31
@amoshydra
Copy link
Author

turns out toPrecision can be fixed without fixing round first.
I have separated out the drafted test for round into #19.

Marking this PR as ready for review

@amoshydra amoshydra marked this pull request as ready for review January 17, 2025 19:37
@amoshydra amoshydra force-pushed the fix-rounding-and-to-precision branch from 8f29b7d to 2aada21 Compare January 17, 2025 19:42
@amoshydra amoshydra changed the title Fix toprecision Fix toPrecision Jan 17, 2025
@amoshydra
Copy link
Author

amoshydra commented Jan 17, 2025

Disclaimer

I am having a hard time understanding what coefficient mean in the spec. Do correct me if my interpretation is wrong.

Also the implementation slightly differ from the spec, as I couldn't get the implementation to work by following these highlighted part:
image

red annotation reflects the changes I've applied

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