Ivan Krylov
2024-Oct-24 12:48 UTC
[Rd] Could .Primitive("[") stop forcing R_Visible = TRUE?
Hello, The "[" primitive operator currently has the 'eval' flag set to 0 in src/main/names.c. This means that the result of subsetting, whether R-native or implemented by a method, will never be invisible(). This is a very reasonable default: if the user goes as far as to subset a value, they probably want to see the result. Unfortunately, there also exists at least one counter-example to that: data.table's modification by reference using the `:=` operator from inside the `[` operator. If a user creates a data.table object `x` and evaluates x[,foo := bar], the desired outcome is to return x invisibly, both to allow chained updates by reference (x[,foo := bar][,bar := baz]) and to avoid cluttering the screen by printing the whole object after updating a few columns. Since .Primitive("[") forces visibility on, the data.table developers had to come up with their own visibility flag [1] and check it from inside the print() method when it looks like it originates from auto-printing [2]. Since the auto-printing detection works by looking at the call stack, this recently broke after a knitr update (but can be reliably repaired [3]) and doesn't work for sub-classes of data.table [4]. Is it feasible for R to consider allowing methods for `[` to set their own visibility flag at this point? The change is deceptively small: set 'eval' to 200 in names.c and R_Visible = TRUE before returning from the non-method-dispatch branch in do_subset(). This results in one change in the saved output of R's own tests/reg-S4.R [5]. Or is the potential breakage for existing code too prohibitive? -- Best regards, Ivan [1] https://github.com/Rdatatable/data.table/blob/e5b845e5cbc6be826558d11d601243240abe7a72/R/print.data.table.R#L164-L169 [2] https://github.com/Rdatatable/data.table/blob/e5b845e5cbc6be826558d11d601243240abe7a72/R/print.data.table.R#L24-L41 [3] https://github.com/Rdatatable/data.table/pull/6589 [4] https://github.com/Rdatatable/data.table/issues/3029 [5] A method for `[` that runs cat() used to return NULL visibly.
Toby Hocking
2024-Oct-24 17:23 UTC
[Rd] Could .Primitive("[") stop forcing R_Visible = TRUE?
Thanks for the detailed analysis and proposition Ivan. The patch you are proposing to base R is https://github.com/Rdatatable/data.table/issues/6566#issuecomment-2428912338 right? On Thu, Oct 24, 2024 at 8:48?AM Ivan Krylov via R-devel <r-devel at r-project.org> wrote:> > Hello, > > The "[" primitive operator currently has the 'eval' flag set to 0 in > src/main/names.c. This means that the result of subsetting, whether > R-native or implemented by a method, will never be invisible(). > > This is a very reasonable default: if the user goes as far as to subset > a value, they probably want to see the result. Unfortunately, there > also exists at least one counter-example to that: data.table's > modification by reference using the `:=` operator from inside the `[` > operator. > > If a user creates a data.table object `x` and evaluates x[,foo := bar], > the desired outcome is to return x invisibly, both to allow chained > updates by reference (x[,foo := bar][,bar := baz]) and to avoid > cluttering the screen by printing the whole object after updating a few > columns. Since .Primitive("[") forces visibility on, the data.table > developers had to come up with their own visibility flag [1] and check > it from inside the print() method when it looks like it originates from > auto-printing [2]. Since the auto-printing detection works by looking > at the call stack, this recently broke after a knitr update (but can > be reliably repaired [3]) and doesn't work for sub-classes of > data.table [4]. > > Is it feasible for R to consider allowing methods for `[` to set their > own visibility flag at this point? The change is deceptively small: set > 'eval' to 200 in names.c and R_Visible = TRUE before returning from the > non-method-dispatch branch in do_subset(). This results in one change > in the saved output of R's own tests/reg-S4.R [5]. Or is the potential > breakage for existing code too prohibitive? > > -- > Best regards, > Ivan > > [1] > https://github.com/Rdatatable/data.table/blob/e5b845e5cbc6be826558d11d601243240abe7a72/R/print.data.table.R#L164-L169 > > [2] > https://github.com/Rdatatable/data.table/blob/e5b845e5cbc6be826558d11d601243240abe7a72/R/print.data.table.R#L24-L41 > > [3] > https://github.com/Rdatatable/data.table/pull/6589 > > [4] > https://github.com/Rdatatable/data.table/issues/3029 > > [5] A method for `[` that runs cat() used to return NULL visibly. > > ______________________________________________ > R-devel at r-project.org mailing list > https://stat.ethz.ch/mailman/listinfo/r-devel
Possibly Parallel Threads
- Could .Primitive("[") stop forcing R_Visible = TRUE?
- Could .Primitive("[") stop forcing R_Visible = TRUE?
- Could .Primitive("[") stop forcing R_Visible = TRUE?
- Could .Primitive("[") stop forcing R_Visible = TRUE?
- [External] Re: Could .Primitive("[") stop forcing R_Visible = TRUE?