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

ElasticArrays.jl/src/elasticarray.jl

Line 108 in 7e0b6f8

@inline function Base.resize!(A::ElasticArray{T,N}, dims::NTuple{N,Integer}) where {T,N}

.

Could you change idxs::Vararg{...} to idxs::NTuple{...} in the primary method and then add a convenience method with idxs::Vararg{...} like in

ElasticArrays.jl/src/elasticarray.jl

Line 106 in 7e0b6f8

@inline Base.resize!(A::ElasticArray{T,N}, dims::Vararg{Integer,N}) where {T,N} = resize!(A, dims)

?

[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]

Why wrap this in a function?

[vlepori]

Just consistency, since other functions are tested with a functionname_test(). But we can also call test_E() directly

[oschulz]

But we can also call test_E() directly

Let's to that, it's simpler. I mainly wrapped tests if there are called several times with different arrays sizes or so.

[vlepori]

Yeah that makes sense, I simplified it.

[oschulz]

Can you add an @inferred here?

[oschulz]

An @inferred might be nice here as well.

[oschulz]

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

[codecov]

Codecov Report

❌ Patch coverage is 80.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.49%. Comparing base (03bfd5d) to head (34d0809).
⚠️ Report is 11 commits behind head on main.

Files with missing lines	Patch %	Lines
src/elasticarray.jl	80.00%	2 Missing ⚠️

Additional details and impacted files

@@            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     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[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

ElasticArrays.jl/test/elasticarray.jl

Line 310 in 34d0809

@test selectdim(E, n, 2:s) == @inferred deleteat!(b, ntuple(i -> i == n ? 1 : (), n))

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.