Enable conversion of Float's to BigNum's #52
Conversation
Codecov Report
@@ Coverage Diff @@
## master #52 +/- ##
=======================================
Coverage 99.09% 99.09%
=======================================
Files 6 6
Lines 331 333 +2
=======================================
+ Hits 328 330 +2
Misses 3 3
Continue to review full report at Codecov.
|
| @@ -131,7 +131,7 @@ def _convert_simple(value, type, raise_coercion_error) | |||
| elsif value.is_a?(type) | |||
| return value | |||
| elsif type == BigDecimal | |||
| return BigDecimal(value) | |||
| return BigDecimal(value, 0) | |||
There was a problem hiding this comment.
I'm not fully sure this is what we would want 🤔
irb(main):001:0> BigDecimal(123.321, 0).to_digits
=> "123.320999999999997953636921010911464691"maybe we are better off calling to_s on the value?
| return BigDecimal(value, 0) | |
| return BigDecimal(value.to_s) |
irb(main):002:0> BigDecimal(123.321.to_s).to_digits
=> "123.321"However, we probably still need to check what to do with nils or things that don't respond to #to_s 🤔
There was a problem hiding this comment.
+1 to value.to_s
we probably still need to check what to do with nils or things that don't respond to #to_s
That's already taken care of on L125.
There was a problem hiding this comment.
This does not need to be part of this PR, but we should also catch argument errors like
ArgumentError (invalid value for BigDecimal(): "a")
in this method, so the error can be properly re-raised and being treated properly by the raise_coercion_error flag.
| @@ -131,7 +131,7 @@ def _convert_simple(value, type, raise_coercion_error) | |||
| elsif value.is_a?(type) | |||
| return value | |||
| elsif type == BigDecimal | |||
| return BigDecimal(value) | |||
| return BigDecimal(value, 0) | |||
There was a problem hiding this comment.
+1 to value.to_s
we probably still need to check what to do with nils or things that don't respond to #to_s
That's already taken care of on L125.
|
This would be useful to us as well. @kyku can you make the |
|
Hi @mattxwang, can this be folded in please? |
|
Hi @hlascelles, I unfortunately don't work at CZI anymore and don't have write access to this repository. I am not sure who still does, but you could always fork |
When converting from a Float, the following error is given:
Exception: can't omit precision for a Float.
This PR fixes that.