The current argument order of the similar function violates the style guide on argument ordering:
The guideline suggests the type argument should come before the input argument not being mutated. However, methods like similar(array, type) (e.g. similar(zeros(10), Float32)) are defined in Base. I think we should flip the order of these arguments to obey the guideline.
I couldn't find any discussions on this topic but #18161 may be related.
Note that similar is somewhat special (though not unique) since the type argument actually gives the element type. similar(zeros(10), Float32) doesn't return a Float32. Also, given the name of the function, it would be weird to ask for a "similar" object but not pass this object as the first argument.
We define zeros(type, size), not zeros(size, type), in which the type argument also gives the element type. So, similar is not consistent with zeros and ones. Considering these functions are used in a similar way, I think we should make the order of arguments more consistent.
A difference with similar is that type is an _optional_ argument to override the element type of the array.
Perhaps it is more consistent to have optional arguments trailing:
A = Matrix{Int}(2,3)
B = similar(A)
C = similar(A, Float64)
rather than:
A = Matrix{Int}(2,3)
B = similar(A)
C = similar(Float64, A)
Since the "base case" for calling similar is just similar(array) you can think of the array as defining the type, element type and size
We define
zeros(type, size), notzeros(size, type), in which the type argument also gives the element type. So,similaris not consistent withzerosandones.
This is not a good comparison, in 0.6 we have methods of zeros and ones corresponding exactly to similar (e.g. you can call zeros(array), zeros(array, type) and zeros(array, type, size)) but those have now been deprecated, see #24656, so comparing the zeros(type, size) methods with similar is not correct.
In general, the similar interface needs an overhaul, but I don't think switching the arguments here is what similar needs.
Yes, the argument order is based on similar(array), which was expected to be the common case. Maybe keyword arguments would be better for the eltype and size?
Okay, I'm inclined to think using keyword arguments is a better choice than relying on argument order. So, this is related to #25395.
Should we do an ad-hoc API change to similar(array; eltype=eltype(array), size=size(array)), or wait for an overhaul discussed in #18161?
Maybe keyword arguments would be better for the eltype and size?
The return type wouldn't be inferred in that case, would it?
I think recent improvements of keyword argument handling makes it possible to infer the return type.
julia> function mysimilar(array::Array; eltype=eltype(array), size=size(array))
return Array{eltype}(uninitialized, size)
end
mysimilar (generic function with 1 method)
julia> @code_typed mysimilar(zeros(10))
CodeInfo(:(begin
# meta: location array.jl size 112
SSAValue(3) = (Base.arraysize)(array, 1)::Int64
# meta: pop location
# meta: location REPL[3] #mysimilar#2 2
# meta: location boot.jl Type 384
# meta: location boot.jl Type 376
# meta: location boot.jl Type 367
SSAValue(15) = $(Expr(:foreigncall, :(:jl_alloc_array_1d), Array{Float64,1}, svec(Any, Int64), :(:ccall), 2, Array{Float64,1}, SSAValue(3), SSAValue(3)))
# meta: pop locations (4)
return SSAValue(15)
end)) => Array{Float64,1}
Ah, this doesn't work:
julia> @code_typed mysimilar(zeros(10); eltype=Int)
CodeInfo(:(begin
SSAValue(0) = (Base.haskey)(#temp#@_2, :eltype)
unless SSAValue(0) goto 5
#temp#@_7 = (Base.getindex)(#temp#@_2, :eltype)
goto 7
5:
#temp#@_7 = Float64
7:
SSAValue(1) = #temp#@_7
SSAValue(2) = (Base.haskey)(#temp#@_2, :size)
unless SSAValue(2) goto 13
#temp#@_8 = (Base.getindex)(#temp#@_2, :size)
goto 19
13:
# meta: location array.jl size 112
SSAValue(11) = (Base.arraysize)(array, 1)::Int64
SSAValue(12) = (Core.tuple)(SSAValue(11))::Tuple{Int64}
# meta: pop location
#temp#@_8 = SSAValue(12)
19:
SSAValue(3) = #temp#@_8
SSAValue(6) = (Base.structdiff)(#temp#@_2, NamedTuple{(:eltype, :size),T} where T<:Tuple)
SSAValue(7) = (Base.pairs)(SSAValue(6))
SSAValue(8) = (Base.isempty)(SSAValue(7))
unless SSAValue(8) goto 26
goto 29
26:
SSAValue(9) = (Base.pairs)(#temp#@_2)
(Base.kwerr)(SSAValue(9), , array)::Union{}
29:
# meta: location REPL[3] #mysimilar#2 2
SSAValue(13) = (Core.apply_type)(Main.Array, SSAValue(1))::Type{Array{_1,N} where N} where _1
SSAValue(14) = (SSAValue(13))(Main.uninitialized, SSAValue(3))
# meta: pop location
return SSAValue(14)
end)) => Any
I don't think the @code_* accurately reflect how kw calls behave within other functions. It'll typically be better than that, but still not good enough:
julia> f() = mysimilar(zeros(10); eltype=Int);
julia> @code_typed f()
CodeInfo(:(begin
…
end)) => Array{_1,1} where _1
My thought on #18161 is that it'll require an entirely new name in any case, so we don't need to stress too much about getting this signature backwards-compatible with a new design.
Most helpful comment
Since the "base case" for calling
similaris justsimilar(array)you can think of thearrayas defining the type, element type and sizeThis is not a good comparison, in 0.6 we have methods of
zerosandonescorresponding exactly tosimilar(e.g. you can callzeros(array),zeros(array, type)andzeros(array, type, size)) but those have now been deprecated, see #24656, so comparing thezeros(type, size)methods with similar is not correct.In general, the
similarinterface needs an overhaul, but I don't think switching the arguments here is whatsimilarneeds.