improvements to stack

[CarloLucibello]

Fix #121 and add a rrule for stack.

Edit: Remove the stack implementation in favor of the one from Base. The change is technically breaking since we don't support anymore dims > N+1 (see https://github.com/JuliaML/MLUtils.jl/issues/119#issuecomment-1229127997)

Fix #121 fix #119

[codecov-commenter]

Codecov Report

Merging #125 (531174a) into main (125205b) will decrease coverage by 0.13%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #125      +/-   ##
==========================================
- Coverage   88.75%   88.61%   -0.14%     
==========================================
  Files          13       13              
  Lines         578      571       -7     
==========================================
- Hits          513      506       -7     
  Misses         65       65              

Impacted Files	Coverage Δ
src/utils.jl	90.40% <100.00%> (-0.51%)	:arrow_down:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[darsnack]

Errors due to #119

[CarloLucibello]

I think it is better to wait for https://github.com/JuliaML/MLUtils.jl/pull/124 to be merged and then remove the stack implementation from here

[darsnack]

Based on the discussion in #119, it doesn't seem like Base.stack/Compat.stack are 1-1 replacements for MLUtils.stack. I'm mostly concerned about MLUtils.stack(x, 2) style conflicting with Compat.stack(f, xs).

I think it is better to wait for https://github.com/JuliaML/MLUtils.jl/pull/124 to be merged

You mean so we release both changes together? Or is there a connection to Tables.jl that I am missing?

[CarloLucibello]

Based on the discussion in https://github.com/JuliaML/MLUtils.jl/issues/119, it doesn't seem like Base.stack/Compat.stack are 1-1 replacements for MLUtils.stack. I'm mostly concerned about MLUtils.stack(x, 2) style conflicting with Compat.stack(f, xs).

MLUtils.stack(x, 2) was deprecated already with v0.2 in favour of the keyword arg version, so we should be fine in removing it for v0.3.

I think it is better to wait for https://github.com/JuliaML/MLUtils.jl/pull/124 to be merged

You mean so we release both changes together? Or is there a connection to Tables.jl that I am missing?

ops, I mentioned the wrong PR, I meant https://github.com/JuliaDiff/ChainRules.jl/pull/681

[mcabbott]

This method should also call stack (if it cannot be removed):

https://github.com/JuliaML/MLUtils.jl/blob/855f95b436e5b9ebe9704486eb3dfdf5dad011ee/src/utils.jl#L332-L342