medium- and long-term plans for tests/knitr.R

[aitap]

• I slightly botched print.data.table: check for knitr at sys.calls()-6 #6563 by aiming it at patch-1.16.2 instead of master. Should we cherry-pick 9de2567 into master too? See also: Master diverges from patch 1.16.2 #6587
• Do we want a more reliable secret handshake with knitr? For example,

  • Have knitr set options(knitr.is.auto.printing = TRUE) and detect that in print.data.table

  • signalCondition(<please_dont_autoprint_me>) from print.data.table and have knitr detect that

  • If you want the exact printing behavior of data.table:::print.data.table and want the result to be a table, you can register an S3 method on xfun::record_print (data.table doesn't have to import xfun).

    Originally posted by @yihui in fuse does not work for quote yihui/litedown#38 (comment)

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

• Do we want to petition R developers to allow returning invisibly from [? The change is just one line, eval = 0 to eval = 200, but it does break R's own tests/reg-tests-2.Rout.save, making matrix(5:-6, 3)[2, invisible(3)] also invisible, and might break a lot of other stuff.

[aitap]

Given the new interest for #3029, perhaps we do want to ask R developers to make .Primitive("[") stop forcing visibility? The complete change that passes LC_ALL=C.UTF-8 make check-devel is below, but this may or may not break a lot of CRAN/Bioconductor packages. (Do we have or know someone who has the infrastructure to run R CMD check on a lot of R packages with two different versions of R?)

Index: src/main/names.c
===================================================================
--- src/main/names.c	(revision 87260)
+++ src/main/names.c	(working copy)
@@ -104,7 +104,7 @@
 {"(",		do_paren,	0,	1,	1,	{PP_PAREN,   PREC_FN,	  0}},
 {".subset",	do_subset_dflt,	1,	1,	-1,	{PP_FUNCALL, PREC_FN,	  0}},
 {".subset2",	do_subset2_dflt,2,	1,	-1,	{PP_FUNCALL, PREC_FN,	  0}},
-{"[",		do_subset,	1,	0,	-1,	{PP_SUBSET,  PREC_SUBSET, 0}},
+{"[",		do_subset,	1,	200,	-1,	{PP_SUBSET,  PREC_SUBSET, 0}},
 {"[[",		do_subset2,	2,	0,	-1,	{PP_SUBSET,  PREC_SUBSET, 0}},
 {"$",		do_subset3,	3,	0,	2,	{PP_DOLLAR,  PREC_DOLLAR, 0}},
 {"@",		do_AT,		0,	0,	2,	{PP_DOLLAR,  PREC_DOLLAR, 0}},
Index: src/main/subset.c
===================================================================
--- src/main/subset.c	(revision 87260)
+++ src/main/subset.c	(working copy)
@@ -697,6 +697,7 @@
 
     /* Method dispatch has failed, we now */
     /* run the generic internal code. */
+    R_Visible = TRUE;
     return do_subset_dflt(call, op, ans, rho);
 }
 
Index: tests/reg-S4.Rout.save
===================================================================
--- tests/reg-S4.Rout.save	(revision 87260)
+++ tests/reg-S4.Rout.save	(working copy)
@@ -738,7 +738,6 @@
 > x <- new("C1"); y <- new("C2")
 > x[1, drop=FALSE]
 drop in C1-[ : FALSE 
-NULL
 > y[1, drop=FALSE]
 drop in C2-[ : FALSE 
 drop in C1-[ : FALSE 

[ben-schwen]

Do we have or know someone who has the infrastructure to run R CMD check on a lot of R packages with two different versions of R?

side-eye to @tdhock and the NAU Monsoon cluster

[tdhock]

yes indeed we run all data table revdeps nightly on monsoon.

• using nightly R-devel
• and current R-release

For each version of R we run R CMD check on each revdep, using two different versions of data.table (master and CRAN), then report diff of revdep results (are there any new issues using master that were not there using CRAN?)

If I understand correctly, you want something different.

• R CMD check on all CRAN/Bioc instead of just data.table revdeps.
• Use two different versions of R, current R-devel and your patch.
• report diff of R CMD check results, between R-devel and your patch (are there any new issues using your patch that were not there using R-devel?)

this seems like it would be substantially different from the existing revdep check code that we have, so probably would take quite a bit of dev time to get it working.

we would have to mirror all CRAN https://cran.r-project.org/mirror-howto.html or bioc https://bioconductor.org/about/mirrors/mirror-how-to/

[tdhock]

I think Brian Ripley does this sort of thing, you may want to ask him. (or R-devel email list)

[aitap]

Thank you for the advice and detailed response, @tdhock! I can start a thread on R-devel if potentially removing shouldPrint and associated code in 8 years is considered worth the effort. FWIW, making this change in R will also require changing print.data.table to avoid breaking tests/autoprint.R:

> withVisible(DT[a %in% 4:8, a:=5L])$visible # used to be TRUE
[1] FALSE
> withVisible(DT) # not printing because previous auto-print not triggered
$value

$visible
[1] TRUE

[tdhock]

The R-devel thread is https://stat.ethz.ch/pipermail/r-devel/2024-October/083681.html

[ben-schwen]

AFAIAO modifying objects by reference is by definition a side-effect and therefore against the white book?

[aitap]

data.table does somewhat defy the conventional R semantics, but it's hard to argue with the results and the popularity that it brings to R. Someone else could have implemented a (slower, much less cross-compatible) environment-based data.table-alike in pure R (cf. "Extending R" by John M. Chambers, chapters 9-11) and have the same problem by allowing replacements using non-standard evaluation from inside the [ method (the class in "Extending R" instead disallows [[<- altogether and asks the user to call $edit(...) instead).