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

Print method for subclass makes := return visible output #3029

Closed
eliocamp opened this issue Sep 4, 2018 · 7 comments · Fixed by #6631
Closed

Print method for subclass makes := return visible output #3029

eliocamp opened this issue Sep 4, 2018 · 7 comments · Fixed by #6631

Comments

@eliocamp
Copy link
Contributor

eliocamp commented Sep 4, 2018

I'm extending data.table class and I'm having a problem. When defining a custom print method operations with := print the object instead of returning it invisibly.

library(data.table)

dt <- data.table(x = 1)

dt[, y := 1]  # No output. Correct!

setattr(dt, "class", c("someclass", class(dt)))

dt[, y := 1] # still no output. Yey!

print.someclass <- function(x, ...) {
  NextMethod("print")
  # maybe other things
}

dt[, y := 1]   # there's output, Noooo!

(The output doesn't seem to appear when reprexed, but it's there in interactive mode, please test)

I don't know what I'm doing wrong and I cannot find much documentation on this.

@MichaelChirico
Copy link
Member

@mattdowle I don't quite get what all is meant to be covered here:

length(SYS) > 3L && is.symbol(thisSYS <- SYS[[length(SYS)-3L]][[1L]]) &&
          as.character(thisSYS) %chin% mimicsAutoPrint

First thing that comes to mind is to look for NextMethod in SYS in addition to the mimicsAutoPrint... Or is there some way for downstream users to expand mimicsAutoPrint (on their own? Through the Issues tracker?)

@nikosbosse
Copy link

We're seeing a similar issue in our R scoringutils. Is there any solution or steps we could take? Thank you!

@ben-schwen
Copy link
Member

First thing that comes to mind is to look for NextMethod in SYS in addition to the mimicsAutoPrint... Or is there some way for downstream users to expand mimicsAutoPrint (on their own? Through the Issues tracker?)

I don't think that expanding mimicsAutoPrint with a curated list for every custom printing is suitable in the long run. Looking for NextMethod on second to last position in addition with length(class(dt)) seems like a viable approach to me

@MichaelChirico
Copy link
Member

MichaelChirico commented Dec 4, 2024

Looking for NextMethod on second to last position in addition with length(class(dt)) seems like a viable approach to me

Still feels pretty clunky to me -- does that distinguish explicit print(dt) on a class extension?

edit: no, it won't, c.f.:

print(dt)

# yields
sys.calls()
# [[1]]
# print(dt)

# [[2]]
# print.someclass(dt)

# [[3]]
# NextMethod("print")

# [[4]]
# print.data.table(dt)

It looks like the key to detecting the autoprint is this esoteric first entry in the call stack:

sys.calls()
# [[1]]
# (function (x, ...) 
# UseMethod("print"))(x)
# ...rest of stack...

That's just the print() generic being invoked in a substitute()-y way:

substitute(f(x), list(f = print))
# (function (x, ...) 
# UseMethod("print"))(x)

identical(sys.calls()[[1]][[1]], print)
# [1] TRUE

That identical() condition looks pretty good to me as a test of "auto-printed in interactive session" -- in particular explicit print() does not pass that, as sys.calls() looks like so in that case:

sys.calls()
# [[1]]
# print(dt)

# [[2]]
# print.data.table(dt)

Another idea is to add an argument to print.data.table() like suppressPrint=FALSE that downstreams could run to force print suppression, sent along through NextMethod().

@MichaelChirico
Copy link
Member

Hmm, is it considered intentional behavior that

print(as.data.table(mtcars)[,y:=2])

Does not print anything? I would guess no?

@eliocamp
Copy link
Contributor Author

eliocamp commented Dec 4, 2024

print(as.data.table(mtcars)[,y:=2]) is equivalent to as.data.table(mtcars)[,y:=2], since results are printed implicitly in the console, right? Since the expected behaviour is for := to suppress printing, then, yeah, it is intended.

@aitap
Copy link
Contributor

aitap commented Dec 4, 2024

The idea in #6631 is that what R does in order to auto-print as.data.table(mtcars)[,y:=2] is not exactly print(as.data.table(mtcars)[,y:=2]), but more like eval(substitute(print(x), list(print = base::print)), list(x = previously_obtained_value), globalenv()), and having the contents of base::print in the call stack instead of just quote(print) can be detected. The exact code is from R-3.5...3.6, but the behaviour before that was the same and dates back to R-3.2.0, so we should be fine depending on R ≥ 3.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants