Repository navigation
Unintuitive warning when referencing a value that doesn't exist during creation of new column in J #5648
Description
Activity
- changed the title
[-]Unintuitive error when referencing a value that doesn't exist during creation of new column in J[/-][+]Unintuitive warning when referencing a value that doesn't exist during creation of new column in J[/+]on Dec 6, 2023 Since the warning is thrown in
assign.c
Line 433 in 3e590f8
and used in several functions, e.g.set,letand:=, I would favor something like:Tried to assign NULL to column 'col3', but column 'col3' does not exist to remove.
Reacted by Jan GoreckiAs far as I understand, to fix this issue I would just have to change the warning in
assign.cand change an old test as well as maybe add a new test. If so, I'll start a PR, or if there are any other changes required to make this change please advise. Thanks!SGTM! If changing the message breaks a test, just fix that test, no need for a new one. If it doesn't break any test, please add a new one.
SGTM! If changing the message breaks a test, just fix that test, no need for a new one. If it doesn't break any test, please add a new one.
RESOLVED
Hmm... A test (316) failed but it's not failing as I expected. It sends a bunch of C code and ends with a segfault. Even if I change the failing test to have the proper expected output this error occurs. Are there any other places where the C code is tested where I might have to change a test?
Error printout:
Traceback:
1:[.data.table(DT, ,:=(c, NULL))
2: DT[,:=(c, NULL)]
3: doTryCatch(return(expr), name, parentenv, handler)
4: tryCatchOne(expr, names, parentenv, handlers[[1L]])
5: tryCatchList(expr, classes, parentenv, handlers)
6: tryCatch(x, error = eHandler)
7: withCallingHandlers(tryCatch(x, error = eHandler), warning = wHandler, message = mHandler)
8: withCallingHandlers(expr, message = function(c) if (inherits(c, classes)) tryInvokeRestart("muffleMessage"))
9: suppressMessages(withCallingHandlers(tryCatch(x, error = eHandler), warning = wHandler, message = mHandler))
10: test(316, DT[,:=(c, NULL)], data.table(a = 20:22, b = 23:25), warning = "Tried to assign NULL to column 'c', but column 'c' does not exist to remove")
11: eval(exprs[i], envir)
12: eval(exprs[i], envir)
13: sys.source(fn, envir = env)
14: doTryCatch(return(expr), name, parentenv, handler)
15: tryCatchOne(expr, names, parentenv, handlers[[1L]])
16: tryCatchList(expr, classes, parentenv, handlers)
17: tryCatch(expr, error = function(e) { call <- conditionCall(e) if (!is.null(call)) { if (identical(call[[1L]], quote(doTryCatch))) call <- sys.call(-4L) dcall <- deparse(call, nlines = 1L) prefix <- paste("Error in", dcall, ": ") LONG <- 75L sm <- strsplit(conditionMessage(e), "\n")[[1L]] w <- 14L + nchar(dcall, type = "w") + nchar(sm[1L], type = "w") if (is.na(w)) w <- 14L + nchar(dcall, type = "b") + nchar(sm[1L], type = "b") if (w > LONG) prefix <- paste0(prefix, "\n ") } else prefix <- "Error : " msg <- paste0(prefix, conditionMessage(e), "\n") .Internal(seterrmessage(msg[1L])) if (!silent && isTRUE(getOption("show.error.messages"))) { cat(msg, file = outFile) .Internal(printDeferredWarnings()) } invisible(structure(msg, class = "try-error", condition = e))})
18: try(sys.source(fn, envir = env), silent = silent)
19: test.data.table()
An irrecoverable exception occurred. R is aborting now ...
Segmentation faultI'll do some digging with the traceback for now.
Found the issue... Little bit rusty with C, obvious change, just forgot to pass an extra column name even after adding an extra
'%s'to the warning, so it would segfault. Sorry about that!Found the issue... Little bit rusty with C, obvious change.
Please describe the issue+solution in your edit! You never know when someone will stumble upon this comment in the future facing the same issue and you could wind up helping them.
Reacted by Joshua Wu
This is hard to describe in words so here is a simple reprex:
Because there is no
col2at all the value retrieved isNULL, which gets interpreted as trying to deletecol3rather than create it. In isolation this is a pretty obvious "duh" moment but in practice the reason this is going to happen in the first place is because someone's accidentally lost track of what order they've done things in.I think a minor tweak to the error message would be a big help for anyone who's gotten themselves mixed up like that: