From 6ddbd340b0d61e0bdbaabd17b92ffd9985ef180f Mon Sep 17 00:00:00 2001 From: Morten Piibeleht Date: Thu, 21 Feb 2019 12:16:26 +1300 Subject: [PATCH] Make convert(Real, ::HalfInteger) yield HalfInteger (#5) As HalfInteger <: Real, there should be no reason to convert anything in this situation. It happens because the convert method resorts to Float64 as an intermediate value. To still get conversion to floats, we can just dispatch on AbstractFloat instead. However, it should be better to convert the numerator to T first and then divide, so that we would not use a potentially lower precision intermediate value. This solves the problem where calling sum on an vector of HalfIntegers yields a floating point value, even though there is no reason to convert in the summation: julia> sum([HalfInteger(1//2), HalfInteger(3//2)]) 2.0 This is because there is an implicit convert(::Real) in the Base.add_sum function. With this patch the sum call correctly yields a HalfInteger. It also updates the tests related to HalfInteger convert methods: - Make sure that the convert tests also check types - Add a few tests for converting out of HalfInteger --- src/halfinteger.jl | 2 +- test/halfinteger.jl | 18 ++++++++++++------ 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/halfinteger.jl b/src/halfinteger.jl index fb98314..0065c80 100644 --- a/src/halfinteger.jl +++ b/src/halfinteger.jl @@ -81,7 +81,7 @@ function Base.convert(::Type{HalfInteger}, r::Real) end Base.convert(T::Type{<:Integer}, s::HalfInteger) = iseven(s.numerator) ? convert(T, s.numerator>>1) : throw(InexactError(Symbol(T), T, s)) Base.convert(T::Type{<:Rational}, s::HalfInteger) = convert(T, s.numerator//2) -Base.convert(T::Type{<:Real}, s::HalfInteger) = convert(T, s.numerator/2) +Base.convert(T::Type{<:AbstractFloat}, s::HalfInteger) = convert(T, s.numerator) / T(2) Base.convert(::Type{HalfInteger}, s::HalfInteger) = s # Arithmetic diff --git a/test/halfinteger.jl b/test/halfinteger.jl index 3d30d65..2198f9c 100644 --- a/test/halfinteger.jl +++ b/test/halfinteger.jl @@ -21,15 +21,21 @@ using WignerSymbols: HalfInteger, ishalfinteger, HalfIntegerRange @test_throws ArgumentError HalfInteger(-1000, -999) # convert methods - @test convert(HalfInteger, 2) == HalfInteger(2, 1) - @test convert(HalfInteger, 1//2) == HalfInteger(1, 2) - @test convert(HalfInteger, 1.5) == HalfInteger(3, 2) + @test convert(HalfInteger, 2) === HalfInteger(2, 1) + @test convert(HalfInteger, 1//2) === HalfInteger(1, 2) + @test convert(HalfInteger, 1.5) === HalfInteger(3, 2) @test_throws InexactError convert(HalfInteger, 1//3) @test_throws InexactError convert(HalfInteger, 0.6) - @test convert(HalfInteger, 2) == 2 - @test convert(HalfInteger, 1//2) == 1//2 - @test convert(HalfInteger, 1.5) == 1.5 + @test convert(HalfInteger, 2) === HalfInteger(2, 1) + @test convert(HalfInteger, 1//2) === HalfInteger(1, 2) + @test convert(HalfInteger, 1.5) === HalfInteger(3, 2) + + @test convert(Integer, HalfInteger(2, 1)) === 2 @test_throws InexactError convert(Integer, HalfInteger(1, 2)) + @test convert(Float64, HalfInteger(3, 2)) isa Float64 + @test convert(Float32, HalfInteger(3, 2)) isa Float32 + @test convert(Float64, HalfInteger(3, 2)) == 1.5 + @test convert(Real, HalfInteger(3, 2)) === HalfInteger(3, 2) # single-argument constructor @test HalfInteger(0) == HalfInteger(0, 2)