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

attempts to print tbl_dfs as paged tables in Notebooks fail with R 3.5. #2748

Closed
kevinushey opened this issue May 5, 2018 · 16 comments
Closed

attempts to print tbl_dfs as paged tables in Notebooks fail with R 3.5. #2748

kevinushey opened this issue May 5, 2018 · 16 comments
Labels
Milestone

Comments

@kevinushey
Copy link
Contributor

@kevinushey kevinushey commented May 5, 2018

System details

RStudio Edition : Desktop
RStudio Version : v1.2.602
OS Version      : macOS 10.13.4
R Version       : 3.5.0

Steps to reproduce the problem

Try running the following chunk in an R Notebook:

library(tibble)
as_tibble(mtcars)

Describe the problem in detail

The table is printed as 'plain' tibble output, rather than as a paged table.

Describe the behavior you expected

This should be printed as a paged table.

Diagnosis

From rstudio/rmarkdown#1331:

Something appears to be going awry with S3 dispatch in the Notebook context. One can see the issue by attempting to debug the print call in a Notebook chunk, using R 3.5.0 with RStudio v1.2:

```{r}
debugonce(print)
as_tibble(mtcars)
```

When running this, I see:

> debugonce(print)
> as_tibble(mtcars)
debugging in: function (x, ...) 
UseMethod("print")(x)
debug: UseMethod("print")
Browse[2]> class(x)
[1] "tbl_df"     "tbl"        "data.frame"
Browse[2]> print.tbl_df
function (x, ...) 
{
    o <- overrideMap(x, options)
    if (!is.null(o)) {
        overridePrint(o$x, o$options, o$className, o$nRow, o$nCol)
    }
}
<bytecode: 0x7fad5be20ec0>
<environment: 0x7fad44a37498>

So it appears like the S3 override we've registered should be in scope for this print call. However, this is not the case -- the S3 method registered in tibble is called instead.

Browse[2]> s
debugging in: print.tbl_df(x)
debug: {
    cat_line(format(x, ..., n = n, width = width, n_extra = n_extra))
    invisible(x)
}

The NEWS file of R 3.5.0 has this:

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

So it seems like R has explicitly switched up the mechanism used for S3 dispatch, and this has broken the way we override the S3 methods when injecting our own printers. We'll have to think about how to accommodate this change in behavior.

@kevinushey
Copy link
Contributor Author

@kevinushey kevinushey commented May 5, 2018

This might meet the bar for v1.1-patched just because it also implies the generated Notebook output is wrong (since v1.1 doesn't understand how to render console output with ANSI escape sequences)

@gregleleu
Copy link

@gregleleu gregleleu commented May 6, 2018

A similar issue happens with leaflet output: the output used to go below the window, below the chunk, now it always goes to the viewer (like in non-notebook mode).

library(leaflet)
leaflet() %>% 
  addCircleMarkers(
    data = breweries91
  )

Do you think it's the same issue?
If it make it v1.1-patched, what would be the timeframe?
Thanks!

sessionInfo:

R version 3.5.0 (2018-04-23)
Platform: x86_64-apple-darwin15.6.0 (64-bit)
Running under: macOS High Sierra 10.13.4

Matrix products: default
BLAS: /System/Library/Frameworks/Accelerate.framework/Versions/A/Frameworks/vecLib.framework/Versions/A/libBLAS.dylib
LAPACK: /Library/Frameworks/R.framework/Versions/3.5/Resources/lib/libRlapack.dylib

locale:
[1] en_US.UTF-8/en_US.UTF-8/en_US.UTF-8/C/en_US.UTF-8/en_US.UTF-8

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] sp_1.2-7           leaflet_2.0.0.9000

loaded via a namespace (and not attached):
 [1] Rcpp_0.12.16    lattice_0.20-35 digest_0.6.15   later_0.7.1     mime_0.5        grid_3.5.0      R6_2.2.2        jsonlite_1.5   
 [9] xtable_1.8-2    magrittr_1.5    promises_1.0.1  tools_3.5.0     htmlwidgets_1.2 crosstalk_1.0.0 shiny_1.0.5     httpuv_1.4.1   
[17] yaml_2.1.19     compiler_3.5.0  htmltools_0.3.6 knitr_1.20     

@jmcphers jmcphers added this to the v1.2 milestone May 7, 2018
@dfalty dfalty added the bug label May 7, 2018
@collinreinking
Copy link

@collinreinking collinreinking commented May 8, 2018

Does anyone have an idea on how we could force a "print as a paged table" while we wait on the bug fix?

@gregleleu
Copy link

@gregleleu gregleleu commented May 8, 2018

I’m using the group_by trick for the time being: you group by any of the columns before printing. It works in notebooks, haven’t tried it in knitted output.

It doesn’t fix the leaflet issue (again, if it’s the same problem).

If the fix is for v1.2, are we more talking a few weeks or 6 months? Just to know if I should rollback to 3.4.4

@kevinushey
Copy link
Contributor Author

@kevinushey kevinushey commented May 8, 2018

I can't give a timeframe, but this will be a pretty high priority bug so we hope to tackle it ASAP and get it into the daily builds + a preview release of RStudio in due course.

@jmcphers
Copy link
Member

@jmcphers jmcphers commented May 9, 2018

The leaflet issue is indeed the same problem; both are caused by the change in R 3.5. I've just made a fix for this which should be in the dailies shortly. We'll also backport it to 1.1 as soon as we've validated it, as without it R 3.5 users will not be able to use some notebook features.

@gregleleu
Copy link

@gregleleu gregleleu commented May 10, 2018

Thanks!
There's no compiled dailies, right? The way to get it would be to build from source?

@rstudio-build
Copy link

@rstudio-build rstudio-build commented May 10, 2018

@Jiang-Li-backup
Copy link

@Jiang-Li-backup Jiang-Li-backup commented May 10, 2018

I installed the latest packages and RStuido (Windows), but still has the same problem. Also, the table in Preview was not rendered. Thanks.

@rstudio-build
Copy link

@rstudio-build rstudio-build commented May 10, 2018

@Jiang-Li-backup
Copy link

@Jiang-Li-backup Jiang-Li-backup commented May 11, 2018

I tried the new windows build today, but the table in notebook was still not rendered...

@jmcphers
Copy link
Member

@jmcphers jmcphers commented May 11, 2018

@Jiang-Li Can you share the code that's reproducing the problem for you?

@Jiang-Li-backup
Copy link

@Jiang-Li-backup Jiang-Li-backup commented May 11, 2018

@jmcphers Oh it works when I removed the kable().

But previously, the kable also worked, for example:

library(knitr)
kable(cars)

@jmcphers
Copy link
Member

@jmcphers jmcphers commented May 16, 2018

@Jiang-Li Thanks, that's helpful. Today's stable version should work for you:

https://www.rstudio.com/products/rstudio/download/

@ronblum
Copy link
Contributor

@ronblum ronblum commented May 17, 2018

There's still an issue with kable() in 1.2.637-1 (on Mac, WIndows, and Ubuntu). The list is displayed as raw text, and ends up partially-hidden and unscrollable when viewed in-line.

@ronblum
Copy link
Contributor

@ronblum ronblum commented Jun 18, 2018

Verified in Windows 1.2.747 and Mac 1.2.747-3.

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

No branches or pull requests

8 participants