Julia: Converting a vector to `Diagonal` shouldn't work

[dlfivefifty]

This new behaviour seems wrong:

julia> convert(Diagonal, rand(5))
5×5 Diagonal{Float64,Array{Float64,1}}:
 0.731736   ⋅         ⋅         ⋅         ⋅      
  ⋅        0.501643   ⋅         ⋅         ⋅      
  ⋅         ⋅        0.520562   ⋅         ⋅      
  ⋅         ⋅         ⋅        0.390789   ⋅      
  ⋅         ⋅         ⋅         ⋅        0.237735

I think the old behaviour of only supporting Diagonal(rand(5)) was correct.

[fredrikekre]

I guess
https://github.com/JuliaLang/julia/blob/f4d82b2bb16704106d1d4ec4e9bc6cc08a50ebb9/base/abstractarray.jl#L15
should be restricted w.r.t the dimension of the array? Something like

diff --git a/base/abstractarray.jl b/base/abstractarray.jl
index ff81756..1077add 100644
--- a/base/abstractarray.jl
+++ b/base/abstractarray.jl
@@ -12,7 +12,7 @@ Supertype for `N`-dimensional arrays (or array-like types) with elements of type
 AbstractArray

 convert(::Type{T}, a::T) where {T<:AbstractArray} = a
-convert(::Type{T}, a::AbstractArray) where {T<:AbstractArray} = T(a)
+convert(::Type{T}, a::AbstractArray{<:Any,N}) where {N,T<:AbstractArray{<:Any,N}} = T(a)

 if nameof(@__MODULE__) === :Base  # avoid method overwrite
 # catch undefined constructors before the deprecation kicks in

?

[dlfivefifty]

I think that line should probably be removed completely, by the same logic that it was removed for general types.

[mbauman]

This is a problem. We've been slowly moving to a world that supports T(a) where T <: AbstractArray and a isa AbstractArray to construct a new instance of T with the contents of a. LinearAlgebra's structured matrices do something totally different.

[Sacha0]

Some discussion in #24595 :).

[dlfivefifty]

I was involved in that discussion and I don't think there's another example in Base as egregiously confusing as Diagonal([1,2,3]).

One solution (which I used in BandedMatrices.jl) is to add an underscore _Diagonal([1,2,3]), but this should only be an internal solution.

This could be combined with an exported (and un-deprecated) diagm, so that diagm([1,2,3]) returns a Diagonal matrix.

[JeffBezanson]

We should narrow the convert definition only to cases that do the right thing (i.e. the input and output are sufficiently similar).

[mbauman]

And we shouldn't require 1-argument constructors for AbstractArrays to construct a new array based upon the iteration of the argument filling all cartesian values of the array.