TaylorSeries.jl icon indicating copy to clipboard operation
TaylorSeries.jl copied to clipboard

Solve most issues detected by Aqua.jl

Open lbenet opened this issue 3 years ago • 1 comments

This is a follow up of #298, where it was mentioned that some ambiguities were detected by Aqua.jl. This PR solves all ambiguities, but leaves some unbound type parameters (unbound_args) unsolved.

Using it, I get the following:

julia> using TaylorSeries

julia> using Aqua

julia> Aqua.test_all(TaylorSeries; unbound_args=false)
Skipping Base.active_repl
Skipping Base.active_repl_backend
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
Skipping Base.BinaryPlatforms.compiler_abi
Skipping Base.active_repl
Skipping Base.active_repl_backend
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
Skipping Base.BinaryPlatforms.compiler_abi
Test Summary:    | Pass  Total
Method ambiguity |    1      1
Test Summary:           |
Unbound type parameters | No tests
Test Summary:     | Pass  Total
Undefined exports |    1      1
Test Summary:                              | Pass  Total
Compare Project.toml and test/Project.toml |    1      1
Test Summary:      | Pass  Total
Stale dependencies |    1      1
Test Summary: | Pass  Total
Compat bounds |    1      1
Test Summary:           | Pass  Total
Project.toml formatting |    1      1
Test.DefaultTestSet("Project.toml formatting", Any[Test.DefaultTestSet("/Users/benet/.julia/dev/TaylorSeries/Project.toml", Any[], 1, false, false)], 0, false, false)

The failing test yields:

julia> Aqua.test_unbound_args(TaylorSeries)
Test Failed at /Users/benet/.julia/packages/Aqua/DaPBP/src/unbound_args.jl:10
  Expression: detect_unbound_args_recursively(m) == []
   Evaluated: Any[
(::TaylorSeries.var"#evaluate##kw")(::Any, ::typeof(evaluate), a::TaylorN, vals::Tuple{Vararg{T, N}}) where {N, T<:AbstractSeries} in TaylorSeries at /Users/benet/.julia/dev/TaylorSeries/src/evaluate.jl:218, 
var"#evaluate#44"(sorting::Bool, ::typeof(evaluate), a::TaylorN, vals::Tuple{Vararg{T, N}}) where {N, T<:AbstractSeries} in TaylorSeries at /Users/benet/.julia/dev/TaylorSeries/src/evaluate.jl:218, 
evaluate(a::TaylorN, vals::Tuple{Vararg{T, N}}; sorting) where {N, T<:AbstractSeries} in TaylorSeries at /Users/benet/.julia/dev/TaylorSeries/src/evaluate.jl:218] == Any[]

All reported cases are related to NTuple{N,T} with N left unspecified.

lbenet avatar Mar 09 '22 20:03 lbenet

Coverage Status

Coverage decreased (-0.7%) to 95.863% when pulling 40ff4ea7e44a823c8e818f662453a9da62a72492 on lb/Aqua_tests into 1461c66102cce2d995364e992b21ba8590af205c on master.

coveralls avatar Mar 09 '22 20:03 coveralls

@lbenet, the current release version still prints these warnings. Maybe you could make a new patch release.

schillic avatar Jan 27 '23 19:01 schillic

Thanks for reporting... I'll have a look on this and release a new patch version.

lbenet avatar Jan 28 '23 14:01 lbenet

So, I updated Aqua in my local system and rerun the tests. I got the same results as before. Regarding the unbound args, which I guess is the warning that appears, this is the output

julia> Aqua.test_unbound_args(TaylorSeries)
Test Failed at /Users/benet/.julia/packages/Aqua/utObL/src/unbound_args.jl:10
  Expression: detect_unbound_args_recursively(m) == []
   Evaluated: Any[(::TaylorSeries.var"#evaluate##kw")(::Any, ::typeof(evaluate), a::TaylorN, vals::Tuple{Vararg{T, N}}) where {N, T<:AbstractSeries} in TaylorSeries at /Users/benet/.julia/dev/TaylorSeries/src/evaluate.jl:218, var"#evaluate#44"(sorting::Bool, ::typeof(evaluate), a::TaylorN, vals::Tuple{Vararg{T, N}}) where {N, T<:AbstractSeries} in TaylorSeries at /Users/benet/.julia/dev/TaylorSeries/src/evaluate.jl:218, evaluate(a::TaylorN, vals::Tuple{Vararg{T, N}}; sorting) where {N, T<:AbstractSeries} in TaylorSeries at /Users/benet/.julia/dev/TaylorSeries/src/evaluate.jl:218] == Any[]
ERROR: There was an error during testing

As I understand it, the error is triggered (again) by the occurrence of {N, T<:AbstractSeries} in evaluate, with unbounded N, but I do not know how to bound N...

lbenet avatar Jan 28 '23 15:01 lbenet

I cannot explain why, but if you replace

https://github.com/JuliaDiff/TaylorSeries.jl/blob/16fe4790a3eeec0d3e6afca5753d0d06696e4eb6/src/evaluate.jl#L217-L218

by

evaluate(a::TaylorN, vals::NTuple{N,<:AbstractSeries}; sorting::Bool=false) where
    {N} = _evaluate(a, vals, Val(sorting))

it works for me.

schillic avatar Jan 28 '23 15:01 schillic

Finally implemented in #324.

lbenet avatar May 07 '23 18:05 lbenet