how to avoid check NOTE for ..variables in j?

[tdhock]

this is related to #2988
I put a line of code in R/fmelt.R

    other.values = lapply(group.dt[, ..is.other], unique)

and when I checked data.table I got

* checking R code for possible problems ... NOTE
measure: no visible binding for global variable ‘..is.other’
Undefined global functions or variables:
  ..is.other

so I tried the usual trick,

..is.other <- NULL

But that gave me this warning from R/data.table.R,

            warning("Both '",name,"' and '..", name, "' exist in calling scope. Please remove the '..", name,"' variable in calling scope for clarity.")

Eventually I removed the two dots altogether and instead used,

    other.values = lapply(group.dt[, is.other, with=FALSE], unique)

which works fine (no NOTE nor warning), but from the docs and #2826 it seems that with=FALSE will be deprecated in the future. If that is the case then how should R package developers who use ..something avoid that NOTE?

[tdhock]

My 2c: seems that warning is useful in some cases, so removing it is undesirable. Instead, can we just NOT deprecate with=FALSE?

[jangorecki]

Also note there is pending PR #4304 which will give another interface for such queries.

[drag05]

I have silenced the Notes on R CMD check regarding "no visible binding for global variable ..a, ..b etc."
by taking the idea from PR# 4304 and set

..a = "..a"; ..b = "..b" inside my function. I have had to do the same with the special J.

The program runs fine but with the warning "both ..a and "..a" etc. are in the calling scope ....".

So I have had to set

options(warn = -1) ; on.exit(options(warn = 0), add = TRUE)

at the top of my function.

One other variable was an index set inside a DT defined as DT$id = seq.int(nrow(DT)) needed for joins. To my surprise the R CMD check did not interpret "id" in on = "id" as the name of column id and I had to set
"id" = names(DT$id) to make the check Note dissapear. Should I have also used the env = list() argument in DT as stated in the PR? I use version 1.14.0

Thank you!

[jangorecki]

@drag05 please post complete minimal example

[drag05]

@jangorecki : I will start with the fix on then, off. The variables are uv, ..uv and id

WITH THE FIX
.
.
#'@importFrom data.table fwrite
#'@importFrom data.table is.data.table
#'@importFrom data.table as.data.table
#'@importFrom data.table copy
#'@importFrom data.table setcolorder
#'@importFrom data.table ":="
#'@importFrom data.table ".SD"
.
.

