Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Solve the knitr auto-printing problem by registering a method for knit_print #6589

Merged
merged 12 commits into from
Dec 5, 2024
1 change: 1 addition & 0 deletions NAMESPACE
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ if (getRversion() >= "4.0.0") {
# version of R (and that is checked in .onLoad with error if not).
export(.rbind.data.table) # only export in R<4.0.0 where it is still used; R-devel now detects it is missing doc, #5600
}
if (getRversion() >= "3.6.0") S3method(knitr::knit_print, data.table) # else manual delayed registration from the onLoad hook
S3method(dim, data.table)
S3method(dimnames, data.table)
S3method("dimnames<-", data.table)
Expand Down
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,8 @@ rowwiseDT(

14. Added a `data.frame` method for `format_list_item()` to fix error printing data.tables with columns containing 1-column data.frames, [#6592](https://github.com/Rdatatable/data.table/issues/6592). Thanks to @r2evans for the bug report and fix.

15. The auto-printing suppression in `knitr` documents is now done by implementing a method for `knit_print` instead of looking up the call stack, [#6589](https://github.com/Rdatatable/data.table/pull/6589). Thanks to @jangorecki for the report [#6509](https://github.com/Rdatatable/data.table/issues/6509) and @aitap for the fix.

## NOTES

1. Tests run again when some Suggests packages are missing, [#6411](https://github.com/Rdatatable/data.table/issues/6411). Thanks @aadler for the note and @MichaelChirico for the fix.
Expand Down
9 changes: 9 additions & 0 deletions R/onLoad.R
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,15 @@
lockBinding("rbind.data.frame",baseenv())
}
}
if (session_r_version < "3.6.0") { # corresponds to S3method() directive in NAMESPACE
# no delayed registration support for NAMESPACE; perform it manually
if (isNamespaceLoaded("knitr")) {
MichaelChirico marked this conversation as resolved.
Show resolved Hide resolved
registerS3method("knit_print", "data.table", knit_print.data.table, envir = asNamespace("knitr"))
}
setHook(packageEvent("knitr", "onLoad"), function(...) {
registerS3method("knit_print", "data.table", knit_print.data.table, envir = asNamespace("knitr"))
})
}

# Set options for the speed boost in v1.8.0 by avoiding 'default' arg of getOption(,default=)
# In fread and fwrite we have moved back to using getOption's default argument since it is unlikely fread and fread will be called in a loop many times, plus they
Expand Down
24 changes: 7 additions & 17 deletions R/print.data.table.R
Original file line number Diff line number Diff line change
Expand Up @@ -32,21 +32,8 @@ print.data.table = function(x, topn=getOption("datatable.print.topn"),
SYS = sys.calls()
if (length(SYS) <= 2L || # "> DT" auto-print or "> print(DT)" explicit print (cannot distinguish from R 3.2.0 but that's ok)
( length(SYS) >= 3L && is.symbol(thisSYS <- SYS[[length(SYS)-2L]][[1L]]) &&
as.character(thisSYS) == 'source') || # suppress printing from source(echo = TRUE) calls, #2369
MichaelChirico marked this conversation as resolved.
Show resolved Hide resolved
( length(SYS) > 3L && is.symbol(thisSYS <- SYS[[length(SYS)-3L]][[1L]]) &&
as.character(thisSYS) %chin% mimicsAutoPrint ) || # suppress printing from knitr, #6509
# In previous versions of knitr, call stack when auto-printing looked like:
# knit_print -> knit_print.default -> normal_print -> print -> print.data.table
# and we detected and avoided that by checking fourth last call in the stack.
# As of September 2024, the call stack can also look like:
# knit_print.default -> normal_print -> render -> evalq -> evalq -> print -> print.data.table
# so we have to check the 7th last call in the stack too.
# Ideally, we would like to return invisibly from DT[, foo := bar] and have knitr respect that, but a flag in
# .Primitive("[") sets all values returned from [.data.table to visible, hence the need for printing hacks later.
( length(SYS) > 6L && is.symbol(thisSYS <- SYS[[length(SYS)-6L]][[1L]]) &&
as.character(thisSYS) %chin% mimicsAutoPrint ) ) {
as.character(thisSYS) == 'source') ) { # suppress printing from source(echo = TRUE) calls, #2369
return(invisible(x))
# is.symbol() temp fix for #1758.
}
}
if (!is.numeric(nrows)) nrows = 100L
Expand Down Expand Up @@ -168,9 +155,6 @@ format.data.table = function(x, ..., justify="none") {
do.call(cbind, lapply(x, format_col, ..., justify=justify))
}

mimicsAutoPrint = c("knit_print.default")
# add maybe repr_text.default. See https://github.com/Rdatatable/data.table/issues/933#issuecomment-220237965

shouldPrint = function(x) {
ret = (identical(.global$print, "") || # to save address() calls and adding lots of address strings to R's global cache
address(x)!=.global$print)
Expand Down Expand Up @@ -303,3 +287,9 @@ trunc_cols_message = function(not_printed, abbs, class, col.names){
domain=NA
)
}

# Maybe add a method for repr::repr_text. See https://github.com/Rdatatable/data.table/issues/933#issuecomment-220237965
knit_print.data.table <- function(x, ...) {
if (!shouldPrint(x)) return(invisible(x))
NextMethod()
}
12 changes: 12 additions & 0 deletions tests/autoprint.R
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,15 @@ DT[1,a:=10L][] # yes. ...[] == oops, forgot print(...)
tryCatch(DT[,foo:=ColumnNameTypo], error=function(e) e$message) # error: not found.
DT # yes
DT # yes

# Regression test for auto-printing suppression in source(), #2369
local({
f = tempfile(fileext = ".R")
on.exit(unlink(f))
writeLines(c(
"library(data.table)",
"DT = data.table(a = 1)",
"DT[,a:=1] # not auto-printed"
), f)
source(f, local = TRUE, echo = TRUE)
})
18 changes: 18 additions & 0 deletions tests/autoprint.Rout.save
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,24 @@ NULL
1: 10
2: 10
>
> # Regression test for auto-printing suppression in source(), #2369
> local({
+ f = tempfile(fileext = ".R")
+ on.exit(unlink(f))
+ writeLines(c(
+ "library(data.table)",
+ "DT = data.table(a = 1)",
+ "DT[,a:=1] # not auto-printed"
+ ), f)
+ source(f, local = TRUE, echo = TRUE)
+ })

> library(data.table)

> DT = data.table(a = 1)

> DT[, `:=`(a, 1)]
>
> proc.time()
user system elapsed
0.223 0.016 0.231
Loading