julia
julia copied to clipboard
Tighten signature of `repeat` to match what's documented
Accidentally passing a float here (due to forgetting that ceil doesn't return an integer) gives an error that's more confusing than it has to be:
julia> repeat(1:3, 4.0)
ERROR: MethodError: no method matching similar(::UnitRange{Int64}, ::Float64)
The function `similar` exists, but no method is defined for this combination of argument types.
Closest candidates are:
similar(::JuliaInterpreter.Compiled, ::Any)
@ JuliaInterpreter ~/.julia/packages/JuliaInterpreter/tYiNO/src/types.jl:7
similar(::AbstractArray, ::Type{T}) where T
@ Base abstractarray.jl:821
similar(::AbstractArray, ::Type{T}, ::NTuple{N, Int64}) where {T, N}
@ Base abstractarray.jl:833
...
Stacktrace:
[1] repeat_outer(a::UnitRange{Int64}, ::Tuple{Float64})
@ Base._RepeatInnerOuter ./abstractarraymath.jl:480
[2] repeat_inner_outer(arr::UnitRange{Int64}, ::Nothing, outer::Tuple{Float64})
@ Base._RepeatInnerOuter ./abstractarraymath.jl:460
[3] repeat(arr::UnitRange{Int64}; inner::Nothing, outer::Tuple{Float64})
@ Base._RepeatInnerOuter ./abstractarraymath.jl:402
[4] repeat(A::UnitRange{Int64}; inner::Nothing, outer::Tuple{Float64})
@ Base ./abstractarraymath.jl:394
[5] repeat(A::UnitRange{Int64}, counts::Float64)
@ Base ./abstractarraymath.jl:357
[6] top-level scope
@ REPL[48]:1
help?> repeat
search: repeat reset pad_repeat keepat! relpath replace read @deprecate realpath repr RecipeData
repeat(A::AbstractArray, counts::Integer...)
Construct an array by repeating array A a given number of times in each dimension, specified by
counts.
See also: fill, Iterators.repeated, Iterators.cycle.
After:
julia> repeat(1:3, 4.0)
ERROR: MethodError: no method matching repeat(::UnitRange{Int64}, ::Float64)
The function `repeat` exists, but no method is defined for this combination of argument types.
Closest candidates are:
repeat(::AbstractArray, ::Integer...)
@ Base REPL[4]:1
This seems to introduce method ambiguities, so perhaps these should be resolved across the ecosystem first
Error is this:
MethodError: repeat(::SparseArrays.SparseMatrixCSC{Float64, Int64}, ::Int64) is ambiguous.
Candidates:
repeat(A::SparseArrays.AbstractSparseMatrixCSC, m)
@ SparseArrays /cache/build/tester-amdci4-14/julialang/julia-master/julia-d4b18e6530/share/julia/stdlib/v1.12/SparseArrays/src/sparsematrix.jl:3974
repeat(A::AbstractArray, counts::Integer...)
@ Base abstractarraymath.jl:435
Possible fix, define
repeat(::SparseArrays.AbstractSparseMatrixCSC, ::Integer)
Unfortunately I imagine there will be many such errors from packages, too.
@nanosoldier runtests()
The package evaluation job you requested has completed - possible new issues were detected. The full report is available.
I see one real failure in Alicorn.jl and many packages that failed CI due to Aqua eagerly detecting a possible ambiguity but not actually hitting it.
Instead of fighting packages over ambiguities, 3ee4ffc changes it to an explicit check with an error. (There's already a function called check performing other sanity checks, such as negative repeats.) The example above becomes:
julia> repeat(1:3, 4.0)
ERROR: ArgumentError: repeat requires integer counts, got outer = (4.0,)
Stacktrace:
[1] check(arr::UnitRange{Int64}, inner::Nothing, outer::Tuple{Float64})
@ Base._RepeatInnerOuter ./REPL[30]:18
[2] repeat(arr::UnitRange{Int64}; inner::Nothing, outer::Tuple{Float64})
@ Base._RepeatInnerOuter ./abstractarraymath.jl:399
[3] repeat(A::UnitRange{Int64}, counts::Float64)
@ Base ./abstractarraymath.jl:356
[4] top-level scope
@ REPL[32]:1
I expect this to be clean
@nanosoldier runtests()
Going forward, folks can define specializations with the latter arguments typed as Integers which could be a path to turning this into a more appropriate (and simpler) method error.
@nanosoldier runtests()
Yes.
One advantage is that this also checks keywords inner/outer. Those can't easily have tight signatures as they accept almost anything:
julia> repeat([1 2 3]; inner=transpose(2:3))
2×9 Matrix{Int64}:
1 1 1 2 2 2 3 3 3
1 1 1 2 2 2 3 3 3
julia> repeat([1 2 3]; outer=(i^2 for i in 1:2))
1×12 Matrix{Int64}:
1 2 3 1 2 3 1 2 3 1 2 3
This needs rebase before merging
The package evaluation job you requested has completed - possible new issues were detected. The full report is available.
@nanosoldier runtests(["LinearRationalExpectations", "EarlyStopping", "PermutationGroups", "Groups", "SimpleWebsockets", "GraphViz", "GLPK", "RandomLinearAlgebraSolvers", "SliceSampling", "SteadyStateDiffEq", "ODEInterfaceDiffEq", "MCMCTempering", "MLJTestIntegration", "UltraDark", "MimiRFFSPs", "Population", "IterativeLearningControl"])
The package evaluation job you requested has completed - possible new issues were detected. The full report is available.