f = function(dir, ..., save = TRUE) {
    options(warn = -1); on.exit((options(warn = 0)), add = TRUE)

     ....

        uv = unique(y0$vars)
      ..uv = "..uv"
     n.uv = length(uv)
    ....

    x0$id = seq.int(nrow(x0))
       "id" = names(x0$id)
    ....

        x = x0[, c('id', ..uv)]
       xr = x0[, -..uv]

    ....

    rez = results[[1L]]
     xx = rez[xr, on = 'id'][, id := NULL]
   ....

R CMD check output:
  ...

* checking R code for possible problems ... OK
* checking Rd files ... OK
* checking Rd metadata ... OK
* checking Rd cross-references ... OK
* checking for missing documentation entries ... OK
* checking for code/documentation mismatches ... OK
* checking Rd \usage sections ... OK
* checking Rd contents ... OK
* checking for unstated dependencies in examples ... OK
* checking installed files from 'inst/doc' ... OK
* checking files in 'vignettes' ... OK
* checking examples ... NONE
* checking for unstated dependencies in vignettes ... OK
* checking package vignettes in 'inst/doc' ... OK
* checking running R code from vignettes ...
  'introduction.Rmd' using 'UTF-8'... OK
 NONE
* checking re-building of vignette outputs ... OK
* checking PDF version of manual ... OK
* DONE



WITHOUT THE FIX:
.
.
#'@importFrom data.table fwrite
#'@importFrom data.table is.data.table
#'@importFrom data.table as.data.table
#'@importFrom data.table copy
#'@importFrom data.table setcolorder
#'@importFrom data.table ":="
#'@importFrom data.table ".SD"
.
.

f = function(dir, ..., save = TRUE) {
#    options(warn = -1); on.exit((options(warn = 0)), add = TRUE)

     ....

       uv = unique(y0$vars)
#    ..uv = "..uv"
     n.uv = length(uv)
    ....

    x0$id = seq.int(nrow(x0))
#    "id" = names(x0$id)
    ....

        x = x0[, c('id', ..uv)]
       xr = x0[, -..uv]

    ....

   rez = results[[1L]]
    xx = rez[xr, on = 'id'][, id := NULL]
   ....


R CMD check
* checking R code for possible problems ... NOTE
someFunction: no visible binding for global variable '..uv'
someFunction: no visible binding for global variable 'id'
Undefined global functions or variables:
  ..uv id
* checking Rd files ... OK
* checking Rd metadata ... OK
* checking Rd cross-references ... OK
* checking for missing documentation entries ... OK
* checking for code/documentation mismatches ... OK
* checking Rd \usage sections ... OK
* checking Rd contents ... OK
* checking for unstated dependencies in examples ... OK
* checking installed files from 'inst/doc' ... OK
* checking files in 'vignettes' ... OK
* checking examples ... NONE
* checking for unstated dependencies in vignettes ... OK
* checking package vignettes in 'inst/doc' ... OK
* checking running R code from vignettes ...
  'introduction.Rmd' using 'UTF-8'... OK
 NONE
* checking re-building of vignette outputs ... OK
* checking PDF version of manual ... OK
* DONE

Status: 1 NOTE

[jangorecki]

Naming your variable with double dot prefix is quite likely confuse reader or even interface. Double dot is used for looking up a variable in different scope. You can use new interface added in PR you mentioned to explicitly refer to variables and not rely on parent environment scope.

[drag05]

@jangorecki

I used double dot per description of "with" in data.table::data.table() because I needed to look for uv: uv = unique(y0$vars) defined outside the scope of x0.

Returning to the issue, please help me understand correctly:

The correct code is, for example:

colNames = "..uv"
xr = x0[,  - colNames, env = list(colNames = ..uv)]

instead of

..uv = "..uv"
xr = x0[, -..uv]

or, instead of:

..uv = "..uv"
xr = x0[, -..uv]
xr = x0[,  -..uv, env = list("..uv" = ..uv)]

UPDATE: After few trials, I got Error: "unused argument "env"..."

Thank you!

[jangorecki]

You can use verbose=TRUE to see how your query looks like after substitution from env argument.

Use of double dot prefix is designed to find variables named like double dot prefixed string, so defining a variable which has double dot in its name is not the correct use of it.

[iofeidis]

Hello!
Just came across the same R CMD check NOTE today.
I was able to eliminate it by using utils::globalVariables() function as instructed by Hadley Wickham in R Packages. All it does is that 'instructs' R CMD check to recognize these undeclared variables as global ones.
So, in case of @tdhock 's first comment, all I had to do is include the call:

## "zzz.R" script

.onLoad <- function(libname, pkgname){
    utils::globalVariables(c("..is.other"))
}

in my .onLoad() call in "R/zzz.R" file, that is being executed when the package is loaded with library().

Thus, the NOTE is eliminated, while with=FALSE is not used.

[jangorecki]

@iofeidis also please read our importing data.table vignette, using onLoad function should not be needed to register global variables.

[jangorecki]

@tdhock could you please check if using new nev argument instead will resolve your problem reported here?

[drag05]

@jangorecki

I have decided to go with

x[, .SD, .SDcols = ...]
with id = "id" = "J" = NULL

for now. Everything seems to be working, including the tests. No Notes or anything else.
Thank you for your time!

[tdhock]

the globalVariables() idea should work to suppress the NOTE I saw but is not function-specific so may lead to false negatives (no NOTE if same variable name used in other functions) and it is not used yet in data.table package.

[tdhock]

hi @jangorecki thanks for the suggestion to use env argument, but I do not think that is the best solution here. (correct me if I am mis-understanding something or using env sub-optimally though)
We currently have the following line in fmelt.R,

    other.values = lapply(group.dt[, is.other, with=FALSE], unique)

I changed that line of code to the following,

    other.values = lapply(group.dt[, is.other, env=list(is.other=is.other)], unique)

both are OK in terms of CRAN checks (no NOTE). So technically env argument works to avoid the NOTE. But using it in this way results in repeating the name is.other three times, which is potentially confusing to readers of the code. I would argue that using with=FALSE is simpler, less repetitive, and easier to understand.

[jangorecki]

Yes, agree in this case with makes more sense. I was just wondering if env can be used to fix the note in the original report. Good to hear it can.
As for the more neat use of this feature you could also write d[, .j, env=list(.j=is.other)].

[jpanfil]

Have any updates or official recommendations been made for this check NOTE due to ..?

[jangorecki]

Afaict no, probably because metaprogramming interface via .. is rather limited as of now. Extending it went into low priority area because env arg already covers all metaprogramming cases.