add missing test for ones and zeros

[johnnychen94]

I'm actually a little surprised that eltype(ones(T)) === typeof(one(T)) not hold true for abstract types (e.g., AbstractFloat) given that one(T) is allowed, so here's one small patch that fixes this.

However, it's also a conflict that eltype(ones(T)) != T... so feedbacks are needed.

cc: @kimikage

---

Edit:

Actually I'm not convinced about this change at all, with two 👎 here I think it's sufficient to just reverted the changes and simply adds some test cases for zeros and ones.

[kimikage]

Create an Array, with element type T

https://docs.julialang.org/en/v1/base/arrays/#Base.ones
https://docs.julialang.org/en/v1/base/arrays/#Base.zeros

You should show the current problem and the benefits of this PR.

[johnnychen94]

My only weakly defense is that abstract type should be treated differently given that it's the same case for one and zero

It's likely to hit a performance issue there (although only once)

julia> X1 = ones(AbstractFloat, 1000, 1000); # Array{AbstractFloat, 2}

julia> X2 = ones(Float64, 1000, 1000);

julia> Y = randn(Float64, 1000, 1000);

julia> @benchmark X1 .* Y
BenchmarkTools.Trial:
  memory estimate:  22.89 MiB
  allocs estimate:  1000011
  --------------
  minimum time:     47.891 ms (0.00% GC)
  median time:      50.460 ms (0.00% GC)
  mean time:        50.898 ms (1.31% GC)
  maximum time:     58.578 ms (2.76% GC)
  --------------
  samples:          99
  evals/sample:     1

julia> @benchmark X2 .* Y
BenchmarkTools.Trial:
  memory estimate:  7.63 MiB
  allocs estimate:  4
  --------------
  minimum time:     1.157 ms (0.00% GC)
  median time:      1.488 ms (0.00% GC)
  mean time:        2.728 ms (45.41% GC)
  maximum time:     22.673 ms (92.12% GC)
  --------------
  samples:          1830
  evals/sample:     1

[kimikage]

For example, if you need an array of Float64, why not specify Float64 as eltype T?
T is not always specified directly by the user, but may be determined by promotion mechanism and so on. I think that we should not deprive us of the decision.

[timholy]

When one(T) and oneunit(T) give different answers, Base will error and your version gives something that might surprise some people:

julia> struct Foo end

julia> Base.one(::Type{Foo}) = 1

julia> Base.oneunit(::Type{Foo}) = Foo()

julia> ones(Foo, 3, 3)
ERROR: MethodError: Cannot `convert` an object of type Int64 to an object of type Foo
Closest candidates are:
  convert(::Type{T}, ::T) where T at essentials.jl:168
Stacktrace:
 [1] fill!(::Array{Foo,2}, ::Int64) at ./array.jl:309
 [2] ones(::Type{Foo}, ::Tuple{Int64,Int64}) at ./array.jl:462
 [3] ones(::Type{Foo}, ::Int64, ::Int64) at ./array.jl:457
 [4] top-level scope at REPL[4]:1

julia> function jc_ones(::Type{T}, dims...) where T
           init = one(T)
           a = Array{typeof(init)}(undef, dims)
           fill!(a, init)
       end
jc_ones (generic function with 1 method)

julia> jc_ones(Foo, 3, 3)
3×3 Array{Int64,2}:
 1  1  1
 1  1  1
 1  1  1

Given the decision in #24444, having Base throw an error is probably useful, as it encourages people to specify what they actually want by calling fill directly.

Note, however, that packages like Unitful (incorrectly, IMO) specialize ones to call oneunit. It's cases like these that leave me skeptical about the decision on #24444.

[johnnychen94]

Close this as the proposing change doesn't have much practical usage.