fix #178 #343
fix #178 #343
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #343 +/- ##
=======================================
Coverage 99.91% 99.91%
=======================================
Files 8 8
Lines 1141 1143 +2
=======================================
+ Hits 1140 1142 +2
Misses 1 1 ☔ View full report in Codecov by Sentry. |
|
apparently Julia 1.6 doesn't have package extensions |
|
This should be included directly on Julia 1.6 |
|
is it possible to have dependency that depends on julia version? |
|
This should work already, see FillArrays.jl/src/FillArrays.jl Lines 673 to 677 in 1788108 The test failure on v1.6 appears to be because SparseArrays on that version limits dot to arrays of numbers, which leads to an ambiguity.
|
|
@jishnub do we test for Ambiguities in |
| if VERSION >= v"1.8" | ||
| dot(x::AbstractFillVector, y::SparseVectorUnion) = _fill_dot(x, y) | ||
| else | ||
| dot(x::AbstractFillVector{<:Number}, y::SparseVectorUnion{<:Number}) = _fill_dot(x, y) |
There was a problem hiding this comment.
It should be covered by the latest update.
https://app.codecov.io/gh/JuliaArrays/FillArrays.jl/pull/343/blob/ext/FillArraysSparseArraysExt.jl
| @@ -1,10 +1,11 @@ | |||
| module FillArraysSparseArraysExt | |||
|
|
|||
| using SparseArrays | |||
| using SparseArrays: SparseVectorUnion | |||
There was a problem hiding this comment.
This seems like an internal type, so I wonder if it'll be better to hard-code the exact types that we want to disambiguate against? Perhaps we may just copy the definition over from SparseArrays.
There was a problem hiding this comment.
This is from one of the ambiguity source https://github.com/JuliaSparse/SparseArrays.jl/blob/a5e95ec23649bd8cb75d2923861c74061016d1dd/src/sparsevector.jl#L1760
so if it changes someday, the ambiguity should also change.
There was a problem hiding this comment.
Perhaps we should define specific methods for the individual elements from the Union, which would not change even if this method signature is altered by SparseArrays, or if the type is removed.
There was a problem hiding this comment.
Seems like it's supposed to be external
https://github.com/JuliaSparse/SparseArrays.jl/blob/a5e95ec23649bd8cb75d2923861c74061016d1dd/src/sparsevector.jl#L92
# the following aliases are unused internally, but widespread in packages
There was a problem hiding this comment.
https://juliahub.com/ui/Search?q=SparseVectorUnion&type=code
SparseVectorUnion to be used in only two packages
There was a problem hiding this comment.
Can I copy the alias definitions over?
There was a problem hiding this comment.
I think that's a good idea, and let's define specific methods for each type instead of for the union (perhaps by calling an internal function _dot within each method). This is the safest.
No description provided.