Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Many method ambiguities #85

Open
KeithWM opened this issue Mar 13, 2023 · 2 comments
Open

Many method ambiguities #85

KeithWM opened this issue Mar 13, 2023 · 2 comments

Comments

@KeithWM
Copy link

KeithWM commented Mar 13, 2023

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?

@quinnj
Copy link
Member

quinnj commented Mar 13, 2023

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

@KeithWM
Copy link
Author

KeithWM commented Mar 15, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants