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.
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:
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.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.
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.