Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

reduce(vcat and and reduce(hcat, are not consistent in the return type for a single element collection, except if that collection is an array #31169

Open
oxinabox opened this issue Feb 25, 2019 · 1 comment
Labels
arrays [a, r, r, a, y, s] collections Data structures holding multiple items, e.g. sets fold sum, maximum, reduce, foldl, etc.

Comments

@oxinabox
Copy link
Contributor

The reduce(vcat and hcat are a bit funny about the types they return when you reduce over a single element

2-element Array{Int64,1}:
 1
 2

julia> reduce(vcat, (1:2,))
1:2

julia> reduce(vcat, [1:2,0:-1])
2-element Array{Int64,1}:
 1
 2

julia> reduce(vcat, (1:2,0:-1))
2-element Array{Int64,1}:
 1
 2

another example

julia> reduce(hcat, ([1,2] for x in 1:1))
2-element Array{Int64,1}:
 1
 2

julia> reduce(hcat, [[1,2]])
2×1 Array{Int64,2}:
 1
 2

julia> reduce(hcat, ([1,2] for x in 1:2))
2×2 Array{Int64,2}:
 1  1
 2  2

So this issue is two fold

  1. Reducing over 1 element vs multiple elements can return a different type
  2. The above is not true for if the collection is a Vector. Return type is always consistent for vectors even if they only have 1 element.

I have this spare enhancement to the tests for these functions,
that I was writing for other reasons (I was thinking about improving their performance like #21672)

@testset "optimized reduce(vcat/hcat, A) for arrays" begin
    function check(op, args)
        Y = op(args...)

        varients = unique((
                args,
                collect(args),
                Tuple(args),
                (arg for arg in args),  # HasLength
                (arg for arg in args if true)  # SizeUnknown
            )) 

        @testset "reduce(::typeof($op), ::$(typeof(xs))" for xs in varients
            X = reduce(op, xs)
            @test X == Y
            xs isa Vector && @test typeof(X) == typeof(Y)
        end
    end

    for args in ([1:2], [[1, 2]], [1:2, 3:4], [[3, 4, 5], 1:2], [1:2, [3.5, 4.5]],
                 [[1 2], [3 4; 5 6]], [reshape([1, 2], 2, 1), 3:4])
        check(vcat,  args)
    end
    for args in ([1:2], [[1, 2]], [1:2, 3:4], [[3, 4, 5], 1:3], [1:2, [3.5, 4.5]],
                 [[1 2; 3 4], [5 6; 7 8]], [1:2, [5 6; 7 8]], [[5 6; 7 8], [1, 2]])
        check(hcat,  args)
    end
end
@simonbyrne
Copy link
Contributor

This should be fixable by defining appropriate reduce_first methods.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrays [a, r, r, a, y, s] collections Data structures holding multiple items, e.g. sets fold sum, maximum, reduce, foldl, etc.
Projects
None yet
Development

No branches or pull requests

3 participants