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,similar
is not consistent withzeros
andones
.
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
similar
is justsimilar(array)
you can think of thearray
as defining the type, element type and sizeThis is not a good comparison, in 0.6 we have methods of
zeros
andones
corresponding 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
similar
interface needs an overhaul, but I don't think switching the arguments here is whatsimilar
needs.