SentinelArrays.jl icon indicating copy to clipboard operation
SentinelArrays.jl copied to clipboard
verified publisher icon JuliaData

Many method ambiguities

Open KeithWM opened this issue 2 years ago • 2 comments

As part of a drive to improve the code quality of a Julia package, I checked for method ambiguities using Aqua. I found 17 ambiguities in our package and its dependencies, of which 13 are from SentinelArrays. These can be found using

using SentinelArrays
import Aqua

Aqua.test_all(SentinelArrays)

Are you aware of this?

KeithWM avatar Mar 13 '23 14:03 KeithWM

I was not; if you can share what they are, or better yet, submit a PR to fix, I would welcome the contribution!

quinnj avatar Mar 13 '23 22:03 quinnj

See below for the full output.

As an example:

Ambiguity #9
==(a::SentinelArrays.ChainedVectorIndex, b::Integer) in SentinelArrays at /Users/keith/.julia/packages/SentinelArrays/BtxKB/src/chainedvector.jl:207
==(i::Integer, x::BigInt) in Base.GMP at gmp.jl:689

Possible fix, define
  ==(::SentinelArrays.ChainedVectorIndex, ::BigInt)

The actual error arrises when trying to compare a ChainedVectorIndex to a BigInt:

julia> ==(SentinelArrays.ChainedVectorIndex(1, 1, 1, 21), BigInt(21))
ERROR: MethodError: ==(::SentinelArrays.ChainedVectorIndex{Int64}, ::BigInt) is ambiguous. Candidates:
  ==(i::Integer, x::BigInt) in Base.GMP at gmp.jl:689
  ==(a::SentinelArrays.ChainedVectorIndex, b::Integer) in SentinelArrays at /Users/keith/.julia/packages/SentinelArrays/BtxKB/src/chainedvector.jl:207
Possible fix, define
  ==(::SentinelArrays.ChainedVectorIndex, ::BigInt)
Stacktrace:
 [1] top-level scope
   @ REPL[34]:1

There is a suggested fix, to implement this (and other) methods also for a BigInt explicitly. I guess that has to then choose whether to revert to "our" definitition ==(a::SentinelArrays.ChainedVectorIndex, b::Integer) or "their" ==(i::Integer, x::BigInt).

I suppose some of this can be solved with an extension to the meta-programming. Let me give it a go sometime this week or next to set up a branch with a fix and make a PR.

julia> Aqua.test_all(SentinelArrays)
Skipping Base.active_repl
Skipping Base.active_repl_backend
Skipping Base.Sys.physical_free_memory
Skipping Base.Sys.physical_total_memory
Skipping Base.Filesystem.JL_O_RANDOM
Skipping Base.Filesystem.JL_O_SEQUENTIAL
Skipping Base.Filesystem.JL_O_SHORT_LIVED
Skipping Base.Filesystem.JL_O_TEMPORARY
13 ambiguities found
Ambiguity #1
<=(a::SentinelArrays.ChainedVectorIndex, b::Integer) in SentinelArrays at /Users/keith/.julia/packages/SentinelArrays/BtxKB/src/chainedvector.jl:207
<=(i::Integer, x::BigInt) in Base.GMP at gmp.jl:697

Possible fix, define
  <=(::SentinelArrays.ChainedVectorIndex, ::BigInt)

Ambiguity #2
copyto!(A::SparseArrays.SparseVector, B::AbstractVector) in SparseArrays at /Applications/Julia-1.8.app/Contents/Resources/julia/share/julia/stdlib/v1.8/SparseArrays/src/sparsevector.jl:502
copyto!(dest::AbstractVector, src::SentinelArrays.ChainedVector) in SentinelArrays at /Users/keith/.julia/packages/SentinelArrays/BtxKB/src/chainedvector.jl:381

Possible fix, define
  copyto!(::SparseArrays.SparseVector, ::SentinelArrays.ChainedVector)

Ambiguity #3
==(a::Integer, b::SentinelArrays.ChainedVectorIndex) in SentinelArrays at /Users/keith/.julia/packages/SentinelArrays/BtxKB/src/chainedvector.jl:208
==(x::BigInt, i::Integer) in Base.GMP at gmp.jl:688

Possible fix, define
  ==(::BigInt, ::SentinelArrays.ChainedVectorIndex)

Ambiguity #4
broadcasted(f::F, A::SentinelArrays.ChainedVector) where F in SentinelArrays at /Users/keith/.julia/packages/SentinelArrays/BtxKB/src/chainedvector.jl:929
broadcasted(::S, f, args...) where S<:Base.Broadcast.BroadcastStyle in Base.Broadcast at broadcast.jl:1306

Possible fix, define
  broadcasted(::S, ::SentinelArrays.ChainedVector) where S<:Base.Broadcast.BroadcastStyle

