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

Bugfix: BigFloat#to_s not showing decimals for whole numbers #7982

Conversation

@Lasvad
Copy link
Contributor

commented Jul 22, 2019

add ".0" if value contains no decimal points for to_s.
update specs to reflect having ".0" for whole numbers.

ex.

Previous
3.to_big_f.to_s -> "3"
(3.0).to_big_f.to_s -> "3"

Changed to
3.to_big_f.to_s -> "3.0"
(3.0).to_big_f.to_s -> "3.0"

Issue #7974

@asterite
Copy link
Member

left a comment

Thank you!

@rodrigopinto
Copy link
Contributor

left a comment

Thanks for the PR @Lasvad! I was looking into the same issue yesterday and wind up with trick cases that I shared in this review. My suggestion is to add more test cases to cover all known unhappy paths.

Some additional examples to the ones you started:

it { 2.4.to_big_f.to_s.should eq("2.4") }
it { 0.to_big_f.to_s.should eq("0") }
it { 0.000001.to_big_f.to_s.should eq("0.000001") }
it { 1.0.to_big_f.to_s.should eq("1.0") }
it { 1.3.to_big_f.to_s.should eq("1.3") }

ps: I did not find out the solution yet.

spec/std/big/big_float_spec.cr Show resolved Hide resolved
@vlazar

This comment has been minimized.

Copy link
Contributor

commented Jul 22, 2019

With changes on PR these still fail to add .0

"10".to_big_f.to_s # => 10
"100".to_big_f.to_s # => 100
"150".to_big_f.to_s # => 150
@Lasvad

This comment has been minimized.

Copy link
Contributor Author

commented Jul 22, 2019

@vlazar hmm...

it { "1234567890123456789".to_big_f.to_s.should eq("1234567890123456789.0") }

this test passed so im surprised that

"10".to_big_f.to_s # => 10

doesn't work.

I'll take a look as soon as I can and put a fix in. Apologies.

@vlazar

This comment has been minimized.

Copy link
Contributor

commented Jul 22, 2019

@Lasvad Any numbers ending with 0 won't add .0 at the end.

@asterite

This comment has been minimized.

Copy link
Member

commented Jul 22, 2019

My suggestion for implementing this: keep a bool that tracks whether . was printed. If not, print it at the end.

@Lasvad

This comment has been minimized.

Copy link
Contributor Author

commented Jul 22, 2019

@vlazar thanks for confirming the source of the error. I'll add your test cases when I implment the fix.
@asterite Cool. I'll look into implementing something like that in a few hours when I get home.

add ".0" if value contains no decimal points for to_s
update specs to reflect having ".0" for whole numbers

@Lasvad Lasvad force-pushed the Lasvad:big_float-to_string-method-not-including-decimal-0-for-values-with-no-decimal-value branch from b8a2780 to be6e87e Jul 23, 2019

@Lasvad

This comment has been minimized.

Copy link
Contributor Author

commented Jul 23, 2019

I've made the changes and used a flag to check if a decimal was set.

src/big/big_float.cr Show resolved Hide resolved

@bcardiff bcardiff added this to the 0.30.0 milestone Jul 25, 2019

@bcardiff bcardiff merged commit 7c257a5 into crystal-lang:master Jul 25, 2019

5 checks passed

ci/circleci: check_format Your tests passed on CircleCI!
Details
ci/circleci: test_darwin Your tests passed on CircleCI!
Details
ci/circleci: test_linux Your tests passed on CircleCI!
Details
ci/circleci: test_linux32 Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.