-
Notifications
You must be signed in to change notification settings - Fork 35
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
Fix method ambiguities #483
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #483 +/- ##
==========================================
- Coverage 94.25% 94.16% -0.10%
==========================================
Files 52 52
Lines 1375 1387 +12
==========================================
+ Hits 1296 1306 +10
- Misses 79 81 +2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
I guess it's even more questionable since actually it is not sufficient to fix the method ambiguities in the tests which load additional packages (see, e.g., https://github.com/JuliaGaussianProcesses/KernelFunctions.jl/actions/runs/3268189406/jobs/5374299901#step:6:222 and https://github.com/JuliaGaussianProcesses/KernelFunctions.jl/actions/runs/3268189406/jobs/5374301250#step:6:224): Evaluated: isempty(Tuple{Method, Method}[(map(f::Tf, A::SparseArrays.AbstractCompressedVector) where Tf @ SparseArrays.HigherOrderFns /opt/hostedtoolcache/julia/nightly/x64/share/julia/stdlib/v1.9/SparseArrays/src/higherorderfns.jl:152, map(t::Transform, x::AbstractVector) @ KernelFunctions ~/work/KernelFunctions.jl/KernelFunctions.jl/src/transform/transform.jl:11), (map(f, A::NamedDims.NamedDimsArray) @ NamedDims ~/.julia/packages/NamedDims/kCxfn/src/functions.jl:157, map(t::Transform, x::AbstractVector) @ KernelFunctions ~/work/KernelFunctions.jl/KernelFunctions.jl/src/transform/transform.jl:11), (map(f, a1::StaticArraysCore.StaticArray, as::AbstractArray...) @ StaticArrays ~/.julia/packages/StaticArrays/PUoe1/src/mapreduce.jl:30, map(t::Transform, x::AbstractVector) @ KernelFunctions ~/work/KernelFunctions.jl/KernelFunctions.jl/src/transform/transform.jl:11), (map(f, As::AxisArray{T, N, D, Ax}...) where {T, N, D, Ax<:Tuple{Vararg{Axis}}} @ AxisArrays ~/.julia/packages/AxisArrays/v7vf4/src/core.jl:431, map(t::Transform, x::AbstractVector) @ KernelFunctions ~/work/KernelFunctions.jl/KernelFunctions.jl/src/transform/transform.jl:11), (map(f, A::AxisArray) @ AxisArrays ~/.julia/packages/AxisArrays/v7vf4/src/core.jl:422, map(t::Transform, x::AbstractVector) @ KernelFunctions ~/work/KernelFunctions.jl/KernelFunctions.jl/src/transform/transform.jl:11), (map(f::Tf, A::Union{SparseArrays.AbstractCompressedVector, SparseArrays.AbstractSparseMatrixCSC}, Bs::Vararg{Union{SparseArrays.AbstractCompressedVector, SparseArrays.AbstractSparseMatrixCSC}, N}) where {Tf, N} @ SparseArrays.HigherOrderFns /opt/hostedtoolcache/julia/nightly/x64/share/julia/stdlib/v1.9/SparseArrays/src/higherorderfns.jl:156, map(t::Transform, x::AbstractVector) @ KernelFunctions ~/work/KernelFunctions.jl/KernelFunctions.jl/src/transform/transform.jl:11)]) |
So I am not sure if we can fix the |
@devmotion Can we get this merged anyway (we can leave some ambiguities) |
Lgtm when the tests are fixed |
@theogf I managed to fix all method ambiguities by completely removing the definition of |
Fixes #481.
I assume the only questionable change is that fixing the method ambiguity ofmap(t::Transform, ::AbstractVector)
, caused bymap(::Tf, ::SparseVector) where {Tf}
in SparseArrays, requires adding a dependency on SparseArrays.