Skip to content

Commit

Permalink
Deprecate similar(f, …) in favor of just dispatching directly on f(…) (
Browse files Browse the repository at this point in the history
…#26733)

Remove AbstractArray constructors; just use dispatch on functions

Make compiler test use a local function that has the known properties that we want to test. The important thing is not necessarily zeros -- it is a relatively complex function. The `Base.zeros` method tree temporarily breaks this test due to its numerous deprecations, but this will be resolved in the future.
  • Loading branch information
mbauman authored and JeffBezanson committed Apr 13, 2018
1 parent f82eb5b commit 2233ec4
Show file tree
Hide file tree
Showing 14 changed files with 116 additions and 61 deletions.
10 changes: 10 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -758,6 +758,16 @@ Deprecated or removed
necessary, consider `fill!(similar(A[, opts...]), {one(eltype(A)) | zero(eltype(A))})`.
For an algebraic multiplicative identity, consider `one(A)` ([#24656]).

* The `similar(dims->f(..., dims...), [T], axes...)` method to add offset array support
to a function `f` that would otherwise create a non-offset array has been deprecated.
Instead, call `f(..., axes...)` directly and, if needed, the offset array implementation
should add offset axis support to the function `f` directly ([#26733]).

* The functions `ones` and `zeros` used to accept any objects as dimensional arguments,
implicitly converting them to `Int`s. This is now deprecated; only `Integer`s or
`AbstractUnitRange`s are accepted as arguments. Instead, convert the arguments before
calling `ones` or `zeros` ([#26733]).

* The `Operators` module is deprecated. Instead, import required operators explicitly
from `Base`, e.g. `import Base: +, -, *, /` ([#22251]).

Expand Down
17 changes: 2 additions & 15 deletions base/abstractarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -590,14 +590,9 @@ indices of the result will match `A`.
would create a 1-dimensional logical array whose indices match those
of the columns of `A`.
similar(dims->zeros(Int, dims), axes(A))
would create an array of `Int`, initialized to zero, matching the
indices of `A`.
"""
similar(f, shape::Tuple) = f(to_shape(shape))
similar(f, dims::DimOrInd...) = similar(f, dims)
similar(::Type{T}, shape::Tuple) where {T} = T(to_shape(shape))
similar(::Type{T}, dims::DimOrInd...) where {T} = similar(T, dims)

"""
empty(v::AbstractVector, [eltype])
Expand Down Expand Up @@ -886,14 +881,6 @@ isempty(a::AbstractArray) = (_length(a) == 0)
# keys with an IndexStyle
keys(s::IndexStyle, A::AbstractArray, B::AbstractArray...) = eachindex(s, A, B...)

"""
of_indices(x, y)
Represents the array `y` as an array having the same indices type as `x`.
"""
of_indices(x, y) = similar(dims->y, oftype(axes(x), axes(y)))


## range conversions ##

map(::Type{T}, r::StepRange) where {T<:Real} = T(r.start):T(r.step):T(last(r))
Expand Down
19 changes: 13 additions & 6 deletions base/array.jl
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,9 @@ function fill!(a::Array{T}, x) where T<:Union{Integer,AbstractFloat}
return a
end

to_dim(d::Integer) = d
to_dim(d::OneTo) = last(d)

"""
fill(x, dims)
Expand All @@ -347,8 +350,10 @@ julia> fill(1.0, (5,5))
If `x` is an object reference, all elements will refer to the same object. `fill(Foo(),
dims)` will return an array filled with the result of evaluating `Foo()` once.
"""
fill(v, dims::Dims) = fill!(Array{typeof(v)}(undef, dims), v)
fill(v, dims::Integer...) = fill!(Array{typeof(v)}(undef, dims...), v)
fill(v, dims::DimOrInd...) = fill(v, dims)
fill(v, dims::NTuple{N, Union{Integer, OneTo}}) where {N} = fill(v, map(to_dim, dims))
fill(v, dims::NTuple{N, Integer}) where {N} = fill!(Array{typeof(v),N}(undef, dims), v)
fill(v, dims::Tuple{}) = fill!(Array{typeof(v),0}(undef, dims), v)

"""
zeros([T=Float64,] dims...)
Expand Down Expand Up @@ -392,10 +397,12 @@ function ones end

for (fname, felt) in ((:zeros, :zero), (:ones, :one))
@eval begin
$fname(::Type{T}, dims::NTuple{N, Any}) where {T, N} = fill!(Array{T,N}(undef, convert(Dims, dims)::Dims), $felt(T))
$fname(dims::Tuple) = ($fname)(Float64, dims)
$fname(::Type{T}, dims...) where {T} = $fname(T, dims)
$fname(dims...) = $fname(dims)
$fname(dims::DimOrInd...) = $fname(dims)
$fname(::Type{T}, dims::DimOrInd...) where {T} = $fname(T, dims)
$fname(dims::Tuple{Vararg{DimOrInd}}) = $fname(Float64, dims)
$fname(::Type{T}, dims::NTuple{N, Union{Integer, OneTo}}) where {T,N} = $fname(T, map(to_dim, dims))
$fname(::Type{T}, dims::NTuple{N, Integer}) where {T,N} = fill!(Array{T,N}(undef, map(to_dim, dims)), $felt(T))
$fname(::Type{T}, dims::Tuple{}) where {T} = fill!(Array{T}(undef), $felt(T))
end
end

Expand Down
16 changes: 10 additions & 6 deletions base/bitarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ julia> BitArray(undef, (3, 1))
"""
BitArray(::UndefInitializer, dims::Integer...) = BitArray(undef, map(Int,dims))
BitArray{N}(::UndefInitializer, dims::Integer...) where {N} = BitArray{N}(undef, map(Int,dims))
BitArray(::UndefInitializer, dims::NTuple{N,Int}) where {N} = BitArray{N}(undef, dims...)
BitArray{N}(::UndefInitializer, dims::NTuple{N,Int}) where {N} = BitArray{N}(undef, dims...)
BitArray(::UndefInitializer, dims::NTuple{N,Integer}) where {N} = BitArray{N}(undef, map(Int, dims)...)
BitArray{N}(::UndefInitializer, dims::NTuple{N,Integer}) where {N} = BitArray{N}(undef, map(Int, dims)...)

const BitVector = BitArray{1}
const BitMatrix = BitArray{2}
Expand Down Expand Up @@ -366,8 +366,10 @@ julia> falses(2,3)
false false false
```
"""
falses(dims::Dims) = fill!(BitArray(undef, dims), false)
falses(dims::Integer...) = falses(map(Int,dims))
falses(dims::DimOrInd...) = falses(dims)
falses(dims::NTuple{N, Union{Integer, OneTo}}) where {N} = falses(map(to_dim, dims))
falses(dims::NTuple{N, Integer}) where {N} = fill!(BitArray(undef, dims), false)
falses(dims::Tuple{}) = fill!(BitArray(undef, dims), false)

"""
trues(dims)
Expand All @@ -382,8 +384,10 @@ julia> trues(2,3)
true true true
```
"""
trues(dims::Dims) = fill!(BitArray(undef, dims), true)
trues(dims::Integer...) = trues(map(Int,dims))
trues(dims::DimOrInd...) = trues(dims)
trues(dims::NTuple{N, Union{Integer, OneTo}}) where {N} = trues(map(to_dim, dims))
trues(dims::NTuple{N, Integer}) where {N} = fill!(BitArray(undef, dims), true)
trues(dims::Tuple{}) = fill!(BitArray(undef, dims), true)

function one(x::BitMatrix)
m, n = size(x)
Expand Down
20 changes: 20 additions & 0 deletions base/deprecated.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1567,6 +1567,26 @@ end
@deprecate Crand Libc.rand false
@deprecate Csrand Libc.srand false

# Deprecate `similar(f, axes)` (PR #26733)
@noinline function similar(f, shape::Tuple)
depwarn("using similar(f, shape) to call `f` with axes `shape` is deprecated; call `f` directly and/or add methods such that it supports axes", :similar)
f(to_shape(shape))
end
@noinline function similar(f, dims::DimOrInd...)
depwarn("using similar(f, shape...) to call `f` with axes `shape` is deprecated; call `f` directly and/or add methods such that it supports axes", :similar)
f(to_shape(dims))
end
# Deprecate non-integer/axis arguments to zeros/ones to match fill/trues/falses
@deprecate zeros(::Type{T}, dims...) where {T} zeros(T, convert(Dims, dims)...)
@deprecate zeros(dims...) zeros(convert(Dims, dims)...)
@deprecate zeros(::Type{T}, dims::NTuple{N, Any}) where {T, N} zeros(T, convert(Dims, dims))
@deprecate zeros(dims::Tuple) zeros(convert(Dims, dims))
@deprecate ones(::Type{T}, dims...) where {T} ones(T, convert(Dims, dims)...)
@deprecate ones(dims...) ones(convert(Dims, dims)...)
@deprecate ones(::Type{T}, dims::NTuple{N, Any}) where {T, N} ones(T, convert(Dims, dims))
@deprecate ones(dims::Tuple) ones(convert(Dims, dims))


@deprecate showcompact(x) show(IOContext(stdout, :compact => true), x)
@deprecate showcompact(io, x) show(IOContext(io, :compact => true), x)
@deprecate sprint(::typeof(showcompact), args...) sprint(show, args...; context=:compact => true)
Expand Down
5 changes: 2 additions & 3 deletions base/multidimensional.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1771,10 +1771,9 @@ unique(A::AbstractArray; dims::Union{Colon,Integer} = :) = _unique_dims(A, dims)
_unique_dims(A::AbstractArray, dims::Colon) = invoke(unique, Tuple{Any}, A)

@generated function _unique_dims(A::AbstractArray{T,N}, dim::Integer) where {T,N}
inds = inds -> zeros(UInt, inds)
quote
1 <= dim <= $N || return copy(A)
hashes = similar($inds, axes(A, dim))
hashes = zeros(UInt, axes(A, dim))

# Compute hash for each row
k = 0
Expand All @@ -1791,7 +1790,7 @@ _unique_dims(A::AbstractArray, dims::Colon) = invoke(unique, Tuple{Any}, A)
uniquerows = collect(values(firstrow))

# Check for collisions
collided = similar(falses, axes(A, dim))
collided = falses(axes(A, dim))
@inbounds begin
@nloops $N i A d->(if d == dim
k = i_d
Expand Down
8 changes: 4 additions & 4 deletions base/reducedim.jl
Original file line number Diff line number Diff line change
Expand Up @@ -746,10 +746,10 @@ function _findmin(A, region)
if prod(map(length, reduced_indices(A, region))) != 0
throw(ArgumentError("collection slices must be non-empty"))
end
(similar(A, ri), similar(dims->zeros(eltype(keys(A)), dims), ri))
(similar(A, ri), zeros(eltype(keys(A)), ri))
else
findminmax!(isless, fill!(similar(A, ri), first(A)),
similar(dims->zeros(eltype(keys(A)), dims), ri), A)
zeros(eltype(keys(A)), ri), A)
end
end

Expand Down Expand Up @@ -795,10 +795,10 @@ function _findmax(A, region)
if prod(map(length, reduced_indices(A, region))) != 0
throw(ArgumentError("collection slices must be non-empty"))
end
similar(A, ri), similar(dims->zeros(eltype(keys(A)), dims), ri)
similar(A, ri), zeros(eltype(keys(A)), ri)
else
findminmax!(isgreater, fill!(similar(A, ri), first(A)),
similar(dims->zeros(eltype(keys(A)), dims), ri), A)
zeros(eltype(keys(A)), ri), A)
end
end

Expand Down
34 changes: 17 additions & 17 deletions base/stat.jl
Original file line number Diff line number Diff line change
Expand Up @@ -282,23 +282,23 @@ operm(st::StatStruct) = UInt8((filemode(st) ) & 0x7)
# mode predicate methods for file names

for f in Symbol[
:ispath
:isfifo
:ischardev
:isdir
:isblockdev
:isfile
:issocket
:issetuid
:issetgid
:issticky
:uperm
:gperm
:operm
:filemode
:filesize
:mtime
:ctime
:ispath,
:isfifo,
:ischardev,
:isdir,
:isblockdev,
:isfile,
:issocket,
:issetuid,
:issetgid,
:issticky,
:uperm,
:gperm,
:operm,
:filemode,
:filesize,
:mtime,
:ctime,
]
@eval ($f)(path...) = ($f)(stat(path...))
end
Expand Down
4 changes: 4 additions & 0 deletions base/sysimg.jl
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ include("reinterpretarray.jl")
# type and dimensionality specified, accepting dims as series of Integers
Vector{T}(::UndefInitializer, m::Integer) where {T} = Vector{T}(undef, Int(m))
Matrix{T}(::UndefInitializer, m::Integer, n::Integer) where {T} = Matrix{T}(undef, Int(m), Int(n))
Array{T,N}(::UndefInitializer, d::Vararg{Integer,N}) where {T,N} = Array{T,N}(undef, convert(Tuple{Vararg{Int}}, d))
# type but not dimensionality specified, accepting dims as series of Integers
Array{T}(::UndefInitializer, m::Integer) where {T} = Array{T,1}(undef, Int(m))
Array{T}(::UndefInitializer, m::Integer, n::Integer) where {T} = Array{T,2}(undef, Int(m), Int(n))
Expand All @@ -163,6 +164,9 @@ Array{T}(::UndefInitializer, d::Integer...) where {T} = Array{T}(undef, convert(
# dimensionality but not type specified, accepting dims as series of Integers
Vector(::UndefInitializer, m::Integer) = Vector{Any}(undef, Int(m))
Matrix(::UndefInitializer, m::Integer, n::Integer) = Matrix{Any}(undef, Int(m), Int(n))
# Dimensions as a single tuple
Array{T}(::UndefInitializer, d::NTuple{N,Integer}) where {T,N} = Array{T,N}(undef, convert(Tuple{Vararg{Int}}, d))
Array{T,N}(::UndefInitializer, d::NTuple{N,Integer}) where {T,N} = Array{T,N}(undef, convert(Tuple{Vararg{Int}}, d))
# empty vector constructor
Vector() = Vector{Any}(undef, 0)

Expand Down
6 changes: 3 additions & 3 deletions stdlib/SparseArrays/src/sparsematrix.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1607,9 +1607,9 @@ end
# and computing reductions along columns into SparseMatrixCSC is
# non-trivial, so use Arrays for output
Base.reducedim_initarray(A::SparseMatrixCSC, region, v0, ::Type{R}) where {R} =
fill!(similar(dims->Array{R}(undef, dims), Base.reduced_indices(A,region)), v0)
fill(v0, Base.reduced_indices(A,region))
Base.reducedim_initarray0(A::SparseMatrixCSC, region, v0, ::Type{R}) where {R} =
fill!(similar(dims->Array{R}(undef, dims), Base.reduced_indices0(A,region)), v0)
fill(v0, Base.reduced_indices0(A,region))

# General mapreduce
function _mapreducezeros(f, op, ::Type{T}, nzeros::Int, v0) where T
Expand Down Expand Up @@ -1823,7 +1823,7 @@ function _findr(op, A, region, Tv)
throw(ArgumentError("array slices must be non-empty"))
else
ri = Base.reduced_indices0(A, region)
return (similar(A, ri), similar(dims->zeros(Ti, dims), ri))
return (similar(A, ri), zeros(Ti, ri))
end
end

Expand Down
18 changes: 15 additions & 3 deletions test/TestHelpers.jl
Original file line number Diff line number Diff line change
Expand Up @@ -95,14 +95,23 @@ function Base.similar(A::AbstractArray, T::Type, inds::Tuple{UnitRange,Vararg{Un
OffsetArray(B, map(indsoffset, inds))
end

Base.similar(f::Union{Function,Type}, shape::Tuple{UnitRange,Vararg{UnitRange}}) =
OffsetArray(f(map(length, shape)), map(indsoffset, shape))
Base.similar(::Type{T}, shape::Tuple{UnitRange,Vararg{UnitRange}}) where {T<:Array} =
OffsetArray(T(undef, map(length, shape)), map(indsoffset, shape))
Base.similar(::Type{T}, shape::Tuple{UnitRange,Vararg{UnitRange}}) where {T<:BitArray} =
OffsetArray(T(undef, map(length, shape)), map(indsoffset, shape))

Base.reshape(A::AbstractArray, inds::Tuple{UnitRange,Vararg{UnitRange}}) = OffsetArray(reshape(A, map(length, inds)), map(indsoffset, inds))
Base.reshape(A::AbstractArray, inds::Tuple{UnitRange,Vararg{UnitRange}}) = OffsetArray(reshape(A, map(indslength, inds)), map(indsoffset, inds))

Base.fill(v, inds::NTuple{N, Union{Integer, AbstractUnitRange}}) where {N} =
fill!(OffsetArray(Array{typeof(v), N}(undef, map(indslength, inds)), map(indsoffset, inds)), v)
Base.zeros(::Type{T}, inds::NTuple{N, Union{Integer, AbstractUnitRange}}) where {T, N} =
fill!(OffsetArray(Array{T, N}(undef, map(indslength, inds)), map(indsoffset, inds)), zero(T))
Base.ones(::Type{T}, inds::NTuple{N, Union{Integer, AbstractUnitRange}}) where {T, N} =
fill!(OffsetArray(Array{T, N}(undef, map(indslength, inds)), map(indsoffset, inds)), one(T))
Base.trues(inds::NTuple{N, Union{Integer, AbstractUnitRange}}) where {N} =
fill!(OffsetArray(BitArray{N}(undef, map(indslength, inds)), map(indsoffset, inds)), true)
Base.falses(inds::NTuple{N, Union{Integer, AbstractUnitRange}}) where {N} =
fill!(OffsetArray(BitArray{N}(undef, map(indslength, inds)), map(indsoffset, inds)), false)

@inline function Base.getindex(A::OffsetArray{T,N}, I::Vararg{Int,N}) where {T,N}
checkbounds(A, I...)
Expand Down Expand Up @@ -162,6 +171,9 @@ _offset(out, ::Tuple{}, ::Tuple{}) = out

indsoffset(r::AbstractRange) = first(r) - 1
indsoffset(i::Integer) = 0
indslength(r::AbstractRange) = length(r)
indslength(i::Integer) = i


Base.resize!(A::OffsetVector, nl::Integer) = (resize!(A.parent, nl); A)

Expand Down
2 changes: 1 addition & 1 deletion test/arrayops.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2322,7 +2322,7 @@ end
test_zeros(zeros(Int, (2, 3)), Matrix{Int}, (2,3))

# #19265"
@test_throws MethodError zeros(Float64, [1.])
@test_throws Any zeros(Float64, [1.]) # TODO: Tighten back up to MethodError once 0.7 deprecations are removed
end

# issue #11053
Expand Down
2 changes: 1 addition & 1 deletion test/bitarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ bitcheck(x) = true
function check_bitop_call(ret_type, func, args...; kwargs...)
r1 = func(args...; kwargs...)
r2 = func(map(x->(isa(x, BitArray) ? Array(x) : x), args)...; kwargs...)
ret_type nothing && !isa(r1, ret_type) && @show ret_type, r1
ret_type nothing && !isa(r1, ret_type) && @show ret_type, typeof(r1)
ret_type nothing && @test isa(r1, ret_type)
@test tc(r1, r2)
@test isequal(r1, ret_type nothing ? r2 : r2)
Expand Down
16 changes: 14 additions & 2 deletions test/compiler/compiler.jl
Original file line number Diff line number Diff line change
Expand Up @@ -514,10 +514,22 @@ end
@test h18679() === nothing


# issue #5575
f5575() = zeros(Type[Float64][1], 1)
# issue #5575: inference with abstract types on a reasonably complex method tree
zeros5575(::Type{T}, dims::Tuple{Vararg{Any,N}}) where {T,N} = Array{T,N}(dims)
zeros5575(dims::Tuple) = zeros5575(Float64, dims)
zeros5575(::Type{T}, dims...) where {T} = zeros5575(T, dims)
zeros5575(a::AbstractArray) = zeros5575(a, Float64)
zeros5575(a::AbstractArray, ::Type{T}) where {T} = zeros5575(a, T, size(a))
zeros5575(a::AbstractArray, ::Type{T}, dims::Tuple) where {T} = zeros5575(T, dims)
zeros5575(a::AbstractArray, ::Type{T}, dims...) where {T} = zeros5575(T, dims)
zeros5575(dims...) = zeros5575(dims)
f5575() = zeros5575(Type[Float64][1], 1)
@test Base.return_types(f5575, ())[1] == Vector

g5575() = zeros(Type[Float64][1], 1)
@test_broken Base.return_types(g5575, ())[1] == Vector # This should be fixed by removing deprecations


# make sure Tuple{unknown} handles the possibility that `unknown` is a Vararg
function maybe_vararg_tuple_1()
x = Any[Vararg{Int}][1]
Expand Down

0 comments on commit 2233ec4

Please sign in to comment.