Docs say collect will return an Array, but implementation is different

[davidanthoff]

The docs for collect(collection) says that it will return an Array (see here). The implementation, though, calls similar(Array{T}, axes(A)) to create the container that will be returned, and that is explicitly not required to return an Array (see here).

The net effect is that for example

using StaticArrays

x = collect(i for i in SA[1,2])

will not return an Array.

I think either the docs should be updated to say that collect will return an AbstractArray, or the collect implementation should be changed to actually always return an Array, no matter what.

It certainly would be very handy to have a function in base that one can call on a collection that guarantees that one gets an Array, and not some custom array type.

[timholy]

In the long run, my favorite way to spell that operation is Array(values(collection)). But collect may be intended to be a shorthand for that.

[davidanthoff]

The place where this caught me off-guard is that this also means that list comprehensions are not guaranteed to return an Array, which I had always assumed to be the case. So currently [i for i in SA[1,2]] does not return an Array.

[tkf]

The docstring clearly says that it returns an Array so I'd expect that it's treated as a bug if the implementation is incompatible with it.

But there is a discussion #36106 for making collect less strict.

[JeffBezanson]

I think the docs should be changed. It seems clear that comprehensions over StaticArrays or OffsetArrays should return arrays of a similar type, and if you really need an Array calling the constructor should be the way to do it. We can add that as well.

[tkf]

Is array comprehension documented to use collect? Why not change it to lower to (say) into(AbstractArray, Base.Generator(f, xs)) #36288 instead? This way, you get a static array from [x + 1 for x in SA[1, 2, 3]] and an Array from collect(x + 1 for x in SA[1, 2, 3]).

[davidanthoff]

I've always wanted the ability to have the array type that any [...] syntax (i.e. both literal array creation and list comprehensions) creates depend on the type of the elements inside the brackets. To me that would be more useful than having it depend on the type of the source in a list comprehension. But it should probably be only one of these two options, and given that right now it can already depend on the type of the source but not the elements, my preferred ship has probably sailed :)

For collect, I don't really care, but agree Array(iterator) seems much nicer in any case.

What is the definition of collect then? A function that creates some AbstractArray, and which one really depends on a complicated set of rules?

[nalimilan]

This is indeed almost exactly the same issue as #36106.

I vote for changing the documentation to allow collect to return the most appropriate array type. The reason is that you can do Array(...) if you really want an object of that type. I don't see the point of having collect if it does exactly the same thing as Array.

The implementation, though, calls similar(Array{T}, axes(A)) to create the container that will be returned, and that is explicitly not required to return an Array (see here).

Well it's not completely clear whether similar(Array{T}, ...) is allowed to return something that isn't an Array. I think the docstring for similar is written with the implicit assumption that you will only define methods for custom array types, nor for ::Array{T}. Jeff seemed to consider this as type piracy at #36106 (comment).

[davidanthoff]

Well it's not completely clear whether similar(Array{T}, ...) is allowed to return something that isn't an Array.

I think the current docs very strongly suggest that you don't have to return an Array. The last example in this section has an example that says "creates an array that "acts like" an Array{Int} (and might indeed be backed by one)", which I think implies pretty clearly that it would be within the design to return something that is not an Array?

So I agree with everyone that the docs for collect probably just need an update. The alternative of changing the behavior of collect would be pretty breaking at this point, so not an option in any case.

[nalimilan]

Interesting. I hadn't noticed that example. OTOH it says "If A has conventional indexing, this will be identical to Array{Int}(undef, size(A))". Clearly we need a clearer specification in general.

[davidanthoff]

Yeah, it depends on the indexing of A, which in the case of StaticArrays.jl is unconventional, so one ends up with not getting an Array for a list comprehension over a static array.