dt[, uniqueN(col)] does not warn when col is evaluated outside of dt's frame

[mbacou]

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!

Tested with data.table 1.10.4.

v <- 1:20
dt <- data.table(a=1:10, b=1:10)
dt[, uniqueN(v)]
# [1] 20

[MichaelChirico]

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

[mbacou]

Yes and no, data.frame would throw an error obviously. Generally, it seems to me that evaluation in ascending scope in j's (and also in by'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 like dt[, unique(..(col))] or dt[, sum(..(col))] would make no sense at all, and should throw warnings or errors.

[MichaelChirico]

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

[franknarf1]

I think the proposed inherits = FALSE would 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 j expressions that we don't want to be writing, just like DT[1, v := x ] will protect us if x does not have the right class, etc.

[mbacou]

I agree here, inherits = FALSE should be default behavior, with an explicit ..(col1, col2) notation possibly allowed.

Else in cases as below, a simple warning that "col and by were evaluated outside of dt'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!

[arunsrinivasan]

As @franknarf1 pointed out, it's a nice feature to have, and is a dup of #633.