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

Some ambiguities found with Aqua #435

Open
andreasvarga opened this issue Jul 18, 2022 · 4 comments
Open

Some ambiguities found with Aqua #435

andreasvarga opened this issue Jul 18, 2022 · 4 comments

Comments

@andreasvarga
Copy link

I tried to check my packages with Aqua.jl (just for fun).

Performing the ambiguity test strictly for my package I obtained no ambiguities in my code

julia> Aqua.test_ambiguities(MatrixPencils,recursive=false)
Test Passed
  Expression: success(pipeline(cmd; stdout = stdout, stderr = stderr))

However, without the recursive = false setting, I obtained a lot of complaints, which apparently originate from Polynomials. So, I performed the test only for Polynomials and got

julia> Aqua.test_ambiguities(Polynomials,recursive=false)
Skipping Polynomials.order
12 ambiguities found
Ambiguity #1
/(p::P, c::S) where {S, T, X, P<:Polynomials.FactoredPolynomial{T, X}} in Polynomials at C:\Users\Andreas\.julia\packages\Polynomials\3kKqS\src\polynomials\factored_polynomial.jl:276
/(p::Polynomials.AbstractPolynomial, q::PQ) where PQ<:Polynomials.AbstractRationalFunction in Polynomials at C:\Users\Andreas\.julia\packages\Polynomials\3kKqS\src\rational-functions\common.jl:354

Possible fix, define
  /(::P, ::PQ) where {PQ<:Polynomials.AbstractRationalFunction, T, X, P<:Polynomials.FactoredPolynomial{T, X}}

Ambiguity #2
+(p::P, c::T) where {T, X, P<:Polynomials.ChebyshevT{T, X}} in Polynomials at C:\Users\Andreas\.julia\packages\Polynomials\3kKqS\src\polynomials\ChebyshevT.jl:225
+(p::Polynomials.AbstractPolynomial, q::Polynomials.AbstractRationalFunction) in Polynomials at C:\Users\Andreas\.julia\packages\Polynomials\3kKqS\src\rational-functions\common.jl:299

Possible fix, define
  +(::P, ::T) where {T<:Polynomials.AbstractRationalFunction, X, P<:Polynomials.ChebyshevT{T, X}}

Ambiguity #3
+(x, ::MutableArithmetics.Zero) in MutableArithmetics at C:\Users\Andreas\.julia\packages\MutableArithmetics\Lnlkl\src\rewrite.jl:62    
+(p::P, c::S) where {T, X, P<:Polynomials.AbstractPolynomial{T, X}, S} in Polynomials at C:\Users\Andreas\.julia\packages\Polynomials\3kKqS\src\common.jl:848

Possible fix, define
  +(::P, ::MutableArithmetics.Zero) where {T, X, P<:Polynomials.AbstractPolynomial{T, X}}

Ambiguity #4
+(p::P, c::T) where {T, P<:(Polynomials.StandardBasisPolynomial{T})} in Polynomials at C:\Users\Andreas\.julia\packages\Polynomials\3kKqS\src\polynomials\standard-basis.jl:75
+(p::Polynomials.AbstractPolynomial, q::Polynomials.AbstractRationalFunction) in Polynomials at C:\Users\Andreas\.julia\packages\Polynomials\3kKqS\src\rational-functions\common.jl:299

Possible fix, define
  +(::P, ::T) where {T<:Polynomials.AbstractRationalFunction, P<:(Polynomials.StandardBasisPolynomial{T})}

Ambiguity #5
isapprox(p1::Polynomials.AbstractPolynomial{T}, n::S; rtol, atol) where {T, S} in Polynomials at C:\Users\Andreas\.julia\packages\Polynomials\3kKqS\src\common.jl:1078
isapprox(::Any, ::Missing; kwargs...) in Base at missing.jl:91

Possible fix, define
  isapprox(::Polynomials.AbstractPolynomial{T}, ::Missing) where T

Ambiguity #6
scalar_mult(p::P, c::S) where {S, T, X, P<:Polynomials.AbstractPolynomial{T, X}} in Polynomials at C:\Users\Andreas\.julia\packages\Polynomials\3kKqS\src\common.jl:935
scalar_mult(c::S, p::P) where {S, T, X, P<:Polynomials.AbstractPolynomial{T, X}} in Polynomials at C:\Users\Andreas\.julia\packages\Polynomials\3kKqS\src\common.jl:941

Possible fix, define
  scalar_mult(::S, ::Union{P, S}) where {S<:(Polynomials.AbstractPolynomial), T, S<:(Polynomials.AbstractPolynomial{T}), X, P<:Polynomials.AbstractPolynomial{T, X}}

Ambiguity #7
convert(P::Type{<:Polynomials.ArnoldiFit}, p::Polynomials.ArnoldiFit{var"#64#T", var"#65#X"}) where {var"#64#T", var"#65#X"} in Polynomials at C:\Users\Andreas\.julia\packages\Polynomials\3kKqS\src\abstract.jl:84
convert(::Type{P}, p::Polynomials.ArnoldiFit) where P<:Polynomials.AbstractPolynomial in Polynomials at C:\Users\Andreas\.julia\packages\Polynomials\3kKqS\src\polynomials\standard-basis.jl:647

