Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

the two meanings of delete! #3405

Closed
StefanKarpinski opened this issue Jun 15, 2013 · 16 comments
Closed

the two meanings of delete! #3405

StefanKarpinski opened this issue Jun 15, 2013 · 16 comments
Labels
needs decision A decision on this change is needed
Milestone

Comments

@StefanKarpinski
Copy link
Member

I was looking at the delete! function and thinking how requiring that the value to be deleted exist in the collection is often annoying. More annoying still is that it makes what could be an idempotent function non-idempotent. The underlying issue is that there are two ways of using delete!:

  1. purely for its effect of removing an item from a collection.
  2. using it to remove something from a collection and for its return value.

Returning a value from delete! is primarily useful for associative collections, where the value associated with the deleted key is returned. This is often handy. This is also why delete!(a,k) throws an error if a doesn't contain the key k since the method doesn't have any value to return. If the three-argument form, delete!(a,k,d), is used, there there is a default value to return, so no error is/need be thrown.

When this logic is applied to non-associative collections, it becomes largely nonsensical: since you already know the value you're deleting, it's unusual to also care about what delete! returns. The exception is when you have mutable objects that hash the same, so object used to do the lookup and the object that's returned are actually distinguishable. We're not even handling this situation correctly at the moment since I think there are cases where delete!(s,x) returns x rather than object equal to x that's in s.

All this leads me to wonder if we shouldn't separate the two meanings of delete! into:

  1. delete!(c,x) which returns nothing and is idempotent,
  2. take!(c,x) which returns the associated value and throws an error if c doesn't contain x and thus is not idempotent.

Although it would be infrequently used, take! could be implemented for non-associative collections and should return the value stored in the collection, not the one passed into the call, so that it is useful in the cases where those objects are mutable and therefore distinguishable. A further distinction could be that delete! returns the collection, making it friendly to chaining operations, whereas take! obviously would not return the collection since returning a value is its raison d'etre.

[Another interesting but possibly awful/infeasible idea is that delete! could return undef when the deleted value doesn't exist. Then if someone uses the returned value in any way an error is thrown upon usage. Thus, if you're using delete! in the idempotent way, ignoring its return value, there's no problem with removing a non-existent key. But if you use the return value of delete! and the key didn't exist, it would be an error. This is probably too tricky and magical though.]

@JeffBezanson
Copy link
Member

It is not possible for a function to return undef (there is no such thing). I think allowing that would be an extremely drastic and ill-advised change.

@StefanKarpinski
Copy link
Member Author

That's why I put it in brackets and said it was possibly "awful/infeasible".

@kmsquire
Copy link
Member

take(x, n) also has a common meaning of "take (and return) the first n elements of x" in stream/sequence processing. (Not that julia has much in the way of stream processing yet, but that could be confusing, even with the !)

@StefanKarpinski
Copy link
Member Author

Yeah, the name is clearly up for debate. Any other ideas for a name?

@JeffBezanson
Copy link
Member

remove! ?

@kmsquire
Copy link
Member

pop!(x,k) or popkey!(x,k)?

remove!(), sematically, seems an awful lot like delete!()

I do rather like the idea that delete! could return the the updated collection.

@StefanKarpinski
Copy link
Member Author

The idea of pop! with a key is intriguing. Perhaps we can come up with a coherent notion of pop! that fits both the traditional array/stack version and this version. I also feel like it would be hard to keep delete! and remove! straight.

@JeffBezanson
Copy link
Member

Fine with me. Python has dict.pop(key) => value.

@StefanKarpinski
Copy link
Member Author

This kind of implies that we should also have pop!(array,index) => value and maybe pop!(array,indices) => values, which of course starts to tread on the territory of the recently-added splice! function.

@JeffBezanson
Copy link
Member

It's slightly unfortunate that none of these provide a way to delete a range of indices in an array without allocating an array of the removed values.

@JeffBezanson
Copy link
Member

Perhaps splice! should return the modified array, and you can simulate the current behavior with v = a[r]; splice!(a,r,x).

@timholy
Copy link
Member

timholy commented Jun 16, 2013

It's slightly unfortunate that none of these provide a way to delete a range of indices in an array without allocating an array of the removed values.

This has bothered me too. Thankfully empty! has been what I needed most of the time.

@StefanKarpinski
Copy link
Member Author

This is really the same distinction: you want one version that just deletes stuff and throws it away and one version that returns the removed items too.

@kmsquire
Copy link
Member

Perhaps splice! should return the modified array, and you can simulate the current behavior with v = a[r]; splice!(a,r,x).

This is a possible gotcha if a[r] ever becomes a reference operation instead of a copy operation.

@StefanKarpinski
Copy link
Member Author

Yikes. That's a slightly terrifying prospect. Array resizing and views do not seem like a good mix at all.

@JeffBezanson
Copy link
Member

The only guarantee I currently provide on combinations of resizing and reshaping is that they won't segfault :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs decision A decision on this change is needed
Projects
None yet
Development

No branches or pull requests

4 participants