Repository navigation
dt[, uniqueN(col)] does not warn when col is evaluated outside of dt's frame #2186
Description
Activity
- I don't think this is anything particular to uniqueN, or even data.table for that matter -- standard R behavior of ascending call scope to find variables, no?…On Jun 3, 2017 9:43 PM, "Melanie Bacou" ***@***.***> wrote: Hi, the very simple code below returns 20 without warning. This is potentially very error-prone, when the user would normally expect a column named v in dt, but that column is accidentally missing. Not sure if this is intended. Thx! v <- 1:20dt <- data.table(a=1:10, b=1:10)dt[, uniqueN(v)] — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#2186>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHQQdekInH6OuWCqtT4RpW5JroHbHOFnks5sAgvAgaJpZM4NvP6P> .
Yes and no,
data.framewould throw an error obviously. Generally, it seems to me that evaluation in ascending scope inj's (and also inby's ?) element is very prone to user errors. Somehow I feel like forcing the user to make an explicit call to an out-of-scope object would be safer (e.g. maybe using something like..(col)-- but even then, calls likedt[, unique(..(col))]ordt[, sum(..(col))]would make no sense at all, and should throw warnings or errors.- maybe... OTOH there's no error/warning in x = 5 f = function(y) x+5 f(0) How/why should that differ for a [] call? the default behavior of setting with=TRUE is well documented in introductory vignettes... and personally I rely on occasionally referring to up-the-scope variables intentionally for clean code (essentially referring to "global" variables). would be strange to get a warning in such cases.…On Jun 3, 2017 11:26 PM, "Melanie Bacou" ***@***.***> wrote: Yes and no, data.frame would throw an error obviously. Generally, it seems to me that evaluation in ascending scope in j's element is very prone to user errors. Somehow I feel like forcing the user to make an explicit call to an out-of-scope object would be safer (e.g. maybe using something like ..(v). — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#2186 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHQQdVWBFedtXAqxKx3NnYv9ztvcjjeUks5sAiPOgaJpZM4NvP6P> .
I think the proposed
inherits = FALSEwould cover this #633 and agree it would be nice to have.How/why should that differ for a [] call?
@MichaelChirico Because it protects us from writing
jexpressions that we don't want to be writing, just likeDT[1, v := x ]will protect us ifxdoes not have the right class, etc.- changed the title
[-]dt[, uniqueN(col)] does not warm when `col` is evaluated outside of dt's frame[/-][+]dt[, uniqueN(col)] does not warn when `col` is evaluated outside of dt's frame[/+]on Jun 4, 2017 I agree here,
inherits = FALSEshould be default behavior, with an explicit..(col1, col2)notation possibly allowed.Else in cases as below, a simple warning that "
colandbywere evaluated outside ofdt's scope" would not hurt!by <- 1:10 col <- 1:10 dt <- data.table(a=1:10, b=1:10) dt[, c := sum(a*col), by=by]Still feel like
dt[, c := sum(a*..(col)), by=..(by)]is (ugly) but safer!As @franknarf1 pointed out, it's a nice feature to have, and is a dup of #633.
Hi, the very simple code below returns
20without warning. This is potentially very error-prone, when the user would normally expect a column namedvindt, but that column is accidentally missing. Not sure if this is intended. Thx!Tested with data.table 1.10.4.