Changed meaning of .+= in Julia 0.5

[stevengj]

Your test code uses x .+= y, so you should know that in Julia 0.5 this has changed meaning to be equivalent to broadcast!(identity, x, x .+ y), so that it mutates the x array (see JuliaLang/julia#17510 … in Julia 0.6 the whole operation will occur in-place without temporaries). So .+ should only be used if the left-hand side is a mutable array, and you don't mind mutating it.

At first glance, this looks like it is okay for you, because you use it in data .+= stat.range * (rand(rng, length(data)) - 0.5) .* span, where data seems like an array that you don't mind mutating. But if it were a problem you could always change it to +=.

[stevengj]

Similarly for .*= and ./=.

[shashi]

I missed this issue and didn't know how my changes fixed the problem. Thanks! :) There are two occurances of this left, both are:

bincounts ./= sum(bincounts) * binwidth

I think this is fine?

[stevengj]

@shashi, I don't quite see what problem your changes fixed. aes.width .*= 1 - pad is equivalent (in 0.5) to aes.width .= aes.width .* (1 - pad), which is equivalent to broadcast!(identity, aes.width , aes.width .* (1 - pad)), which seems like it should work (albeit suboptimally). Changing it to broadcast!(*, aes.width, aes.width, 1 - pad) is faster, because it avoids allocating an extra temporary array, but shouldn't it give the same result?

(In 0.6, the .* will eventually fuse with the .= and give the same result as what you did manually.)

[shashi]

Ah, the error was:

ERROR: LoadError: MethodError: Cannot `convert` an object of type Float64 to an object of type Gadfly.Stat.Identity
This may have arisen from a call to the constructor Gadfly.Stat.Identity(...),
since type constructors fall back to convert methods.
 in macro expansion at ./broadcast.jl:129 [inlined]
 in macro expansion at ./simdloop.jl:73 [inlined]
 in macro expansion at ./broadcast.jl:123 [inlined]
 in _broadcast!(::Type{Gadfly.Stat.Identity}, ::Array{Float64,1}, ::Tuple{Tuple{Bool}}, ::Tuple{Tuple{Int64}}, ::Tuple{Array{Float64,1}}, ::Type{Val{1}}) at ./broadcast.jl:117
 in broadcast!(::Type{T}, ::Array{Float64,1}, ::Array{Float64,1}) at ./broadcast.jl:172
 in apply_statistic(::Gadfly.Stat.ViolinStatistic, ::Dict{Symbol,Gadfly.ScaleElement}, ::Gadfly.Coord.Cartesian, ::Gadfly.Aesthetics) at /home/shashi/.julia/v0.5/Gadfly/src/statistics.jl:1683
...

I now see how this happens... It looks like this syntactic transform just places the identifier identity as the first argument to broadcast! which picks up the variable defined here instead...

Maybe the parser should expand it to Base.identity?

[stevengj]

@shashi, good catch, I'll submit a PR to Julia ASAP.

[shashi]

Thanks! I tried editing the scheme file, but couldn't get it to work with '(|.| Base identity), I'll wait for your patch

(edit: vimium clicked on comment and close before I was finished. I'll let this remain close)