fix dropstored for sparse matrices

[KristofferC]

Should fix #20513

[tkelman]

who's counting anyway

[KristofferC]

I did not understand your comment.

[StefanKarpinski]

Would it make sense for dropstored! to be a method of delete!?

[KristofferC]

Probably no. I would argue they have different semantics.

[StefanKarpinski]

Don't they both delete the value associated with keys, if there is one, reverting back to a default value, and returning the collection?

[Sacha0]

Good catch!

[Sacha0]

Thanks for fixing this! :)

[KristofferC]

@StefanKarpinski for example:

julia> d = Dict(1 => 1.0);

julia> delete!(d, 1);

julia> d[1]
ERROR: KeyError: key 1 not found
Stacktrace:
 [1] getindex(::Dict{Int64,Float64}, ::Int64) at ./dict.jl:474

julia> x = sparse([1]);

julia> Base.SparseArrays.dropstored!(x, 1);

julia> x[1]
0

[StefanKarpinski]

julia> using DataStructures

julia> d = DefaultDict(0.0, 1 => 1.0)
DataStructures.DefaultDict{Int64,Float64,Float64} with 1 entry:
  1 => 1.0

julia> delete!(d, 1)
DataStructures.DefaultDict{Int64,Float64,Float64} with 0 entries

julia> d[1]
0.0

[StefanKarpinski]

I.e. the difference is only because the standard Dict has no default value while sparse matrices and vectors do. The generic delete! operation covers both of these behaviors.

[Sacha0]

Would it make sense for dropstored! to be a method of delete!?

+1 for this suggestion. @KristofferC, have you other semantic differences in mind? Best!

[KristofferC]

Nah, I was wrong :)

[tkelman]

#20531 would make it not such a good idea though, as deleteat! resizes but dropstored! does not.

[StefanKarpinski]

That seems like a gotcha distinction that should be eliminated.

[tkelman]

they serve different purposes, and resizing deleteat! doesn't work for anything other than 1D

[StefanKarpinski]

Then it seems mostly like delete! and deleteat! should not be merged after all.

[tkelman]

Depends whether people expect delete! to resize or not. Is that only defined for Associatives right now? In that case resizing isn't all that meaningful. I could see these 3 functions being consolidated into 2 in a few combinations, but not into 1.

[StefanKarpinski]

The thinking behind deleteat! being separate from delete! is coming back to me and IIRC, it was precisely that delete! didn't make sense in an indexed collection because it has the effect of change all subsequent "keys" – i.e. indices. The delete! function, on the other hand only affects the key you ask to delete, which applies to sparse matrices/vectors and dicts, but not arrays.