Merge pull request #17266 from JuliaLang/teh/mapslices_copy

RFC: prevent mapslices from mutating the original array

Author: timholy

NEWS.md

@@ -93,7 +93,9 @@ Breaking changes
     If a reshaped copy is needed, use `copy(reshape(a))` or `copy!` to a new array of
     the desired shape ([#4211]).
 
-  * `mapslices` will always pass a view, so passing mutating functions will mutate the underlying array ([#16260])
+  * `mapslices` now re-uses temporary storage. Recipient functions
+    that expect input slices to be persistent should copy data to
+    other storage ([#17266]).
 
   * Local variables and arguments are represented in lowered code as numbered `Slot`
     objects instead of as symbols ([#15609]).
@@ -294,3 +296,4 @@ Deprecated or removed
 [#16645]: https://github.com/JuliaLang/julia/issues/16645
 [#16731]: https://github.com/JuliaLang/julia/issues/16731
 [#16972]: https://github.com/JuliaLang/julia/issues/16972
+[#17266]: https://github.com/JuliaLang/julia/issues/17266

base/abstractarray.jl

@@ -1417,7 +1417,8 @@ function mapslices(f, A::AbstractArray, dims::AbstractVector)
         idx[d] = Colon()
     end
 
-    r1 = f(view(A, idx...))
+    Aslice = A[idx...]
+    r1 = f(Aslice)
 
     # determine result size and allocate
     Rsize = copy(dimsA)
@@ -1449,7 +1450,8 @@ function mapslices(f, A::AbstractArray, dims::AbstractVector)
             for i in 1:nidx
                 idx[otherdims[i]] = ridx[otherdims[i]] = I.I[i]
             end
-            R[ridx...] = f(view(A, idx...))
+            _unsafe_getindex!(Aslice, A, idx...)
+            R[ridx...] = f(Aslice)
         end
     end
 

base/statistics.jl

@@ -492,8 +492,7 @@ end
 median!{T}(v::AbstractArray{T}) = median!(vec(v))
 median{T}(v::AbstractArray{T}) = median!(copy!(Array(T, length(v)), v))
 
-median!{T}(v::AbstractArray{T}, region) = mapslices(median!, v, region)
-median{T}(v::AbstractArray{T}, region) = median!(copy(v), region)
+median{T}(v::AbstractArray{T}, region) = mapslices(median!, v, region)
 
 # for now, use the R/S definition of quantile; may want variants later
 # see ?quantile in R -- this is type 7

test/arrayops.jl

@@ -843,6 +843,12 @@ let
     n3a = mapslices(x-> ones(1,6), c, [2,3])
     @test size(n1a) == (1,6,4) && size(n2a) == (1,3,6)  && size(n3a) == (2,1,6)
     @test size(n1) == (6,1,4) && size(n2) == (6,3,1)  && size(n3) == (2,6,1)
+
+    # mutating functions
+    o = ones(3, 4)
+    m = mapslices(x->fill!(x, 0), o, 2)
+    @test m == zeros(3, 4)
+    @test o == ones(3, 4)
 end