add deleteat! method

[vlepori]

As discussed in #55. I kept the deleteat!(A,(),i) API.

The following are supported

a = ElasticArray(rand(7, 7, 7))
deleteat!(a, (), (), ())
deleteat!(a, (), (), 6)
deleteat!(a, (), (), [1, 5])
deleteat!(a, (), (), 3:4)

And this errors

deleteat!(a, 1:2, (), 1:2)
# ERROR: ArgumentError: Can only delete elements in the last dimension of A

[oschulz]

@vlepori looking at this again, maybe the primary way to do it should be deleteat!(A, ((), i)), but we should support deleteat!(A, (), i) as well. It matches how handle resize! in https://github.com/JuliaArrays/ElasticArrays.jl/blob/7e0b6f8b4134d0b203fc45f6f80296524fa419eb/src/elasticarray.jl#L108 .

Could you change idxs::Vararg{...} to idxs::NTuple{...} in the primary method and then add a convenience method with idxs::Vararg{...} like in https://github.com/JuliaArrays/ElasticArrays.jl/blob/7e0b6f8b4134d0b203fc45f6f80296524fa419eb/src/elasticarray.jl#L106

?

[oschulz]

Oh, and could you add a test or two?

[vlepori]

Good point, both APIs are supported now. I also added a couple of tests.

[oschulz]

Ok, let's run Ci on it then. :-)

[codecov[bot]]

Codecov Report

Merging #56 (34d0809) into main (03bfd5d) will decrease coverage by 0.25%. The diff coverage is 80.00%.

@@            Coverage Diff             @@
##             main      #56      +/-   ##
==========================================
- Coverage   84.74%   84.49%   -0.25%     
==========================================
  Files           2        3       +1     
  Lines         118      129      +11     
==========================================
+ Hits          100      109       +9     
- Misses         18       20       +2     

Impacted Files	Coverage Δ
src/elasticarray.jl	84.00% <80.00%> (-0.35%)	:arrow_down:

... and 1 file with indirect coverage changes

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[oschulz]

Hm, looks like there trouble on v1.0 .

[vlepori]

At least one problem here is that the method first(v,i) was introduced only in Julia 1.6. But since idxs is a Tuple I think we can safely change first(idxs, length(idxs) - 1) to idxs[1:end-1] and not worry about non-standard indexing? I can update the PR later today.

[oschulz]

But since idxs is a Tuple I think we can safely change first(idxs, length(idxs) - 1) to idxs[1:end-1] and not worry about non-standard indexing?

Yes, I think that should be Ok.

[oschulz]

Hey @vlepori I'm so sorry this got stalled, I completely forgot to click "run CI" ...

[oschulz]

@vlepori could you take a look at the failing tests?

[vlepori]

@oschulz yes sorry, I'll try to get a working installation of Julia 1.0 and look into this soon !

[oschulz]

Thanks @vlepori , no worries!

[jeremiahpslewis]

Would love to see this added, could be great to use it to implement pop!

[oschulz]

@vlepori want to get back on this?

[vlepori]

Sure! Got sidetracked and forgot about this but if there is interest I will try to get it done!

[vlepori]

So I went down the rabbit hole behind this failing test on julia 1.0 https://github.com/JuliaArrays/ElasticArrays.jl/blob/34d0809ed801caa5700a05977eacc071e9f9cb26/test/elasticarray.jl#L310

It seems to have nothing to do with ElasticArrays but seems related to a weird side effect of the @test macro in older versions of Julia. I can reproduce it in Julia 1.0.3 like so

using Test

function fun()
    d = 2
    @test true # works as aspected when commenting out this line
    ntuple(i -> i == d ? true : false, 2)
end
fun() # should return (false,true) but gives (true,true)

I don't quite understand further (?) but the bug has been fixed in more recent versions, so maybe just skip the test ?

[oschulz]

Hm, now that v1.10 is set to become the new LTS, maybe it's time to drop v1.0 support?

[vlepori]

Not sure if it's necessary for just this one test, but I'd be surprised if many people still use 1.0. For comparison DifferentialEquations stopped supporting it 4 years ago and now requires julia 1.9

[oschulz]

Looks like among the package that depend on ElasticArrays, the packages EfficientGlobalOptimization, StanSample and SphericalHarmonics still (claim to ) support Julia v1.0.

So I suggest we do indeed skip the test, but keep v1.0 compatibility for now.