Merge pull request #24413 from Sacha0/depoddfillbang

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

Author: Sacha0

NEWS.md

@@ -370,6 +370,9 @@ Deprecated or removed
     Instead, reshape the array or add trailing indices so the dimensionality and number of indices
     match ([#14770], [#23628]).
 
+  * `fill!(A::Diagonal, x)` and `fill!(A::AbstractTriangular, x)` have been deprecated
+    in favor of `Base.LinAlg.fillslots!(A, x)` ([#24413]).
+
   * Using Bool values directly as indices is now deprecated and will be an error in the future. Convert
     them to `Int` before indexing if you intend to access index `1` for `true` and `0` for `false`.
 

base/deprecated.jl

@@ -1881,6 +1881,10 @@ end
 # After deprecation is removed, enable the @testset "indexing by Bool values" in test/arrayops.jl
 # Also un-comment the new definition in base/indices.jl
 
+# deprecate odd fill! methods
+@deprecate fill!(D::Diagonal, x)                       fillslots!(D, x)
+@deprecate fill!(A::Base.LinAlg.AbstractTriangular, x) fillslots!(A, x)
+
 function diagm(v::BitVector)
     depwarn(string("diagm(v::BitVector) is deprecated, use diagm(0 => v) or ",
         "BitMatrix(Diagonal(v)) instead"), :diagm)

base/linalg/bidiag.jl

@@ -607,10 +607,6 @@ function fillslots!(A::SpecialArrays, x)
     return A
 end
 
-# for historical reasons:
-fill!(a::AbstractTriangular, x) = fillslots!(a, x)
-fill!(D::Diagonal, x) = fillslots!(D, x)
-
 _small_enough(A::Bidiagonal) = size(A, 1) <= 1
 _small_enough(A::Tridiagonal) = size(A, 1) <= 2
 _small_enough(A::SymTridiagonal) = size(A, 1) <= 2

base/linalg/diagonal.jl

@@ -61,6 +61,11 @@ convert(::Type{Array}, D::Diagonal) = convert(Matrix, D)
 similar(D::Diagonal, ::Type{T}) where {T} = Diagonal(similar(D.diag, T))
 similar(D::Diagonal, ::Type{T}, dims::Union{Dims{1},Dims{2}}) where {T} = spzeros(T, dims...)
 
+Base.zeros(D::Diagonal) = Diagonal(fill!(similar(D.diag), 0))
+Base.zeros(D::Diagonal, ::Type{T}) where {T} = Diagonal(fill!(similar(D, T), 0))
+Base.zeros(D::Diagonal, ::Type{T}, dims::Dims) where {T} = fill!(similar(D, T, dims), 0)
+Base.zeros(D::Diagonal, ::Type{T}, dims::Integer...) where {T} = fill!(similar(D, T, dims), 0)
+
 copy!(D1::Diagonal, D2::Diagonal) = (copy!(D1.diag, D2.diag); D1)
 
 size(D::Diagonal) = (length(D.diag),length(D.diag))

test/linalg/bidiag.jl

@@ -324,17 +324,25 @@ import Base.LinAlg: fillslots!, UnitLowerTriangular
             Bidiagonal(randn(3), randn(2), rand([:U,:L])),
             SymTridiagonal(randn(3), randn(2)),
             sparse(randn(3,4)),
-            Diagonal(randn(5)),
+            # Diagonal(randn(5)), # Diagonal fill! deprecated, see below
             sparse(rand(3)),
-            LowerTriangular(randn(3,3)),
-            UpperTriangular(randn(3,3))
+            # LowerTriangular(randn(3,3)), # AbstractTriangular fill! deprecated, see below
+            # UpperTriangular(randn(3,3)) # AbstractTriangular fill! deprecated, see below
             ]
             for A in exotic_arrays
                 fill!(A, 0)
                 for a in A
                     @test a == 0
                 end
             end
+            # Diagonal and AbstractTriangular fill! were defined as fillslots!,
+            # not matching the general behavior of fill!, and so have been deprecated.
+            # In a future dev cycle, these fill! methods should probably be reintroduced
+            # with behavior matching that of fill! for other structured matrix types.
+            # In the interm, equivalently test fillslots! below
+            @test iszero(fillslots!(Diagonal(fill(1, 3)), 0))
+            @test iszero(fillslots!(LowerTriangular(fill(1, 3, 3)), 0))
+            @test iszero(fillslots!(UpperTriangular(fill(1, 3, 3)), 0))
         end
         let # fill!(small, x)
             val = randn()

test/linalg/triangular.jl

@@ -34,9 +34,6 @@ for elty1 in (Float32, Float64, BigFloat, Complex64, Complex128, Complex{BigFloa
         # full!
         @test full!(copy(A1)) == A1
 
-        # fill!
-        @test full!(fill!(copy(A1), 1)) == t1(ones(size(A1)...))
-
         # similar
         @test isa(similar(A1), t1)
         @test eltype(similar(A1)) == elty1