Julia: Proposal: add support for nothing in deleteat!

Created on 12 Jun 2019  Â·  15Comments  Â·  Source: JuliaLang/julia

Hi guys!

I was planning to submit a PR but then I realized it is better to discuss here first.

The function deleteat! can receive an integer or a set of integers. It works fine if an empty array is passed. Hence, I can use deleteat! with findall and everything will be fine if the elements I am searching for does not exist.

However, some other functions like findfirst, findlast, etc. return nothing if the element is not found. Thus, it seems very logical to me to add support for nothing to deleteat!. In this case, I can use deleteat!(v, findfirst(x->x==1,v)) with no problem.

Most helpful comment

As i understand it now the case is "if this function could return nothing, deal with it", if this idea goes through that will changed to "if this function could return nothing, the next thing you do might just work in call cases, but if it won't then you should deal with it." The second requires remembering more special cases.

All 15 comments

Is there a problem with

ind = findfirst(x->x==1, v)
if !isnothing(ind)
    deleteat!(v, something(ind))
end

in general nothing is treated as an error condition.
(wherre as missing is propegated)

No there is not, but I think it would make much more sense. It would be like delete nothing. It will be like what happen if you pass an empty array.

in general nothing is treated as an error condition.
(wherre as missing is propegated)

But findfirst returns nothing if the element is not found. IMHO, this is not necessarily an error. In your point of view, then findfirst should return an empty array in this case, right?

findall should return a empty array.
findfirst should return a lack of a scalar.
(Which we represent asnothing)

But then can't we apply the same concept to deleteat!? Like, a nothing is a lack of scalar and then nothing should be deleted?

Seems reasonable to me, but I'm no expert. Note that if this doesn't get support it's pretty trivial to get the behaviour you want:

my_deleteat!(args...) = deleteat!(args...)
my_deleteat!(v, ::Nothing) = v

julia> my_deleteat!([1, 2, 3], findfirst(x-> x==10, [1, 2, 3]))
3-element Array{Int64,1}:
 1
 2
 3

julia> my_deleteat!([1, 2, 3], findfirst(x-> x==1, [1, 2, 3]))
2-element Array{Int64,1}:
 2
 3

Note that if this doesn't get support it's pretty trivial to get the behaviour you want:

Yes I know :) It will be a very small PR. But, IMHO, it will make things more consistent.

nothing is not supposed to propagate, we are not going to add nothing-accepting methods to every function.

I think this is a little different and worth some discussion. This isn't "propagating nothing" like propagating missing that we find ourselves getting never-ending requests for—the desired behavior isn't that when nothing is the argument of some function, it return nothing as well. The concept here is that we use nothing to indicate that a single value wan't found, so we could support nothing to indicate looking up and acting on a single value that isn't there. There's a well-defined, limited set of functions that take indices in that way. That's not saying this is a good idea, but the argument about propagation doesn't really apply. I think the real question is whether this would potentially hide bugs.

As i understand it now the case is "if this function could return nothing, deal with it", if this idea goes through that will changed to "if this function could return nothing, the next thing you do might just work in call cases, but if it won't then you should deal with it." The second requires remembering more special cases.

Hi @ggggggggg ,

Notice that nothing will change from the users point of view. If they are treating nothing as special cases, then nothing (:D) will happen. However, IMHO, adding this support to at least deleteat! will make more sense. Like: findfirst and findlast returns nothing if the value is not found. Then, deleteat! with nothing as input will delete nothing.

Another way is to modify findfirst and findlast for the case in which the element is not found. But this should be way more problematic.

Of the top of my head, I can think of many, many functions for which it semantically makes sense to do nothing when passed nothing, e.g. pop!, getindex, convert, even functions like + and * (where it'd presumably mean zero, "add nothing to 5" is surely 5, right?).
In addition to what's already been mentioned, I'd argue:

  • It has limited value to add a whole lot of methods that literally do nothing
  • In the more general case (forgetting deleteat! for a moment), it is not always so clear what the indended behaviour of passing nothing to a function should be. And if the author of some code doesn't want to do nothing when passing nothing to a function, then we're just creating pitfalls.
  • The functions for which it makes sense to take nothing is a gray area and pretty subjective (does union(x, nothing) make sense? I think it does, it's cleary the empty set), so how do users know which have been implemented and which haven't?

Perhaps the real problem here is that the pattern deleteat!(arr, findfirst(x -> x == 5, arr)) is a little unintuitive. Certainly less clear than delete!(arr, 5). Anecdotally, I've seen a bunch of forum posts of people asking how to delete the first element from an array. Would it instead be worth it to add a new method to delete! that accepts arrays?

The distinction between delete! and deleteat! is subtle but important — both select a key/index to remove, but the latter _also_ remaps/shifts all the subsequent values up to "fill" the hole. Repurposing delete! to mean that it selects by value (and shifts) for arrays would introduce quite the disconnect between its behavior for arrays and dictionaries.

One possibility here would be to support nothing in to_indices — and have it return an Int[]. That would limit its scope to functions that accept index-like things. One reason not to do this is that numpy uses None to mean [CartesianIndex()] (that is, a new axis). Another reason not to do this is that there are lots of other things you can do with indices that _aren't_ indexing (like arithmetic) and they _definitely_ do not warrant nothing propagation/no-op support.

@mbauman I'm not sure I'm entirely convinced - what you're talking about seems to me to be more of a fundamental property of Sets and Dicts vs Arrays, and not of delete! vs deleteat!. I mean, Sets and Dicts are by their nature not dense, it makes little sense to shift values to fill a hole in a Dict.
Consider the inverse operation, push!, it states in the documentation that it inserts an item "at the end of the collection" - but that is not true for Sets either, where there conceptually is no "end". To me, the argument that delete! is not suitable for arrays implies that push! is not suitable for Sets.

The salient part isn't the shifting— it's the fact that delete! currently chooses which thing to remove based upon a _key_ (that is, an _index_; delete!(d, k) removes d[k]). Making delete! choose its element to remove based upon a _value_ for arrays but a _key_ for dictionaries is problematic.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

omus picture omus  Â·  3Comments

dpsanders picture dpsanders  Â·  3Comments

Keno picture Keno  Â·  3Comments

yurivish picture yurivish  Â·  3Comments

omus picture omus  Â·  3Comments