Ambiguity #5
reduce(op::OP, x::SentinelArrays.ChainedVector) where OP in SentinelArrays at /Users/keith/.julia/packages/SentinelArrays/BtxKB/src/chainedvector.jl:778
reduce(::typeof(vcat), A::AbstractVector{<:AbstractVecOrMat}) in Base at abstractarray.jl:1650

Possible fix, define
  reduce(::typeof(vcat), ::SentinelArrays.ChainedVector{T, A} where {T<:(AbstractVecOrMat), A<:AbstractVector{T}})

Ambiguity #6
reduce(op::OP, x::SentinelArrays.ChainedVector) where OP in SentinelArrays at /Users/keith/.julia/packages/SentinelArrays/BtxKB/src/chainedvector.jl:778
reduce(::typeof(hcat), A::AbstractVector{<:AbstractVecOrMat}) in Base at abstractarray.jl:1653

Possible fix, define
  reduce(::typeof(hcat), ::SentinelArrays.ChainedVector{T, A} where {T<:(AbstractVecOrMat), A<:AbstractVector{T}})

Ambiguity #7
<=(a::Integer, b::SentinelArrays.ChainedVectorIndex) in SentinelArrays at /Users/keith/.julia/packages/SentinelArrays/BtxKB/src/chainedvector.jl:208
<=(x::BigInt, i::Integer) in Base.GMP at gmp.jl:696

Possible fix, define
  <=(::BigInt, ::SentinelArrays.ChainedVectorIndex)

Ambiguity #8
findall(f::Function, x::SentinelArrays.ChainedVector) in SentinelArrays at /Users/keith/.julia/packages/SentinelArrays/BtxKB/src/chainedvector.jl:902
findall(pred::Base.Fix2{typeof(in)}, x::AbstractArray) in Base at array.jl:2481

Possible fix, define
  findall(::Base.Fix2{typeof(in)}, ::SentinelArrays.ChainedVector)

Ambiguity #9
==(a::SentinelArrays.ChainedVectorIndex, b::Integer) in SentinelArrays at /Users/keith/.julia/packages/SentinelArrays/BtxKB/src/chainedvector.jl:207
==(i::Integer, x::BigInt) in Base.GMP at gmp.jl:689

Possible fix, define
  ==(::SentinelArrays.ChainedVectorIndex, ::BigInt)

Ambiguity #10
<(a::SentinelArrays.ChainedVectorIndex, b::Integer) in SentinelArrays at /Users/keith/.julia/packages/SentinelArrays/BtxKB/src/chainedvector.jl:207
<(i::Integer, x::BigInt) in Base.GMP at gmp.jl:703

Possible fix, define
  <(::SentinelArrays.ChainedVectorIndex, ::BigInt)

Ambiguity #11
copyto!(dest::AbstractVector, src::SentinelArrays.ChainedVector) in SentinelArrays at /Users/keith/.julia/packages/SentinelArrays/BtxKB/src/chainedvector.jl:381
copyto!(dest::PermutedDimsArray{T, N}, src::AbstractArray{T, N}) where {T, N} in Base.PermutedDimsArrays at permuteddimsarray.jl:211

Possible fix, define
  copyto!(::PermutedDimsArray{T, 1}, ::SentinelArrays.ChainedVector{T, A} where A<:AbstractVector{T}) where T

Ambiguity #12
<(a::Integer, b::SentinelArrays.ChainedVectorIndex) in SentinelArrays at /Users/keith/.julia/packages/SentinelArrays/BtxKB/src/chainedvector.jl:208
<(x::BigInt, i::Integer) in Base.GMP at gmp.jl:702

Possible fix, define
  <(::BigInt, ::SentinelArrays.ChainedVectorIndex)

Ambiguity #13
copyto!(dest::AbstractVector, src::SentinelArrays.ChainedVector) in SentinelArrays at /Users/keith/.julia/packages/SentinelArrays/BtxKB/src/chainedvector.jl:381
copyto!(dest::PermutedDimsArray, src::AbstractArray) in Base.PermutedDimsArrays at permuteddimsarray.jl:215

Possible fix, define
  copyto!(::PermutedDimsArray{T, 1} where T, ::SentinelArrays.ChainedVector)

Method ambiguity: Test Failed at /Users/keith/.julia/packages/Aqua/utObL/src/ambiguities.jl:117
  Expression: success(pipeline(cmd; stdout = stdout, stderr = stderr))
Stacktrace:
 [1] macro expansion
   @ /Applications/Julia-1.8.app/Contents/Resources/julia/share/julia/stdlib/v1.8/Test/src/Test.jl:464 [inlined]
 [2] _test_ambiguities(packages::Vector{Base.PkgId}; color::Nothing, exclude::Vector{Any}, detect_ambiguities_options::Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
   @ Aqua ~/.julia/packages/Aqua/utObL/src/ambiguities.jl:117
Test Summary:    | Fail  Total  Time
Method ambiguity |    1      1  4.8s
ERROR: Some tests did not pass: 0 passed, 1 failed, 0 errored, 0 broken.

KeithWM avatar Mar 15 '23 08:03 KeithWM