Allow BigDecimal accept Float without precision

[mrzasa]

Make sure that conversion from a Float works consistently with the conversion from a string representing that float.

Current master state:

pry(main)> BigDecimal(1.2)
ArgumentError: can't omit precision for a Float.
from (pry):22:in `BigDecimal'
pry(main)> BigDecimal('1.2')
=> 0.12e1

After this changes:

pry(main)> BigDecimal(1.2)
=> 0.12e1
pry(main)> BigDecimal('1.2')
=> 0.12e1

When precision is omitted it's set to 0.

Fixes #213
Blocked by #313

[mrkn]

@mrzasa Could you add more assertions to check many cases, especially for numbers that don't have accurate representations in Float?
Moreover, we must compare two values by assert_in_delta and pass an appropriate value to the delta argument for the original Float value; it should be 0.5ulp.

[mrkn]

I reconsidered how to check here.

I suggest comparing the result of to_f with the original Float value, like this: assert_equal(4.2, BigDecimal(4.2).to_f).
This approach is preferable because it doesn’t depend on the specific decimal representation for a given Float value. I’d like to avoid relying on the particular algorithm used for converting a Float to a BigDecimal.

[tompng]

Adding a same test case as BigDecimal(float, 0) is enough.
But since test for BigDecimal(float, 0) is currently missing, adding both will be nice.

assert_equal(BigDecimal("string representation"), BigDecimal(float_literal_here, 0))
assert_equal(BigDecimal("string representation"), BigDecimal(float_literal_here))

# Just like this one on line 166-168
assert_equal(BigDecimal("0.1235"), BigDecimal(0.1234567, 4))

[mrzasa]

Thanks! I've added the floats test cases

[tompng]

Thank you. looks good 👍

Perhaps some tests for BigDecimal(x, various_number) are missing, some test are too strict.

# this test is too strict
assert_equal(BigDecimal("0.01"), BigDecimal(0.01, Float::DIG + 1))
# because:
BigDecimal(0.07, Float::DIG + 1) #=> 0.7000000000000001e-1

but let's consider it a separate issue of test.

[henrik]

Is this intentional?

DEV (main)> BigDecimal(1.1111111111111129).to_s
"1.111111111111113"
DEV (main)> BigDecimal("1.1111111111111129").to_s
"1.1111111111111129"

DEV (main)> RUBY_VERSION
"3.4.5"
DEV (main)> BigDecimal::VERSION
"3.2.3"
DEV (main)> RUBY_PLATFORM
"x86_64-linux"

[tompng]

@henrik Yes, it's intentional. That's what Float number is.
1.1111111111111129 and 1.111111111111113 are exactly the same value in Ruby.
So passing it to BigDecimal gets the same result.

1.1111111111111129 == 1.111111111111113
#=> true

p 1.1111111111111129
#=> 1.111111111111113

BigDecimal(1.1111111111111129) == BigDecimal(1.111111111111113)
#=> true 

p 1.100000000000000123456789
# => 1.1
p BigDecimal(1.100000000000000123456789)
#=> 0.11e1

[henrik]

@tompng Thank you for clarifying!

It still does not seem ideal that BigDecimal will now happily accept a float where the literal representation does not match the underlying decimal value, e.g.

DEV (main)> BigDecimal("1.1111111111111129") == BigDecimal(1.1111111111111129)
false

I know that this was already the case before if you passed an explicit precision argument (BigDecimal(1.1111111111111129, 0)). It just seems like something of a step backwards that if a user is not aware of this nuance of float imprecision, it's now easier for them to shoot themselves in the foot.

That said, I don't myself have a use case for BigDecimal with this many decimals – I was just experimenting.

[crankycoder]

@tompng Thank you for clarifying!

It still does not seem ideal that BigDecimal will now happily accept a float where the literal representation does not match the underlying decimal value, e.g.

This is at least a breaking API change and should necessitate a major version bump.

I have seen multiple cases of production code that expects BigDecimal(<float>) to throw an ArgumentException.

For usecases like validation - the new API will allow what would have been considered invalid data to now be considered valid.

[henrik]

Yes, agreed, assuming BigDecimal intends to follow semver. It broke code for us and rails/rails@194ff4c above shows the same thing happening to Rails.