Support collecting to offset arrays

[tkf]

This is something I noticed while writing #97. Not sure if you need this but I guess it's easier to fix than checking has_offset_axes and giving a good error message.

[tkf]

Broken due to this?

StructArrays.jl/src/collect.jl

Lines 28 to 29 in acb24c8

	# temporary workaround before it gets easier to support reshape with offset axis
	reshapestructarray(v::AbstractArray, d) = reshape(v, d)

[codecov-io]

Codecov Report

Merging #107 into master will increase coverage by 0.02%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #107      +/-   ##
==========================================
+ Coverage   91.22%   91.24%   +0.02%     
==========================================
  Files           9        9              
  Lines         376      377       +1     
==========================================
+ Hits          343      344       +1     
  Misses         33       33

Impacted Files	Coverage Δ
src/collect.jl	95.77% <100%> (+0.06%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ef21985...acb24c8. Read the comment docs.

[piever]

Thanks for spotting the issue! I think at least for now I would prefer to disallow changing the shape of the iterator, my attempt to clean up the inconsistency you spotted is #110, do you think that's satisfactory? I could maybe add a check that initializer preserves the shape that it receives.

[tkf]

Thanks for fixing this. #110 looks like a much better solution. It would be nice to add some tests as in this PR (or something similar) as well?

[piever]

In retrospect, I actually need to allow to collect with a different shape, as IndexedTables always wants to collect as a 1D vector. That is to say in the end I should add your test and make it work (which I believe it does with the current implementation in #110 ).

The only doubt that I have is, in the case where the iterator does not have shape, should I pass to initializer simply the length, i.e. call initializer(T, length(itr)), or is it better to be consistent and always pass a shape, i.e. (Base.OneTo(length(itr),), as in https://github.com/JuliaArrays/StructArrays.jl/pull/110/files#diff-e4214fcf2ccd8a85f7b4de8c5a642290R26 ? The trade-off is that one implementation allows custom collection behavior, depending whether the iterator has a shape or not, whereas the other makes it a bit more annoying to define a custom initializer, as one needs to be able to handle both cases. I would probably prefer passing (Base.OneTo(length(itr),).

UPDATE: have gone for the consistent option (always passing axes to initializer) and merged the PR, including your tests which are no longer broken.

[tkf]

have gone for the consistent option (always passing axes to initializer)

This sounds like a good approach. Thanks for including the tests.