Skip to content

Commit 8cdc98a

Browse files
authored
disable make without explicit 1st-arg type, like make(2, 3) (#5)
Having the shortcut `make(2, 3)` for `make(Array, 2, 3)`, which mirrors `rand(2, 3)`, is convenient, but the form `make(Int, 2, 3)` leads to ambiguities for types that take 2 integer args, e.g. `make(MyType, 2, 3)`. Let's favor simplicity for custom-defined types. Moreover, the terminology "make" makes sense when the first arg refers to the type to be "made", e.g. in `make(Int, 2, 3)` we didn't make an `Int`, but an array, so the meaning of "make" was lost.
1 parent bee547b commit 8cdc98a

4 files changed

Lines changed: 19 additions & 40 deletions

File tree

README.md

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -141,10 +141,11 @@ julia> rand(Normal(), SparseMatrixCSC, 0.3, 2, 3) # equivalent to sprandn(2, 3,
141141
[2, 1] = 0.448981
142142
[1, 2] = 0.730103
143143

144-
# like for Array, sparse arrays enjoy to be special cased: `SparseVector` or `SparseMatrixCSC` can be omitted:
144+
# like for Array, sparse arrays enjoy to be special cased: `SparseVector` or `SparseMatrixCSC`
145+
# can be omitted in the `rand` call (not in the `make` call):
145146

146-
julia> rand(make(make(1:9, 0.3, 2, 3), 0.1, 4)) # possible, bug ugly output when non-empty :-/
147-
4-element SparseVector{SparseMatrixCSC{Int64,Int64},Int64} with 0 stored entries
147+
julia> rand(make(SparseVector, 1:9, 0.3, 2), 0.1, 4, 3) # possible, bug ugly output when non-empty :-/
148+
4×3 SparseMatrixCSC{SparseVector{Int64,Int64},Int64} with 0 stored entries
148149

149150
julia> rand(String, 4) # equivalent to randstring(4)
150151
"5o75"

src/containers.jl

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,10 @@ _make_cont(args...) = make(args...)
118118
@make_array_container(t::Type{<:Array})
119119
@make_array_container(t::Type{<:BitArray})
120120
@make_array_container(t::AbstractFloat)
121-
_make_cont(t::AbstractFloat, x, dims::Dims) = make(x, t, dims)
121+
_make_cont(t::AbstractFloat, x, dims::Dims{1}) = make(SparseVector, x, t, dims)
122+
_make_cont(t::AbstractFloat, dims::Dims{1}) = make(SparseVector, t, dims)
123+
_make_cont(t::AbstractFloat, x, dims::Dims{2}) = make(SparseMatrixCSC, x, t, dims)
124+
_make_cont(t::AbstractFloat, dims::Dims{2}) = make(SparseMatrixCSC, t, dims)
122125

123126
## sets/dicts
124127

src/sampling.jl

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -456,17 +456,6 @@ maketype(A::Type{Array{T,N}}, _, ::Dims{N}) where {T, N} = Array{T, N}
456456
maketype(A::Type{Array{T,N} where T}, X, ::Dims{N}) where {N} = Array{val_gentype(X), N}
457457
maketype(A::Type{Array}, X, ::Dims{N}) where {N} = Array{val_gentype(X), N}
458458

459-
# special shortcut
460-
461-
make(X, dims::Dims) = make(Array, X, dims)
462-
make(X, d1::Integer, dims::Integer...) = make(Array, X, Dims((d1, dims...)))
463-
make(::Type{X}, dims::Dims) where {X} = make(Array, X, dims)
464-
make(::Type{X}, d1::Integer, dims::Integer...) where {X} = make(Array, X, Dims((d1, dims...)))
465-
make( dims::Integer...) = make(Array, default_sampling(Array), Dims(dims))
466-
467-
# omitted: make(dims::Dims)
468-
# for the same reason that rand(dims::Dims) doesn't produce an array, i.e. it produces a scalar picked from the tuple
469-
470459
#### BitArray
471460

472461
default_sampling(::Type{<:BitArray}) = Uniform(Bool)
@@ -495,21 +484,6 @@ make(T::Type{SparseMatrixCSC}, p::AbstractFloat, d1::Integer, d2::Integer) = mak
495484
make(T::Type{SparseVector}, p::AbstractFloat, dims::Dims{1}) = make(T, default_sampling(T), p, dims)
496485
make(T::Type{SparseMatrixCSC}, p::AbstractFloat, dims::Dims{2}) = make(T, default_sampling(T), p, dims)
497486

498-
make(X, p::AbstractFloat, dims::Dims{1}) = make(SparseVector, X, p, dims)
499-
make(::Type{X}, p::AbstractFloat, dims::Dims{1}) where {X} = make(SparseVector, X, p, dims)
500-
make(X, p::AbstractFloat, dims::Dims{2}) = make(SparseMatrixCSC, X, p, dims)
501-
make(::Type{X}, p::AbstractFloat, dims::Dims{2}) where {X} = make(SparseMatrixCSC, X, p, dims)
502-
503-
make(X, p::AbstractFloat, d1::Integer) = make(X, p, Dims(d1))
504-
make(X, p::AbstractFloat, d1::Integer, d2::Integer) = make(X, p, Dims((d1, d2)))
505-
make(::Type{X}, p::AbstractFloat, d1::Integer) where {X} = make(X, p, Dims(d1))
506-
make(::Type{X}, p::AbstractFloat, d1::Integer, d2::Integer) where {X} = make(X, p, Dims((d1, d2)))
507-
make( p::AbstractFloat, dims::Dims) = make(default_sampling(AbstractArray), p, dims)
508-
make( p::AbstractFloat, d1::Integer) = make(default_sampling(AbstractArray), p, Dims(d1))
509-
make( p::AbstractFloat, d1::Integer, d2::Integer) = make(default_sampling(AbstractArray), p, Dims((d1, d2)))
510-
511-
# disambiguate (away from make(String, chars, n::Integer))
512-
make(::Type{String}, p::AbstractFloat, d1::Integer) = make(String, p, Dims(d1))
513487

514488
Sampler(RNG::Type{<:AbstractRNG}, c::Make3{A}, n::Repetition) where {A<:AbstractSparseArray} =
515489
SamplerTag{Cont{A}}((sp = sampler(RNG, c.x, n),

test/runtests.jl

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -498,20 +498,21 @@ end
498498

499499
@test rand(make(Array, spString, 9)) isa Array{String}
500500
@test rand(make(BitArray, Sampler(MersenneTwister, [0, 0, 0, 1]), 9)) isa BitArray
501-
501+
# TODO: below was testing without explicit `Array` as 1st argument, so this test
502+
# may now be obsolete, redundant with tests above
502503
for dims = ((), (2, 3), (0x2, 3), (2,), (0x2,)),
503504
s = ([], [Int], [1:3])
504505

505506
T = s == [] ? Float64 : Int
506507
if dims != ()
507-
a = rand(make(s..., dims...))
508+
a = rand(make(Array, s..., dims...))
508509
@test a isa Array{T,length(dims)}
509510
if s == [1:3]
510511
@test all(in(1:3), a)
511512
end
512513
end
513-
if s != [] && dims isa Dims
514-
a = rand(make(s..., dims))
514+
if dims isa Dims
515+
a = rand(make(Array, s..., dims))
515516
@test a isa Array{T,length(dims)}
516517
if s == [1:3]
517518
@test all(in(1:3), a)
@@ -526,17 +527,17 @@ end
526527
[(2,3)] => 2,
527528
[6] => 1,
528529
[2, 3] => 2,
529-
[Int8(2), Int16(3)] => 2),
530-
form = ([], [dim == 1 ? SparseVector : SparseMatrixCSC])
530+
[Int8(2), Int16(3)] => 2)
531+
532+
typ = dim == 1 ? SparseVector : SparseMatrixCSC
531533

532-
s = rand(make(form..., k..., 0.3, d...))
534+
s = rand(make(typ, k..., 0.3, d...))
533535
@test s isa (dim == 1 ? SparseVector{Float64,Int} :
534536
SparseMatrixCSC{Float64,Int})
535537
@test length(s) == 6
536538
end
537-
@test rand(make(spString, 0.3, 9)) isa SparseVector{String}
538-
@test rand(make(String, 0.9, 2)) isa SparseVector{String} # can be ambiguous with make(String, chars, n)
539-
@test rand(make(make(1:9, 0.3, 2, 3), .1, 4)) isa SparseVector{SparseMatrixCSC{Int64,Int64},Int64}
539+
@test rand(make(SparseVector, spString, 0.3, 9)) isa SparseVector{String}
540+
@test rand(make(SparseVector, make(SparseMatrixCSC, 1:9, 0.3, 2, 3), .1, 4)) isa SparseVector{SparseMatrixCSC{Int64,Int64},Int64}
540541
end
541542

542543
@testset "rand(make(default))" begin

0 commit comments

Comments
 (0)