You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Expand nomatch to accept any value (control fills, rolls, omits during joins) #857
DT[i, nomatch=NULL]
# Error in if (!is.na(nomatch) && nomatch != 0L) stop("nomatch must either be NA or 0, or (ideally) NA_integer_ or 0L") :
By expanding it to accept wider range of values, one can perform for example na.fill operation during the join operation, not after, lending to faster code. Related SO
Proposed behavior:
DT[i, nomatch=any_character_or_real] # fill with supplied value; could be 0, "foo", even NA ...DT[i, nomatch=NA] # fill with NA. Special case of above. Same behavior as today.DT[i, nomatch=0] # fill with zeroes. Special case of above. DT[i, nomatch=NULL] # omit non-matched rows. Equivalent to today's nomatch = 0DT[i, nomatch=locf] # reserved symbol. Equivalent to roll=+InfDT[i, nomatch=nocb] # reserved symbol. Equivalent to roll=-Inf
Alternatively, this could be achieved by introducing new argument fill, accepting any value:
[.data.table(..., fill=NA, ...)
jangorecki: adding some features to scope
rollequal=TRUE|FALSE
nomatch=.(0L) # fill numeric columns with 0, character columns with NAnomatch=.(0L, "missing") # fill numeric columns with 0, character columns with "missing"nomatch=.(NA, price=LOCF()) # fill NA, other than price column which should be rolled
changed the title [-]Expand `nomatch` to accept any value - control fills, rolls, omits all in one place[/-][+]Expand `nomatch` to accept any value (control fills, rolls, omits during joins)[/+]on Oct 5, 2014
Added to 1.9.6. Will add a global option and issue a warning message so that transition can be made in the next major release. Setting the global option will allow users to use the new implementation already.
nomatch = NA - current behaviour (and default). nomatch = 0 - fills with 0 instead of removing non-matched rows. nomatch = NULL - current behaviour of nomatch = 0. nomatch = LOCF(val, rollends=TRUE/FALSE) and nomatch = NOCB(val, rollends=TRUE/FALSE) - could potentially replace roll and rollends arguments?? nomatch = val - should type convert appropriately and use val for all columns.
I think that second option is a bit revolutionary and potentially going to "piss" a lot of people who either don't read documentation/warnings or will have to rewrite huge code chunks...
It's bad enough that this will break a lot of code, but now we'd have to type even more characters for this very common operation? Uhh...
I don't like just saying I dislike smth without presenting a viable alternative, but the only thing I can think of is a new argument, which sucks in its own little way :-\
On extra characters, I guess you're talking about "roll" arguments? I'm not quite certain what are the extra characters otherwise.. And, I am not sure about the "roll" part, hence the "??". The idea is to first implement nomatch = <any_value> first.
# previouslyX[Y, roll=Inf, rollends=TRUE]
# afterX[Y, nomatch= LOCF(Inf, TRUE)] # default case can be just nomatch = "locf"
Is this what you find hard?
For one, I find it straightforward, as "roll" actually is used to "fill in" missing values. Second, the number of arguments that are mutually exclusive in [.data.table are one too many, and this helps address that.. (by removing roll and rollends as separate arguments). Right now, using roll=TRUE and nomatch = 0L returns the rolled result, which I find confusing. Providing nomatch = roll is explicit in that it asks the results to be rolled in the event of a no match.
On breaking code.. like I said before, in this version (1.9.6), we'll have a global argument which will be set to current behaviour and will issue a warning that nomatch = 0L should be replaced to nomatch = NULL and the current behaviour will be deprecated in the next version (which is plenty time), and we'll add a new vignette to explain the benefits. There will also be an option to switch off this warning. (or at least this is what I've in mind right now). @mattdowle thoughts?
I think this is a hugely welcoming change, similar to by = .EACHI (which you advocated btw, @eddi, which also would have resulted in lot of code being broken). The point is to bring in as much consistency as possible, by transitioning smoothly (and by giving ample time for users to shift their code), and this kind of "major" change shouldn't happen a lot (it's a lot of effort for us as well), but it's essential here.
I'm talking about typing NULL instead of 0, I didn't even think about the roll. For me, nomatch=0 has been a major annoyance from the very beginning, and I'm not too happy about it becoming even more annoying.
That said I totally understand the need for filling with other numbers/throwing an error, I'm just not particularly happy with this implementation.
Actually the root of my annoyance comes from nomatch=NA being the default, instead of nomatch=NULL/0. Since we're talking about a breaking change, can we please discuss making NULL/0 the default? I'll make a short "pro" argument: it's the default for base merge and ime by far the more common type of a join/merge.
Taking into account what Gsee mentioned about having a release as fallback in case changes are too major (and possibly affecting current code too much), I've changed it to 1.9.8. 1.9.6 will keep things as expected for people who don't want their code to be affected (and they need not upgrade if they wish so).
I agree, purely on consistency, that nomatch = NULL should be the default. IMO, we'll have to do that as well. The default should be an inner join.
This means, this'll be default in 2.0.0 (depending on what Matt feels as well).
As for breaking the code. The sooner we have it as an option to better tested it will be, so if you are not in rush with 1.9.6 (sad) it can be worth to put it there. But for the breaking changes - moving from option to default - the best will be 2.0.0 release.
If we consider adding new argument to [ to it is worth to review other open issues which would like to get own new arguments. Maybe they can be nicely combined. The one very similar would be #614, or maybe also #830 would need one - this can be combined with roll maybe?
PS. do you drop the nomatch=quote(stop("my error")) FR at all?
changed the title [-]Expand `nomatch` to accept any value (control fills, rolls, omits during joins)[/-][+]Expand nomatch to accept any value (control fills, rolls, omits during joins)[/+]on Feb 19, 2019
it seems like there are 2 discrete ideas that are being combined in a single parameter:
the nature of the join (inner or right)
the fill values to use for a left join
granted, if you do an inner join, you don't need fill values, but adding new parameters would get you around the transition and backward compatibility quite easily:
add join.type as a new parameter (currently only allowing inner, right)
add join.fill as a new parameter taking a list of name/value pars as described in Matt's Oct4th post.
the join.type could be implemented now, with warnings and backward compatibility would be a simple thunk to the new parameter.
this would also leave open the possibility for supporting full join in the future, or possibly even the sacrilegious left join ;)
# `nomatch` argument value on the left, translation on the rightNA->list(a=NA, b=NA)
0L->NULL"locf"->list(a=LOCF(), b=LOCF())
"stop"->list(a=stop(), b=stop())
NULL->"[remove non matching rows]"list(0L) ->list(a=0L, b=0L) # fill with zeroslist(a=NA, b=LOCF()) # fill with a column with NA, and b with LOCFlist(a=NA, b=stop()) # fill a column is irrelevant becase we will raise exception anyway# this should be supported as well, so `nomatch` can be more easily passed as a parameterlist(a=NA, b=quote(LOCF()))
list(a=NA, b=quote(stop()))
# of we could also provide LOCF() function, that crease a specification object, rather than a symbol that we handle in unevaluated expression.list(a=NA, b=LOCF())
rolling join options rollends and rollequal (#579) are whole tables join specification, rather than per-column specification. Not sure if we want to have them as argument to be specified in LOCF(rollends, rollequal) as then it would be defined per-column.
# some extra guessing of user intentlist(0L, "missing") # fill numeric columns with 0, character columns with "missing"list(NA, price=LOCF()) # fill NA, other than price column which should be rolled
Assuming we will implement an argument that controls join type ("inner","left","right","full"#3946) then nomatch will be provided automatically
left, right, full -> NA
inner -> NULL
note that there is an idea of provided two kinds of nomatch, one for LHS table, and another for RHS, which would control if join is inner, left, right, full: #614 (comment)
Activity
[-]Expand `nomatch` to accept any value - control fills, rolls, omits all in one place[/-][+]Expand `nomatch` to accept any value (control fills, rolls, omits during joins)[/+]jangorecki commentedon Dec 13, 2014
Merging with #940 as it is the same feature.
Extending Daniel's examples for an prepared error message in case of nomatch.
We have 2 dts:
Which we want join:
Consider the case when we expect to have match on all data in the join.
In case of nomatch we want to raise error.
Currently it is possible using:
Which makes us unable to make chaining in such case.
If we could use something like this:
just to stop process and raise the error in case of any nomatch.
arunsrinivasan commentedon Apr 23, 2015
Added to
1.9.6. Will add a global option and issue a warning message so that transition can be made in the next major release. Setting the global option will allow users to use the new implementation already.nomatch = NA- current behaviour (and default).nomatch = 0- fills with 0 instead of removing non-matched rows.nomatch = NULL- current behaviour ofnomatch = 0.nomatch = LOCF(val, rollends=TRUE/FALSE)andnomatch = NOCB(val, rollends=TRUE/FALSE)- could potentially replacerollandrollendsarguments??nomatch = val- should type convert appropriately and usevalfor all columns.DavidArenburg commentedon Apr 23, 2015
I think that second option is a bit revolutionary and potentially going to "piss" a lot of people who either don't read documentation/warnings or will have to rewrite huge code chunks...
eantonya commentedon Apr 23, 2015
It's bad enough that this will break a lot of code, but now we'd have to type even more characters for this very common operation? Uhh...
I don't like just saying I dislike smth without presenting a viable alternative, but the only thing I can think of is a new argument, which sucks in its own little way :-\
arunsrinivasan commentedon Apr 23, 2015
On extra characters, I guess you're talking about "roll" arguments? I'm not quite certain what are the extra characters otherwise.. And, I am not sure about the "roll" part, hence the "??". The idea is to first implement
nomatch = <any_value>first.Is this what you find hard?
For one, I find it straightforward, as "roll" actually is used to "fill in" missing values. Second, the number of arguments that are mutually exclusive in
[.data.tableare one too many, and this helps address that.. (by removing roll and rollends as separate arguments). Right now, usingroll=TRUEandnomatch = 0Lreturns the rolled result, which I find confusing. Providingnomatch = rollis explicit in that it asks the results to be rolled in the event of a no match.On breaking code.. like I said before, in this version (1.9.6), we'll have a global argument which will be set to current behaviour and will issue a warning that
nomatch = 0Lshould be replaced tonomatch = NULLand the current behaviour will be deprecated in the next version (which is plenty time), and we'll add a new vignette to explain the benefits. There will also be an option to switch off this warning. (or at least this is what I've in mind right now). @mattdowle thoughts?I think this is a hugely welcoming change, similar to
by = .EACHI(which you advocated btw, @eddi, which also would have resulted in lot of code being broken). The point is to bring in as much consistency as possible, by transitioning smoothly (and by giving ample time for users to shift their code), and this kind of "major" change shouldn't happen a lot (it's a lot of effort for us as well), but it's essential here.eantonya commentedon Apr 23, 2015
I'm talking about typing
NULLinstead of0, I didn't even think about the roll. For me,nomatch=0has been a major annoyance from the very beginning, and I'm not too happy about it becoming even more annoying.That said I totally understand the need for filling with other numbers/throwing an error, I'm just not particularly happy with this implementation.
Actually the root of my annoyance comes from
nomatch=NAbeing the default, instead ofnomatch=NULL/0. Since we're talking about a breaking change, can we please discuss makingNULL/0the default? I'll make a short "pro" argument: it's the default for base merge and ime by far the more common type of a join/merge.arunsrinivasan commentedon Apr 23, 2015
Taking into account what Gsee mentioned about having a release as fallback in case changes are too major (and possibly affecting current code too much), I've changed it to 1.9.8.
1.9.6will keep things as expected for people who don't want their code to be affected (and they need not upgrade if they wish so).I agree, purely on consistency, that
nomatch = NULLshould be the default. IMO, we'll have to do that as well. The default should be an inner join.This means, this'll be default in 2.0.0 (depending on what Matt feels as well).
eantonya commentedon Apr 23, 2015
Ok, if
nomatch=NULLis made the default, then I have no issues, and the change would be great.jangorecki commentedon Apr 23, 2015
As for breaking the code. The sooner we have it as an option to better tested it will be, so if you are not in rush with 1.9.6 (sad) it can be worth to put it there. But for the breaking changes - moving from option to default - the best will be 2.0.0 release.
If we consider adding new argument to
[to it is worth to review other open issues which would like to get own new arguments. Maybe they can be nicely combined. The one very similar would be #614, or maybe also #830 would need one - this can be combined with roll maybe?PS. do you drop the
nomatch=quote(stop("my error"))FR at all?22 remaining items
[-]Expand `nomatch` to accept any value (control fills, rolls, omits during joins)[/-][+]Expand nomatch to accept any value (control fills, rolls, omits during joins)[/+]ethanbsmith commentedon May 14, 2019
it seems like there are 2 discrete ideas that are being combined in a single parameter:
granted, if you do an inner join, you don't need fill values, but adding new parameters would get you around the transition and backward compatibility quite easily:
add
join.typeas a new parameter (currently only allowing inner, right)add
join.fillas a new parameter taking a list of name/value pars as described in Matt's Oct4th post.the join.type could be implemented now, with warnings and backward compatibility would be a simple thunk to the new parameter.
this would also leave open the possibility for supporting
fulljoin in the future, or possibly even the sacrilegiousleftjoin ;)jangorecki commentedon Apr 10, 2020
To summarize current requirements
rolling join options
rollendsandrollequal(#579) are whole tables join specification, rather than per-column specification. Not sure if we want to have them as argument to be specified inLOCF(rollends, rollequal)as then it would be defined per-column.Assuming we will implement an argument that controls join type (
"inner","left","right","full"#3946) thennomatchwill be provided automaticallynote that there is an idea of provided two kinds of
nomatch, one for LHS table, and another for RHS, which would control if join is inner, left, right, full: #614 (comment)