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

Patch base S3 dispatch table directly for R Notebook S3 overrides #2762

Merged
merged 1 commit into from May 9, 2018

Conversation

jmcphers
Copy link
Member

@jmcphers jmcphers commented May 9, 2018

R 3.5 includes a breaking change which prevents tables and HTML widgets from appearing in R Notebook chunks. The change is this (from NEWS):

S3 method lookup now searches the namespace registry after the top level environment of the calling environment.

In order to capture printed tables and HTML widgets, we create S3 methods in the tools:rstudio environment, which are used in favor of the base methods since they precede them on the search() path. However, starting in R 3.5, the namespace registry is searched first instead, so our S3 methods are never executed.

There no longer appears to be an elegant way to create short-lived S3 overrides of the kind used in the notebook, so with this change we take a longer route: we patch the S3 dispatch table directly (and temporarily) to use our methods. This should work for any R version, but for caution's sake we're continuing to use the known-good lower-touch mechanism in releases of R older than 3.5.

Fixes #2748.

@jmcphers jmcphers requested a review from kevinushey May 9, 2018 20:32

if (getRversion() < "3.5") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do the objects created by running this code get cached somewhere (and then reloaded for subsequent sessions)? (Just want to make sure that it's not possible for someone to end up with R 3.5 components when running with R 3.4)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I don't believe so. We source the module .R files, including this one, at the beginning of every session, whether new or resumed.

Copy link
Contributor

@kevinushey kevinushey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

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

Successfully merging this pull request may close these issues.

None yet

2 participants