RFC: Implementation of deleteat! (Fixes #5118)

[kmsquire]

As noted in #5118, delindex! was previously proposed but never implemented. Here is an implementation delindex! deleteat! for

• single indexes
• Range1
• general iterable of indices (must be sorted)

delindex! deleteat! is more efficient than splice! when the items being removed from the collection are not needed and returning them is costly.

This PR also changes/fixes the return type of one of the splice! implementations for BitArrays, which was returning the modified array instead of the spliced-out region. cc:@carlobaldassi

Edit: changed name from delindex! to deleteat!.

[StefanKarpinski]

How about calling this operation deleteat! instead of delindex. The former name seems to have more historical precedent.

[StefanKarpinski]

#bikeshed

[kmsquire]

After thinking about it, I kind of like the parallels with getindex and setindex!.

[kmsquire]

If we had deleteat!, would we also want insertat!? This is already handled by splice!, of course, except that insertat! could return the modified vector...

Like all of these things, it's easy to get carried away. delindex (or deleteat), at least, I've had a need for.

[ivarne]

You should document the need for a sorted iterable.

Why only sorted anyway? I see that it is good for performance, but could we have a slow fallback instead of an error?

[StefanKarpinski]

The abbreviated del just doesn't sit well with me. We've expunged almost all other abbreviations in collection operations. I'm also not sure that analogy to getindex and setindex! is very important – because no one every uses those names except when they want to overload x[i] and x[i] = v.

[kmsquire]

Why only sorted anyway? I see that it is good for performance, but could we have a slow fallback instead of an error?

This was the easiest way to implement the function without worrying about explicitly storing/copying/modifying the iterable. That version currently starts to modify the array immediately on iteration through the indices, so if something is out of order, it's not possible to undo the changes already made.

I don't think it's possible to have a slow fallback without considerably slowing down the fast version. If we want a version which checks the input order, we might as well always copy/collect and sort it and get rid of this version.

It does need documentation, though, if it doesn't change.

[kmsquire]

The abbreviated del just doesn't sit well with me. We've expunged almost all other abbreviations in collection operations.

Just so I'm clear, it's the del at the beginning of delindex! you're reacting to, not that someone suggested using del, right?

I'm also not sure that analogy to getindex and setindex! is very important – because no one every uses those names except when they want to overload x[i] and x[i] = v.

Okay, I can certainly change it to deleteat!.

[StefanKarpinski]

Right, it's the del at the beginning that rubs me a bit wrong. I think deleteat! feels much better.

[JeffBezanson]

This strikes me as a lot of repeated code from splice!.

[kmsquire]

I refactored one of the BitArray versions, but the rest didn't jump out as
quickly. But I'll take a look.

On Monday, December 16, 2013, Jeff Bezanson wrote:

This strikes me as a lot of repeated code from splice!.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/5176#issuecomment-30725050
.

[kmsquire]

Okay, factored out common code between deleteat! and splice!.

[StefanKarpinski]

bump. @JeffBezanson, @ViralBShah? thoughts on this operation?

[StefanKarpinski]

I'm cool with this and it is still mergeable.