make x[...] .= ... assign in-place

[stevengj]

This fixes a bug in #17510: x[...] .= ... and x[...] .+= ... etcetera should call broadcast! on view(x, ...), so that it continues to work in-place. (In Julia 0.4, x[...] .+= ... translates into a setindex! call.)

[stevengj]

(@timholy, is there anything about the current view implementation that would prevent broadcast!(identity, view(x, ...), rhs) from being a drop-in replacement for setindex!(x, rhs, ...)? It seems to work in all the cases I've tried.)

[stevengj]

@JeffBezanson, should this be (top view) rather than view?

[JeffBezanson]

Yes; I would go with top to avoid capturing locals that might be called view or broadcast.

[stevengj]

Marking as 0.5, since this needs to be resolved one way or the other (either by merging this in some form or by reverting #17510) by 0.5. There is code in existing packages that uses x[...] .+= ... and expects in-place updates.

[stevengj]

Okay to merge?

[tkelman]

would be good to get a confirmation on #17546 (comment) ?

[stevengj]

@tkelman, even if there is some case where view is problematic, I think that this PR is a strict improvement on the broken behavior we have currently (my fault!).

If view fails, my suspicion is that it will fail noisily — most likely a MethodError, if there is some custom array-like type that implements an odd getindex method but not a corresponding view method. (e.g. PyCall uses getindex in a weird way, and view is not implemented for PyObject. I'll have to think about whether .= even makes sense for a PyObject.)

[StefanKarpinski]

Seems like sound reasoning, @stevengj – in favor of merging.

[stevengj]

One case that we can't handle yet is that of a dictionary. Neither view nor broadcast! currently handle dictionaries, so something like mydict[key] .*= ... will throw a MethodError for now.

One way to handle this would be to implement a view(a::Associative, key) method that just returns a[key], I think. That way, if a[key] is a mutable array then a[key] .= ... will work as expected, while if a[key] is an immutable object then broadcast! will throw a MethodError.

[iamed2]

I think people are going to be confused when .= works when LHS is an array but not when LHS returns an array. Another case is:

julia> a = [[4, 5], [6, 7]]
julia> a[1] .= 3
ERROR: MethodError: Cannot `convert` an object of type Int64 to an object of type Array{Int64,1}
This may have arisen from a call to the constructor Array{Int64,1}(...),
since type constructors fall back to convert methods.
 in fill!(::SubArray{Array{Int64,1},0,Array{Array{Int64,1},1},Tuple{Int64},false}, ::Int64) at ./multidimensional.jl:549
 in broadcast!(::Base.#identity, ::SubArray{Array{Int64,1},0,Array{Array{Int64,1},1},Tuple{Int64},false}, ::Int64) at ./broadcast.jl:19
 in eval(::Module, ::Any) at ./boot.jl:234
 in macro expansion at ./REPL.jl:92 [inlined]
 in (::Base.REPL.##3#4{Base.REPL.REPLBackend})() at ./event.jl:46

The way to get the intended result is:

julia> a[1][:] .= 3
2-element SubArray{Int64,1,Array{Int64,1},Tuple{Colon},true}:
 3
 3
julia> a
2-element Array{Array{Int64,1},1}:
 [3,3]
 [6,7]

but that's not exactly obvious and might require a manual entry (unless a special case is made for scalar indexing).

AFAIK this is why MATLAB cell arrays have two types of indexing brackets: () to slice and {} to extract (not that I'm suggesting that).

[stevengj]

@iamed2, I think that most cases of interest could be supported by adding appropriate methods to broadcast! and/or view.

e.g. your particular case could be handled by defining:

Base.broadcast!{T<:AbstractArray}(f, x::SubArray{T,0}, args...) = broadcast!(f, x[], args...)
Base.broadcast!{T<:AbstractArray}(f::typeof(identity), x::SubArray{T,0}, arg::Number) = broadcast!(f, x[], arg) # prevent method ambiguity

We could also get more fine-grained control by defining a broadcastview(...) function, which defaults to view(...), to allow us to define behaviors that only affect the implicit views from .= and not explicit calls to view.

[stevengj]

Note also that this PR can be reverted in 0.6 if the arraypocolypse happens and slices become views. So potentially this is just a temporary fix.