Merge pull request #78 from JuliaStats/sjk/ambiguity

Fix ambiguity warnings (#77)

src/DataArrays.jl

@@ -68,6 +68,7 @@ module DataArrays
     include("statistics.jl")
     include("predicates.jl")
     include("literals.jl")
+    include("deprecated.jl")
 
     Base.@deprecate removeNA dropna
     Base.@deprecate set_levels setlevels

src/dataarray.jl

@@ -134,6 +134,12 @@ function Base.copy(d::DataArray) # -> DataArray{T}
     return DataArray(copy(d.data), copy(d.na))
 end
 
+function Base.copy!(dest::DataArray, src::DataArray) # -> DataArray{T}
+    copy!(dest.data, src.data)
+    copy!(dest.na, src.na)
+    dest
+end
+
 #' @description
 #'
 #' Create a deep copy of a DataArray.

src/deprecated.jl

@@ -0,0 +1,12 @@
+# Note: These methods need a more helpfull error message than a `NoMethodError`,
+#       when the deprecation is removed
+import Base.@deprecate
+@deprecate (+)(A::AbstractDataArray{Bool},x::Bool)              A .+ x
+@deprecate (+)(x::Bool,A::AbstractDataArray{Bool})              x .+ A 
+@deprecate (-)(A::AbstractDataArray{Bool},x::Bool)              A .- x
+@deprecate (-)(x::Bool,A::AbstractDataArray{Bool})              x .- A
+@deprecate (+)(A::AbstractDataArray,x::Union(NAtype,Number))    A .+ x
+@deprecate (+)(x::Union(NAtype,Number),A::AbstractDataArray)    x .+ A
+@deprecate (-)(A::AbstractDataArray,x::Union(NAtype,Number))    A .- x
+@deprecate (-)(x::Union(NAtype,Number),A::AbstractDataArray)    x .- A
+@deprecate (/)(x::Union(NAtype,Number),A::AbstractDataArray)    x ./ A

src/operators.jl

@@ -283,31 +283,39 @@ end
 #
 
 # Binary operators with one scalar argument
-macro dataarray_binary_scalar(vectorfunc, scalarfunc, outtype)
+macro dataarray_binary_scalar(vectorfunc, scalarfunc, outtype, swappable)
     esc(Expr(:block,
         # DataArray and AbstractDataArray with scalar
         # XXX It would be really nice to make this work with arbitrary
         # types, but doing so results in a bunch of method ambiguity
         # warnings
         {
-            quote
-                @swappable function $(vectorfunc)(a::DataArray, b::$t)
-                    data = a.data
-                    res = similar(data, $outtype)
-                    @bitenumerate a.na i na begin
-                        if !na
-                            @inbounds res[i] = $(scalarfunc)(data[i], b)
+            begin
+                fns = {
+                    :(function $(vectorfunc)(a::DataArray, b::$t)
+                        data = a.data
+                        res = similar(data, $outtype)
+                        @bitenumerate a.na i na begin
+                            if !na
+                                @inbounds res[i] = $(scalarfunc)(data[i], b)
+                            end
                         end
-                    end
-                    DataArray(res, copy(a.na))
-                end $scalarfunc
-                @swappable function $(vectorfunc)(a::AbstractDataArray, b::$t)
-                    res = similar(a, $outtype)
-                    for i = 1:length(a)
-                        res[i] = $(scalarfunc)(a[i], b)
-                    end
-                    res
-                end $scalarfunc
+                        DataArray(res, copy(a.na))
+                    end),
+                    :(function $(vectorfunc)(a::AbstractDataArray, b::$t)
+                        res = similar(a, $outtype)
+                        for i = 1:length(a)
+                            res[i] = $(scalarfunc)(a[i], b)
+                        end
+                        res
+                    end)
+                }
+                if swappable
+                    # For /, Array/Number is valid but not Number/Array
+                    # All other operators should be swappable
+                    map!(x->Expr(:macrocall, symbol("@swappable"), x, scalarfunc), fns)
+                end
+                Expr(:block, fns...)
             end
             for t in (:String, :Number)
         }...
@@ -320,10 +328,9 @@ macro dataarray_binary_array(vectorfunc, scalarfunc, outtype)
         # DataArray with other array
         {
             quote
-                function $(vectorfunc)(a::$(adata ? :DataArray : :AbstractArray),
-                                       b::$(bdata ? :DataArray : :AbstractArray))
-                    data1 = $(adata ? :(a.data) : :a)
-                    data2 = $(bdata ? :(b.data) : :b)
+                function $(vectorfunc)(a::$atype, b::$btype)
+                    data1 = $(atype == :DataArray || atype == :(DataArray{Bool}) ? :(a.data) : :a)
+                    data2 = $(btype == :DataArray || btype == :(DataArray{Bool}) ? :(b.data) : :b)
                     res = Array($outtype, promote_shape(size(a), size(b)))
                     resna = $narule
                     @bitenumerate resna i na begin
@@ -334,9 +341,12 @@ macro dataarray_binary_array(vectorfunc, scalarfunc, outtype)
                     DataArray(res, resna)
                 end
             end
-            for (adata, bdata, narule) in ((true, true, :(a.na | b.na)),
-                                           (true, false, :(copy(a.na))),
-                                           (false, true, :(copy(b.na))))
+            for (atype, btype, narule) in ((:(DataArray{Bool}), :(DataArray{Bool}), :(a.na | b.na)),
+                                           (:(DataArray{Bool}), :(AbstractArray{Bool}), :(copy(a.na))),
+                                           (:(AbstractArray{Bool}), :(DataArray{Bool}), :(copy(b.na))),
+                                           (:DataArray, :DataArray, :(a.na | b.na)),
+                                           (:DataArray, :AbstractArray, :(copy(a.na))),
+                                           (:AbstractArray, :DataArray, :(copy(b.na))))
         }...,
         # AbstractDataArray with other array
         # Definitinons with DataArray necessary to avoid ambiguity
@@ -350,7 +360,12 @@ macro dataarray_binary_array(vectorfunc, scalarfunc, outtype)
                     res
                 end
             end
-            for (asim, atype, btype) in ((true, :DataArray, :AbstractDataArray),
+            for (asim, atype, btype) in ((true, :(DataArray{Bool}), :(AbstractDataArray{Bool})),
+                                         (false, :(AbstractDataArray{Bool}), :(DataArray{Bool})),
+                                         (true, :(AbstractDataArray{Bool}), :(AbstractDataArray{Bool})),
+                                         (true, :(AbstractDataArray{Bool}), :(AbstractArray{Bool})),
+                                         (false, :(AbstractArray{Bool}), :(AbstractDataArray{Bool})),
+                                         (true, :DataArray, :AbstractDataArray),
                                          (false, :AbstractDataArray, :DataArray),
                                          (true, :AbstractDataArray, :AbstractDataArray),
                                          (true, :AbstractDataArray, :AbstractArray),
@@ -636,7 +651,7 @@ for (sf,vf) in zip(scalar_comparison_operators, array_comparison_operators)
         @swappable ($(vf))(::NAtype, b) = NA
         @swappable ($(sf))(::NAtype, b) = NA
 
-        @dataarray_binary_scalar $(vf) $(sf) Bool
+        @dataarray_binary_scalar $(vf) $(sf) Bool true
         @dataarray_binary_array $(vf) $(sf) Bool
     end
 end
@@ -653,16 +668,85 @@ for f in (:(Base.(:+)), :(Base.(:.+)), :(Base.(:-)), :(Base.(:.-)),
           :(Base.(:*)), :(Base.(:.*)), :(Base.(:.^)), :(Base.div),
           :(Base.mod), :(Base.fld), :(Base.rem))
     @eval begin
-        # Array with NA
-        @swappable $(f){T,N}(::NAtype, b::AbstractArray{T,N}) =
-            DataArray(Array(T, size(b)), trues(size(b)))
-
         # Scalar with NA
         ($f)(::NAtype, ::NAtype) = NA
         @swappable ($f)(d::NAtype, x::Number) = NA
+    end
+end
+
+# Define methods for UniformScaling. Otherwise we get ambiguity
+# warnings...
+function +{TA,TJ}(A::DataArray{TA,2},J::UniformScaling{TJ})
+    n = chksquare(A)
+    B = similar(A,promote_type(TA,TJ))
+    copy!(B,A)
+    @inbounds for i = 1:n
+        if !B.na[i,i]
+            B.data[i,i] += J.λ
+        end
+    end
+    B
+end
++{TA}(J::UniformScaling,A::DataArray{TA,2}) = A + J
+
+function -{TA,TJ<:Number}(A::DataArray{TA,2},J::UniformScaling{TJ})
+    n = chksquare(A)
+    B = similar(A,promote_type(TA,TJ))
+    copy!(B,A)
+    @inbounds for i = 1:n
+        if !B.na[i,i]
+            B.data[i,i] -= J.λ
+        end
+    end
+    B
+end
+function -{TA,TJ<:Number}(J::UniformScaling{TJ},A::DataArray{TA,2})
+    n = chksquare(A)
+    B = -A
+    @inbounds for i = 1:n
+        if !B.na[i,i]
+            B.data[i,i] += J.λ
+        end
+    end
+    B
+end
+
++(A::DataArray{Bool,2},J::UniformScaling{Bool}) =
+    invoke(+, (AbstractArray{Bool,2}, UniformScaling{Bool}), A, J)
++(J::UniformScaling{Bool},A::DataArray{Bool,2}) =
+    invoke(+, (UniformScaling{Bool}, AbstractArray{Bool,2}), J, A)
+-(A::DataArray{Bool,2},J::UniformScaling{Bool}) =
+    invoke(-, (AbstractArray{Bool,2}, UniformScaling{Bool}), A, J)
+-(J::UniformScaling{Bool},A::DataArray{Bool,2}) =
+    invoke(-, (UniformScaling{Bool}, AbstractArray{Bool,2}), J, A)
+
++{TA,TJ}(A::AbstractDataArray{TA,2},J::UniformScaling{TJ}) =
+    invoke(+, (AbstractArray{TA,2}, UniformScaling{TJ}), A, J)
++{TA}(J::UniformScaling,A::AbstractDataArray{TA,2}) =
+    invoke(+, (UniformScaling, AbstractArray{TA,2}), J, A)
+-{TA,TJ<:Number}(A::AbstractDataArray{TA,2},J::UniformScaling{TJ}) =
+    invoke(-, (AbstractArray{TA,2}, UniformScaling{TJ}), A, J)
+-{TA,TJ<:Number}(J::UniformScaling{TJ},A::AbstractDataArray{TA,2}) =
+    invoke(-, (UniformScaling{TJ}, AbstractArray{TA,2}), J, A)
+
++(A::AbstractDataArray{Bool,2},J::UniformScaling{Bool}) =
+    invoke(+, (AbstractArray{Bool,2}, UniformScaling{Bool}), A, J)
++(J::UniformScaling{Bool},A::AbstractDataArray{Bool,2}) =
+    invoke(+, (UniformScaling{Bool}, AbstractArray{Bool,2}), J, A)
+-(A::AbstractDataArray{Bool,2},J::UniformScaling{Bool}) =
+    invoke(-, (AbstractArray{Bool,2}, UniformScaling{Bool}), A, J)
+-(J::UniformScaling{Bool},A::AbstractDataArray{Bool,2}) =
+    invoke(-, (UniformScaling{Bool}, AbstractArray{Bool,2}), J, A)
+
+for f in (:(Base.(:.+)), :(Base.(:.-)), :(Base.(:*)), :(Base.(:.*)),
+          :(Base.(:.^)), :(Base.div), :(Base.mod), :(Base.fld), :(Base.rem))
+    @eval begin
+        # Array with NA
+        @swappable $(f){T,N}(::NAtype, b::AbstractArray{T,N}) =
+            DataArray(Array(T, size(b)), trues(size(b)))
 
         # DataArray with scalar
-        @dataarray_binary_scalar $f $f promote_type(eltype(a), eltype(b))
+        @dataarray_binary_scalar $f $f promote_type(eltype(a), eltype(b)) true
     end
 end
 
@@ -679,27 +763,32 @@ for (vf, sf) in ((:(Base.(:+)), :(Base.(:+))),
                  (:(Base.(:.^)), :(Base.(:^))))
     @eval begin
         # Necessary to avoid ambiguity warnings
-        @swappable ($vf)(A::BitArray, B::AbstractDataArray) = ($vf)(bitunpack(A), B)
-        @swappable ($vf)(A::BitArray, B::DataArray) = ($vf)(bitunpack(A), B)
+        @swappable ($vf)(A::BitArray, B::AbstractDataArray{Bool}) = ($vf)(bitunpack(A), B)
+        @swappable ($vf)(A::BitArray, B::DataArray{Bool}) = ($vf)(bitunpack(A), B)
 
         @dataarray_binary_array $vf $sf promote_type(eltype(a), eltype(b))
     end
 end
 
-@swappable Base.(:./)(A::BitArray, B::AbstractDataArray) = ./(bitunpack(A), B)
-@swappable Base.(:./)(A::BitArray, B::DataArray) = ./(bitunpack(A), B)
+@swappable Base.(:./)(A::BitArray, B::AbstractDataArray{Bool}) = ./(bitunpack(A), B)
+@swappable Base.(:./)(A::BitArray, B::DataArray{Bool}) = ./(bitunpack(A), B)
 
 # / and ./ are defined separately since they promote to floating point
 for f in ((:(Base.(:/)), :(Base.(:./))))
     @eval begin
-        @swappable $(f){T,N}(::NAtype, b::AbstractArray{T,N}) =
-            DataArray(Array(T, size(b)), trues(size(b)))
         ($f)(::NAtype, ::NAtype) = NA
         @swappable ($f)(d::NAtype, x::Number) = NA
-        @dataarray_binary_scalar $f Base.(:/) eltype(a) <: FloatingPoint || typeof(b) <: FloatingPoint ?
-                                              promote_type(eltype(a), typeof(b)) : Float64
     end
 end
+
+Base.(:/){T,N}(b::AbstractArray{T,N}, ::NAtype) =
+    DataArray(Array(T, size(b)), trues(size(b)))
+@dataarray_binary_scalar Base.(:/) Base.(:/) eltype(a) <: FloatingPoint || typeof(b) <: FloatingPoint ?
+                                      promote_type(eltype(a), typeof(b)) : Float64 false                   
+@swappable Base.(:./){T,N}(::NAtype, b::AbstractArray{T,N}) =
+    DataArray(Array(T, size(b)), trues(size(b)))
+@dataarray_binary_scalar Base.(:./) Base.(:/) eltype(a) <: FloatingPoint || typeof(b) <: FloatingPoint ?
+                                      promote_type(eltype(a), typeof(b)) : Float64 true
 @dataarray_binary_array Base.(:./) Base.(:/) eltype(a) <: FloatingPoint || eltype(b) <: FloatingPoint ?
                                              promote_type(eltype(a), eltype(b)) : Float64
 

test/data.jl

@@ -84,7 +84,7 @@ module TestData
     @assert eltype(pdvstr) == ASCIIString
 
     #test_group("DataVector operations")
-    @assert isequal(dvint + 1, DataArray([2, 3, 4, 5], [false, false, true, false]))
+    @assert isequal(dvint .+ 1, DataArray([2, 3, 4, 5], [false, false, true, false]))
     @assert isequal(dvint .* 2, @data([2, 4, NA, 8]))
     @assert isequal(dvint .== 2, @data([false, true, NA, false]))
     @assert isequal(dvint .> 1, @data([false, true, NA, true]))

test/dataarray.jl

@@ -36,6 +36,10 @@ module TestDataArray
     y = [1, 9, 3, 2, 2]
     @assert append!(x, y) == [9, 9, 8, 1, 9, 3, 2, 2]
 
+    x = @data [1, 2, NA]
+    y = @data [3, NA, 5]
+    @test isequal(copy(x), x)
+    @test isequal(copy!(y, x), x)
 
 	# Test vecbind
 	# a = [1:4]

test/operators.jl

@@ -109,24 +109,32 @@ module TestOperators
     # Broadcasting operations between NA's and DataVector's
     dv = convert(DataArray, ones(5))
     @test_da_pda dv begin
-        for f in map(eval, DataArrays.arithmetic_operators)
+        for f in map(eval, [:(Base.(:.+)),
+                            :(Base.(:.-)),
+                            :(Base.(:*)),
+                            :(Base.(:.*)),
+                            :(Base.(:./)),
+                            :(Base.(:.^)),
+                            :(Base.div),
+                            :(Base.mod),
+                            :(Base.fld),
+                            :(Base.rem)])
             for i in 1:length(dv)
                 @assert isna(f(dv, NA)[i])
                 @assert isna(f(NA, dv)[i])
+                @assert f(dv, 1)[i] == f(dv[i], 1)
+                @assert f(1, dv)[i] == f(1, dv[i])
             end
         end
     end
 
-    # Broadcasting operations between scalars and DataVector's
-    dv = convert(DataArray, ones(5))
     @test_da_pda dv begin
-        for f in map(eval, DataArrays.arithmetic_operators)
-            for i in 1:length(dv)
-                @assert f(dv, 1)[i] == f(dv[i], 1)
-                @assert f(1, dv)[i] == f(1, dv[i])
-            end
+        for i in 1:length(dv)
+            @assert isna(/(dv, NA)[i])
+            @assert /(dv, 1)[i] == /(dv[i], 1)
         end
     end
+
     dv = @data([false, true, false, true, false])
     for f in map(eval, DataArrays.bit_operators)
         for i in 1:length(dv)
@@ -178,6 +186,18 @@ module TestOperators
         end
     end
 
+    # + and - with UniformScaling
+    # mI = zeros(5, 5) + 5I
+    # for dm in (convert(DataArray, ones(5, 5)), convert(DataArray, trues(5, 5)))
+    #     dm[1] = NA
+    #     @test_da_pda dm begin
+    #         @test dm + 5I == dm + mI
+    #         @test 5I + dm == mI + dm
+    #         @test dm - 5I == dm - mI
+    #         @test 5I - dm == mI - dm
+    #     end
+    # end
+
     # Division (special case since return type for Int is a Float64)
     for curdv in (dv,
                   convert(DataVector{Int}, dv),