eachslice + collect

[mcabbott]

I believe this closes #59 and #34.

I ‘m not sure why write a custom generator for eachslice. This PR changes it to just looks up dims, then run the same code as base:

julia> ab = NamedDimsArray((1:3) .+ zeros(Int,3)', (:a, :b));

julia> @btime eachslice($ab, dims=:b); # master: 319.215 ns (5 allocations: 112 bytes)
  220.690 ns (3 allocations: 64 bytes) 

julia> @btime eachslice($(parent(ab)), dims=2);
  216.897 ns (3 allocations: 64 bytes)

julia> @btime [sum(x)^2 for x in eachslice($ab, dims=:b)]; # master: 14.636 μs (40 allocations: 1.73 KiB)
  383.851 ns (8 allocations: 336 bytes)

Still much slower than eachcol, for which there is no special code at all:

julia> NamedDims.names(first(eachcol(ab)))
(:a,)

julia> @btime eachcol($ab);
  12.992 ns (2 allocations: 48 bytes)

julia> @btime [sum(x)^2 for x in eachcol($ab)];
  76.208 ns (7 allocations: 320 bytes)

It also adds the simplest possible overload of collect so that simple comprehensions will keep names. Doesn’t work on generators of generators, nor products:

julia> [x^2 for x in ab]
3×3 NamedDimsArray{(:a, :b),Int64,2,Array{Int64,2}}:
 1  1  1
 4  4  4
 9  9  9

julia> NamedDims.names([sum(x)^2 for x in eachcol(ab)]) # doesn't work yet
(:_,)

[codecov]

Codecov Report

Merging #73 into master will decrease coverage by 0.1%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master      #73      +/-   ##
==========================================
- Coverage   86.63%   86.53%   -0.11%     
==========================================
  Files           8        8              
  Lines         247      260      +13     
==========================================
+ Hits          214      225      +11     
- Misses         33       35       +2

Impacted Files	Coverage Δ
src/functions.jl	97.67% <100%> (+0.37%)	⬆️
src/name_core.jl	90.24% <0%> (-1.76%)	⬇️

---

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 26e974b...97d070d. Read the comment docs.

[nickrobinson251]

thanks!

[nickrobinson251]

@oxinabox previously objected to copying Base code here, so I'll defer to him on this

[oxinabox]

I was wrong

[mcabbott]

Other approaches are possible, I see that Compat.jl does have eachslice etc. I don't know what you think of depending on that.

[nickrobinson251]

I see that Compat.jl does have eachslice etc. I don't know what you think of depending on that.

Now that Compat.jl is for Julia v1.0+ I'd be happy to use it, rather than copy code here :)

[nickrobinson251]

why are these test_broken?
If there's a good reason, let's open an issue to fix them, and comment a link to that issue here

[oxinabox]

This doesn't seem like it is broken at all.
that is a normal vector of named vectors.
I think I would find it surprising if it wasn't.

[mcabbott]

I think the name of the generator dimension would ideally be passed along, as is now done for [f(x) for x in nda]. But I couldn't see a way to make this work without JuliaLang/julia#32310 .

[oxinabox]

It also adds the simplest possible overload of collect so that simple comprehensions will keep names. Doesn’t work on generators of generators, nor products:

Can this be in a seperare PR, it is a more contentious feature that IDK how I feel about right now

[mcabbott]

Perhaps we should have an issue for how many more general generators can or should be handled? Making [sqrt(x) for x in ab] behave like map(sqrt, ab) seemed straightforward.
Edit: perhaps more general maps, like map(+, ab, rand(3,3)), which also currently forgets names, fall in the same basket.

[mcabbott]

Now that Compat.jl is for Julia v1.0+ I'd be happy to use it, rather than copy code here :)

OK then perhaps I close this, and leave you to make a Compat PR.

it is a more contentious feature ...

If you believe map & collected generators ought not to simply propagate names along, then perhaps open a discussion issue?