colnamesInt makes inscrutable errors

[MichaelChirico]

It's great that we've simplified internals to rely consistently on logic in colnamesInt(), but unfortunately it has made for some head-scratching debugging, e.g.:

call_neighbors[token == 'expr'][!exclude_parents, on = c('file', 'id')][grepl('/merge', file)]
Error in colnamesInt(i, unname(on), check_dups = FALSE) : 
  argument specifying columns specify non existing column(s): cols[2]='id'

I have enough context to basically figure out what's gone wrong, but including some context here as an extra helper would be useful, IMO.

What about adding an argument to colnamesInt where we can supply a hint about the source of the error?

A quick pass:

colnamesInt(..., source){
 ...
 stopf(call. = FALSE, "Column(s) not found in %s: cols[%d]='%s'", source, ...)
}

Then call colnamesInt(..., source = "i table's columns in on= join")

WDYT Jan? Somewhat off-the-cuff with no repro, LMK if an example is needed.

[Nj221102]

Hi , I have given it a try, here is what i have implemented

SEXP colnamesInt(SEXP x, SEXP cols, SEXP check_dups, char *source ) {
  if (!isNewList(x))
    error(_("'x' argument must be data.table compatible"));
  if (!IS_TRUE_OR_FALSE(check_dups))
    error(_("%s must be TRUE or FALSE, source = %a"), "check_dups", source);

and then call it intensionally with check_dups = null , so error occurs

 colnamesInt(x, names(on), check_dups=NULL, source ="x function in y file at line abc" );

Error in colnamesInt(x, names(on), check_dups = NULL, source = "x function in y file at line abc") : 
  unused argument (source = "x function in y file at line abc")

[MichaelChirico]

I recommend playing around with building/installing/testing the package for figuring out how to add features in the C code. pkgload::load_all() should work, or we also have cc() (see README) much more tested with data.table development specifically.

Immediately I see char *source but this will need to accept arguments from R, therefore the signature needs to be SEXP source and you'll have to extract the char array from the string object (e.g. CHAR(STRING_ELT(source, 0))); see this line e.g.:

data.table/src/utils.c

Line 138 in 1fb3c75

error(_("argument specifying columns received non-existing column(s): cols[%d]='%s'"), i+1, CHAR(STRING_ELT(cols, i))); // handles NAs also

That line also gives a hint to where I think source can best be used, e.g. "argument specifying columns" might be replaced with "In %s, argument specifying columns". We'll definitely want to check the existing colnamesInt() call sites to be sure whatever we come up with fits all the existing use cases too.

[katrinabrock]

Is there any advice for those hitting these "inscrutable errors"? I'm getting one of the errors you mentioned above:

Error in colnamesInt(x, names(on), check_dups = FALSE) : 
  argument specifying columns received non-existing column(s): cols[2]='v_time'

I guess it means something like that there is an undefined variable/column somewhere that I'm trying to use, but any other hints on where to look for the problem?

[jangorecki]

We could add argument to this internal function providing context in which it was called

[MichaelChirico]

We have draft PR #6008 for that. Exact approach for doing so is not exactly clear AIRI

[katrinabrock]

I figured out the problem. I'm not clear on the exact behavior of the draft PR, but not sure if a better error message would have actually clarified in my case. I think better explanation in the docs would have helped, so I created a separate issue for that: #6623