Allow rounding with sigdigits

[mcabbott]

Minimal fix for #326

[gustaphe]

This is a nobrainer to me, and I was looking into trying to fix whatever made the build fail, but that coveralls message is incomprehensible.

[mcabbott]

These things always seem noisy. It does say "4 of 4 new or added lines in 1 file covered. (100.0%)"

[JeffFessler]

bump :)

[sostock]

I’m not sure I like this, since it is not invariant under unit conversion:

julia> x = 2.5u"inch"
2.5 inch

julia> y = uconvert(u"cm", x)
6.35 cm

julia> x == y
true

julia> round(x, sigdigits=2) ≈ round(y, sigdigits=2)
false

[mcabbott]

After rounding, shouldn't the test be approximate equality at the resolution you chose? The default is about 8 digits, and you've declared that you don't care about most of them. Of course sigdigits isn't a very precise specification of rtol, but I hope we're not keeping too many digits on the number of digits we're keeping.

julia> x = (exp(1))u"inch"
2.718281828459045 inch

julia> y = uconvert(u"cm", x)
6.904435844285975 cm

julia> @test x ≈ round(y, sigdigits=2) rtol=0.01
Test Passed
  Expression: ≈(x, round(y, sigdigits = 2), rtol = 0.01)
   Evaluated: ≈(2.718281828459045 inch, 6.9 cm; rtol = 0.01)

julia> @test y ≈ round(x, sigdigits=2) rtol=0.01
Test Passed
  Expression: ≈(y, round(x, sigdigits = 2), rtol = 0.01)
   Evaluated: ≈(6.904435844285975 cm, 2.7 inch; rtol = 0.01)

julia> @test round(x, sigdigits=2) ≈ round(y, sigdigits=2) rtol=0.01
Test Passed
  Expression: ≈(round(x, sigdigits = 2), round(y, sigdigits = 2), rtol = 0.01)
   Evaluated: ≈(2.7 inch, 6.9 cm; rtol = 0.01)

[sostock]

It’s a goal of this package that all functionality is unit-invariant. That’s why round takes a unit argument in the first place.

For x == y, you will always have round(u"m", x, sigdigits=2) == round(u"m", y, sigdigits=2) (ignoring floating-point effects) even when x and y have different units. This is not the case when you do not specify the first argument.

However, there are already some non-invariant methods in this package, so I guess one more doesn’t hurt. For example, I really don’t like that div(5000u"m", 2) != div(5u"km", 2), but other maintainers don’t think it’s too bad. I’m not generally opposed to this PR. I just wanted to mention that there is a reason that round without a unit argument was forbidden, and this PR ignores that reason. Even though the non-invariance may be less significant when using sigdigits, I don’t think it is completely unsignificant.

[codecov-commenter]

Codecov Report

Merging #328 (ec7cbba) into master (e97233b) will increase coverage by 3.50%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #328      +/-   ##
==========================================
+ Coverage   80.23%   83.74%   +3.50%     
==========================================
  Files          15       16       +1     
  Lines        1174     1347     +173     
==========================================
+ Hits          942     1128     +186     
+ Misses        232      219      -13     

Impacted Files	Coverage Δ
src/quantities.jl	93.77% <100.00%> (+0.44%)	⬆️
src/pkgdefaults.jl	18.60% <0.00%> (-11.40%)	⬇️
src/types.jl	89.79% <0.00%> (-3.83%)	⬇️
src/display.jl	95.23% <0.00%> (-0.85%)	⬇️
src/user.jl	94.26% <0.00%> (-0.77%)	⬇️
src/Unitful.jl	100.00% <0.00%> (ø)
src/dates.jl	95.77% <0.00%> (ø)
src/units.jl	85.51% <0.00%> (+0.58%)	⬆️
src/fastmath.jl	84.50% <0.00%> (+1.96%)	⬆️
... and 6 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e97233b...ec7cbba. Read the comment docs.