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

Fix keytype and zero #574

Open dpinol opened this issue 1 year ago • 7 comments

Fixes https://github.com/JuliaSparse/SparseArrays.jl/issues/574

dpinol avatar Oct 25 '24 16:10 dpinol

keytype isn't the type of the stored indices though, it should match eltype(keys(...)):

julia> v = spzeros(Int, Int32, 10)
10-element SparseVector{Int64, Int32} with 0 stored entries

julia> eltype(keys(v))
Int64

fredrikekre avatar Oct 25 '24 16:10 fredrikekre

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 84.00%. Comparing base (485fd4b) to head (8c6a370).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #575      +/-   ##
==========================================
- Coverage   84.14%   84.00%   -0.15%     
==========================================
  Files          12       12              
  Lines        9160     9161       +1     
==========================================
- Hits         7708     7696      -12     
- Misses       1452     1465      +13     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Oct 25 '24 17:10 codecov[bot]

keytype isn't the type of the stored indices though, it should match eltype(keys(...)):

Should I also adapt keys then, for consistency with eachindex & length?

help?> keys
search: keys keytype KeyError haskey getkey UndefKeywordError WeakKeyDict

  keys(a::AbstractArray)
Return an efficient array describing all valid indices....

dpinol avatar Oct 25 '24 18:10 dpinol

No, I think keys is correct, and thus think that keytype should always be Int.

fredrikekre avatar Oct 25 '24 18:10 fredrikekre

No, I think keys is correct, and thus think that keytype should always be Int.

why?

dpinol avatar Oct 25 '24 18:10 dpinol

What should do further here?

ViralBShah avatar Jan 23 '25 11:01 ViralBShah

@fredrikekre Should this be closed?

ViralBShah avatar Feb 17 '25 22:02 ViralBShah