deprecate fill!(A::[Diagonal|AbstractTriangular], x) = fillslots!(A, x) methods

[Sacha0]

This pull request deprecates two methods, fill!(A::S, x) = fillslots!(A, x) for S in (Diagonal, AbstractTriangular). As the comment attached to the associated definitions states, these methods are an artifact of history.

Should we consider deprecating fill!(A, x) for structured A as well? Unless x is zero or A is small enough that all entries are mutable (i.e. A is one-by-one or two-by-two depending on the special matrix type), fill!(A, x) errors.

Best!

[fredrikekre]

LoadError("sysimg.jl", 438, LoadError("deprecated.jl", 1884, UndefVarError(:AbstractTriangular)))

[Sacha0]

LoadError...

Should be fixed. Thanks! :) Thoughts re. structured fill!? Best!

[fredrikekre]

Looks like this testset

julia/test/linalg/bidiag.jl

Line 309 in ce1dc9c

@testset "fill! and fillslots!" begin

needs some work too.

Thoughts re. structured fill!? Best!

I agree that these methods are a bit weird. Although, it is convenient to zero out an array with fill! for generic code. Consider a function that will add elements to the diagonal. In theory this function can accept Matrix or Diagonal, since both of these allows for setindex! on the diagonal indices. But there is no common function that can "reset" the matrix by filling with 0. I have not had this problem with Diagonal and friends, but the same thing applies to Matrix vs Symmetric{Matrix} where I often have to add something like isa(A, Symmetric) ? fill!(A.data, 0) : fill!(A, 0) which is not very generic and kind of ugly. One option would be to have a fallback fillslots!(A, x) = fill!(A, x) so that fillslots! work on more containers, and the other option is to allow fill!(A, x) iff x == 0 for Diagonal, Symmetric etc.

[Sacha0]

The linalg/bidiag tests should be fixed. Thanks for isolating that failure! :)

I agree that these methods are a bit weird. Although, it is convenient to zero out an array with fill! for generic code.

Agreed, the convenience of fill!(A, 0) is my one hesitation.

One option would be to have a fallback fillslots!(A, x) = fill!(A, x) so that fillslots! work on more containers

Coincidentally I have work in progress extending fillslots! to additional types :). Best!

[fredrikekre]

Agreed, the convenience of fill!(A, 0) is my one hesitation.

Perhaps time to reconsider #19912?

[Sacha0]

Perhaps time to reconsider #19912?

I tend to agree that reducing rather than expanding such redundancy is preferable :).

[Sacha0]

Absent objections or requests for time, I plan to merge these changes tomorrow. Best!

[Sacha0]

Thanks all! :)