Skip to content

Commit

Permalink
Merge pull request #17266 from JuliaLang/teh/mapslices_copy
Browse files Browse the repository at this point in the history
RFC: prevent mapslices from mutating the original array
  • Loading branch information
timholy committed Jul 5, 2016
2 parents b25739d + d9ce339 commit 4913c25
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 5 deletions.
5 changes: 4 additions & 1 deletion NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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]).
Expand Down Expand Up @@ -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
6 changes: 4 additions & 2 deletions base/abstractarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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

Expand Down
3 changes: 1 addition & 2 deletions base/statistics.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions test/arrayops.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down

0 comments on commit 4913c25

Please sign in to comment.