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

WIP: R Notebook #394

Draft
wants to merge 79 commits into
base: master
Choose a base branch
from
Draft

Conversation

renkun-ken
Copy link
Member

@renkun-ken renkun-ken commented Aug 12, 2020

This is a very early, proof-of-concept demo of R Notebook using the VSCode Notebook API as suggested by #378.

The following tasks are initial steps to make the R notebook useful enough:

  • Read R notebook from Rmd
  • Define R notebook format (Notebook format #395)
  • Language server support in notebook code cells (Support notebook cells vscode-r-lsp#59)
  • R kernel (based on socketConnection)
  • Support executeCell (based on callr::r_session)
  • Support executeAllCells
  • Support cancelCellExecution (based on r_session$interrupt())
  • Support cancelAllCellsExecution
  • Plot support (based on svglite)
  • Display htmlwidgets in cell output
  • Handle browser request
  • Handle View() request
  • Display R error in cell output
  • Show R error traceback in cell error output traceback
  • Read/save full R notebook with cell outputs
  • Data frame and list viewer in cell output
  • Cross-platform check: R executable, etc.
  • Handle R kernel process crash

It seems there's ongoing API iteration recently and some adaptation is needed to fix the features already implemented.

Kapture 2020-08-29 at 0 25 35

Suggestions are welcome!

@andycraig
Copy link
Collaborator

Wow, this looks amazing!

I tried to run it in Windows but didn't have much luck so I'm posting the details.

Version without YAML header

Content of R Markdown document:

```{r}
print("hello")
```

I run command Notebook: Execute Notebook. Nothing changes in the notebook itself. Here is the output from the debug console:

R stderr (24952): Loading required namespace: jsonlite

extension.js:6336
R stdout (24952): Waiting

extension.js:6340
connected to server!
extension.js:6360
Send: {"time":1597228000521,"expr":""}

extension.js:6365
R stdout (24952): [1597228000521] 

extension.js:6340
{"type":"output","result":"NULL"}

extension.js:6374
disconnected from server
extension.js:6369
R stdout (24952): Waiting

Version with YAML header

Content of R Markdown document:

---
title: test
---

```{r}
print("hello")
```

When I do Notebook: Execute Notebook I get this output in the debug console:

R stderr (16720): Loading required namespace: jsonlite

extension.js:6336
R stdout (16720): Waiting

extension.js:6340
R stderr (16720): Error in socketConnection(host = "127.0.0.1", port = env$port, blocking = TRUE,  : 
  cannot open the connection
Calls: local ... eval.parent -> eval -> eval -> eval -> eval -> socketConnection
In addition: Warning message:

extension.js:6336
R stderr (16720): In socketConnection(host = "127.0.0.1", port = env$port, blocking = TRUE,  :
  problem in listening on this socket
Execution halted

extension.js:6336
R exited with code 1

The cell displays the message connect ECONNREFUSED 127.0.0.1:64110. Also the header shows 'title'.

header

Hope that helps, and let me know if it's because my setup is wrong or similar.

This is very exciting feature, I'm really looking forward to using it!

@andycraig
Copy link
Collaborator

The debug output for the version with the YAML header is probably because I was running it in the same session. If I run it in a fresh session, here's the debug console output I get:

R stderr (23076): Loading required namespace: jsonlite

extension.js:6336
R stdout (23076): Waiting

extension.js:6340
connected to server!
extension.js:6360
Send: {"time":1597229316281,"expr":"---\ntitle: test\n---"}

extension.js:6365
R stdout (23076): [1597229316281] ---
title: test
---

extension.js:6340
R stderr (23076): Error in parse(text = request$expr) : <text>:4:0: unexpected end of input
2: title: test
3: ---
  ^
Calls: local ... eval.parent -> eval -> eval -> eval -> eval -> parse
Execution halted

extension.js:6336
R exited with code 1

@renkun-ken
Copy link
Member Author

The demo is probably too early for any meaningful test. Nothing works at the moment. 🤦

@andycraig
Copy link
Collaborator

Ah, okay! Still very exciting 😃

@Ikuyadeu
Copy link
Member

Great works!
It will be a connector with python users and R users.

@renkun-ken
Copy link
Member Author

julia-vscode/julia-vscode#980 has a good discussion of the API and implementation details we might also benefit from.

@renkun-ken
Copy link
Member Author

renkun-ken commented Aug 28, 2020

@jrieken, I encounter another problem: When I delete any cell in the notebook, the language server receives a shutdown signal, and the language server is stopped. I'm wondering if it is intended behavior by sending shutdown request to language server on cell delete? Or am I missing something?

@renkun-ken
Copy link
Member Author

I guess it is caused by https://github.com/REditorSupport/vscode-r-lsp/pull/59/files#diff-45327f86d4438556066de133327f4ca2R215-L183 where I want to stop the langauge server for single file outside workspace. A notebook is a collection of TextDocument. Removing a cell closes the TextDocment and then stops the language server.

@renkun-ken
Copy link
Member Author

Fixed via REditorSupport/vscode-r-lsp@1a159d4.

@jrieken
Copy link

jrieken commented Aug 31, 2020

Yeah, whenever a cell is removed we emit a onDidCloseTextDocument-event. Your fix looks OK, tho it builds on the "internal" cell uri structure. We have also proposed TextDocument#notebook (see microsoft/vscode#102091) which would allow you to check on the notebook of a document directly - assuming a document references a notebook. We should maybe also add NotebookDocument#isClosed which would then allow you to have document.notebook?.isClosed

@renkun-ken
Copy link
Member Author

@jrieken Thanks!

@renkun-ken
Copy link
Member Author

I re-implement the RKernel and notebook.R using callr::r_session to support cancelling execution of cells.

@jrieken I've got a few questions before I move on.

  1. Regarding cancelAllCellsExecution. It seems Notebook: Cancel Notebook Execution command does not call cancelAllCellsExecution. I'm not sure how I could get it called?
  2. Regarding Display htmlwidgets in cell output. An htmlwidget produced in R is an HTML file that uses a number of local js and css files with relative paths. The source code looks like:

image

which looks like the following in web browser

image

I'm wondering if it is already supported in Notebook to show such HTML content properly in cell output? I tried replacing the resource paths to something like vscode-webview-resource:// (microsoft/vscode#106516) but it doesn't work.

@renkun-ken renkun-ken marked this pull request as draft November 3, 2020 15:59
@renkun-ken
Copy link
Member Author

A note: R_BROWSER could be used to allow callr session to launch browser as discussed at r-lib/callr#177.

@markbaas
Copy link
Contributor

markbaas commented Feb 3, 2021

@renkun-ken I'm playing around a bit with your code. For starters I rebased it to master: https://github.com/markbaas/vscode-R/tree/notebook-mark, perhaps you want to merge that part already ;)

@markbaas
Copy link
Contributor

markbaas commented Feb 5, 2021

@renkun-ken supports the view now. Did the trick with an iframe.
html output is also supported. But i'm rewriting this. Any chance you can a have a look at my progress?

@renkun-ken
Copy link
Member Author

Good to see you continue working on the notebook feature! I'll take a look when possible.

@markbaas
Copy link
Contributor

markbaas commented Feb 9, 2021

@renkun-ken @jrieken

  • proper implementation of table output with a custom renderer
  • proper implementation of the viewer output through a custom renderer + iframe

prent
prent
also supports raw data
prent

@markbaas
Copy link
Contributor

markbaas commented Feb 9, 2021

julia-vscode/julia-vscode#980 has a good discussion of the API and implementation details we might also benefit from.

About saving output I see we have a few options.

  1. Like the html_notebook output saving output in a rendered html file. But this would divert from the vscode structure.
  2. Save output in a separate json file. Benefit here is that we can just put in the raw response that we get from the notebook kernel. Probably it would also be beneficial to keep an .Rdata file with the current workspace, so you can continue working where you left off.

@renkun-ken what approach would be preferred?

@renkun-ken
Copy link
Member Author

Thanks for the work, @markbaas!

Save output in a separate json file. Benefit here is that we can just put in the raw response that we get from the notebook kernel. Probably it would also be beneficial to keep an .Rdata file with the current workspace, so you can continue working where you left off.

I think this makes good sense and might better fit more scenarios.

@markbaas
Copy link
Contributor

@jrieken

why is the are the outputs and metadata properties deprecated for reading as well. I understand this is the case for writing, but for reading its a convenient way to get the metadata for the specific cell and I see no other way.

		outputs: CellOutput[];
		// readonly outputs2: NotebookCellOutput[];
		/** @deprecated use WorkspaceEdit.replaceCellMetadata */
		metadata: NotebookCellMetadata;

@markbaas
Copy link
Contributor

@renkun-ken Added support for restoring saved cells... only that it doesn't work. I have no clue what the problem is. Perhaps a bug in the api.

@Tal500
Copy link
Contributor

Tal500 commented Jul 17, 2023

Looks awesome! Any update on this?

@GitHunter0
Copy link

This is a great initiative.
Have you thought about join forces with RStudio team to further develop their Quarto Notebook VScode extension?
I think (hope) Quarto format is the future of notebooks.

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.

7 participants