Possible fix, define
  convert(::Type{P}, ::Polynomials.ArnoldiFit{var"#64#T", var"#65#X"}) where {P<:Polynomials.ArnoldiFit, var"#64#T", var"#65#X"<:AbstractMatrix{var"#64#T"}}

Ambiguity #8
(::Base.var"#isapprox##kw")(::Any, ::typeof(isapprox), n::S, p1::Polynomials.AbstractPolynomial{T}) where {T, S} in Polynomials at C:\Users\Andreas\.julia\packages\Polynomials\3kKqS\src\common.jl:1088
(::Base.var"#isapprox##kw")(::Any, ::typeof(isapprox), ::Missing, ::Any) in Base at missing.jl:90

Possible fix, define
  (::Base.var"#isapprox##kw")(::Any, ::typeof(isapprox), ::Missing, ::Polynomials.AbstractPolynomial{T}) where T

Ambiguity #9
convert(::Type{P}, q::Polynomials.AbstractPolynomial) where P<:Polynomials.LaurentPolynomial in Polynomials at C:\Users\Andreas\.julia\packages\Polynomials\3kKqS\src\polynomials\LaurentPolynomial.jl:154
convert(::Type{P}, p::Polynomials.ArnoldiFit) where P<:Polynomials.AbstractPolynomial in Polynomials at C:\Users\Andreas\.julia\packages\Polynomials\3kKqS\src\polynomials\standard-basis.jl:647

Possible fix, define
  convert(::Type{P}, ::Polynomials.ArnoldiFit) where P<:Polynomials.LaurentPolynomial

Ambiguity #10
(::Base.var"#isapprox##kw")(::Any, ::typeof(isapprox), p1::Polynomials.AbstractPolynomial{T}, n::S) where {T, S} in Polynomials at C:\Users\Andreas\.julia\packages\Polynomials\3kKqS\src\common.jl:1078
(::Base.var"#isapprox##kw")(::Any, ::typeof(isapprox), ::Any, ::Missing) in Base at missing.jl:91

Possible fix, define
  (::Base.var"#isapprox##kw")(::Any, ::typeof(isapprox), ::Polynomials.AbstractPolynomial{T}, ::Missing) where T

Ambiguity #11
/(p::Polynomials.ImmutablePolynomial{T, X, N}, c::S) where {T, X, N, S} in Polynomials at C:\Users\Andreas\.julia\packages\Polynomials\3kKqS\src\polynomials\ImmutablePolynomial.jl:206
/(p::Polynomials.AbstractPolynomial, q::PQ) where PQ<:Polynomials.AbstractRationalFunction in Polynomials at C:\Users\Andreas\.julia\packages\Polynomials\3kKqS\src\rational-functions\common.jl:354

Possible fix, define
  /(::Polynomials.ImmutablePolynomial{T, X, N}, ::PQ) where {PQ<:Polynomials.AbstractRationalFunction, T, X, N}

Ambiguity #12
isapprox(n::S, p1::Polynomials.AbstractPolynomial{T}; rtol, atol) where {T, S} in Polynomials at C:\Users\Andreas\.julia\packages\Polynomials\3kKqS\src\common.jl:1088
isapprox(::Missing, ::Any; kwargs...) in Base at missing.jl:90

Possible fix, define
  isapprox(::Missing, ::Polynomials.AbstractPolynomial{T}) where T

Test Failed at C:\Users\Andreas\.julia\packages\Aqua\HWLbM\src\ambiguities.jl:117
  Expression: success(pipeline(cmd; stdout = stdout, stderr = stderr))
ERROR: There was an error during testing

I wonder if this is of any relevance for this package.

@jverzani
Copy link
Member

Thanks for this! In #403 many method invalidations were pointed out and that led to some improved package load times. Hopefully pursing this will as well.

@jverzani
Copy link
Member

Closed with #436
Thanks again!

@nsajko
Copy link
Contributor

nsajko commented Apr 15, 2023

Since this is still open:

  | | |_| | | | (_| |  |  Version 1.10.0-DEV.982 (2023-04-10)
 _/ |\__'_|_|_|\__'_|  |  Commit ea72b942792 (4 days old master)
|__/                   |

julia> using Test, MutableArithmetics, Polynomials
WARNING: using MutableArithmetics.Test in module Main conflicts with an existing identifier.

julia> detect_ambiguities(Polynomials, recursive = true)
1-element Vector{Tuple{Method, Method}}:
 (+(x, ::Zero) @ MutableArithmetics ~/.julia/packages/MutableArithmetics/geMUn/src/rewrite.jl:57, +(p::P, c::S) where {T, X, P<:AbstractPolynomial{T, X}, S} @ Polynomials ~/.julia/packages/Polynomials/Fh8md/src/common.jl:948)

I'm not exactly sure whose bug this is (MA or Polynomials), but AFAIK ambiguities sometimes require coordination between multiple packages, so I thought it's best to report this.

@jverzani
Copy link
Member

Thanks. I wasn't sure how to manage this, thinking it lie with the other package. But I'll have alook to see.

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

3 participants