Add Python code chunks (& other language support) using RMarkdown language engines

[cassws]

(This is an early PR as a place to discuss these changes, but please let me know if another format for discussing/troubleshooting a PR is better @schloerke !)

As per #213 I am working on a PR to enable Python (and eventually more language) support in learnr. As proof of concept, I was able to modify exercise.R to manually set knitr::opts_chunk$set(engine='python') and test with code permissible in syntax in R and Python, but with different evaluation:

x = 8
x <-10
print(x)

As a code chunk in a learnr lesson now returns:

False
8

Indicating that this chunk runs with the Python engine. However, I was unable to set this on a chunk-by-chunk basis. I would like to use the language tag specified in the knitr code chunk (e.g. the python in {python exercise=TRUE...}) OR a new knitr hook ({r exercise=TRUE, exercise.engine='python'...}.

I ran into issues modifying knitr-hooks.R to pass along this information. I tried to pass along the engine name to preserved_options in order to accesspython via options$engine in exercise.R, but this failed. This may be my lack of understanding of how knitr-hooks work (OR they might be inappropriate for this task and another method would be preferable).

Question: is there another strategy for passing along and accessing the engine option for a code chunk within exercise.R? I feel like I'm missing an aspect of the interaction between code chunks -> knitr -> exercise.R. Note that we definitely will want to use this variable to e.g. introduce conditional syntax/last line checking as per #213 for implementing multi-language support robustly in learnr.

I'll be able to move on to working with code checking/etc. issues once I can access the engine parameter from within exercise.R. Thank you!

[schloerke]

@zoews

Thank you for looking into this!

is there another strategy for passing along and accessing the engine option for a code chunk within exercise.R?

Yes. Ideally, it would be great to use engine = 'python', but knitr will highjack that value for it's own purposes. Instead, we could make our own parameter, similar to exercise.checker.

So make sure this is passed along, we can make a similar line for exercise.engine as exercise.checker here:

learnr/R/knitr-hooks.R

Line 190 in 6518724

preserved_options$exercise.checker <- deparse(options$exercise.checker)

Then, in exercise.R where you are currently setting the engine, we could read the exercise.engine and set the knitr engine(defaulting to "R").

engine <- knitr::opts_chunk$get("exercise.engine")
if (!is.null(engine)) {
  knitr::opts_chunk$set(engine = engine)
}

The engine retrieval line could be moved to the top of the function (just above code checking) to be passed into the checker function.

[schloerke]

Making notes of things that should be addressed at some point (maybe not this PR)

• Browser auto complete
  • I am fine with turning it off for non-R based languages
• Would be good to display the execution language somewhere in the browser exercise UI
• Accessing engine specific setup chunks.
  • I am currently imagining this to follow the same paradigm that currently exists, but to use a global name of setup_ENGINE.
  • setup_R could be shimmed into the current design with setup being the first chunk executed. Ex: setup -> setup_ENGINE -> exercise_chunk-setup -> exercise_chunk
• Checker functions are passed the engine as a parameter
• Pass along all chunk options
  • mysql uses a connection chunk option. Many other languages have similar features. It would be good to pass along all remaining chunk options to the preserved chunk options to be able to be used at run time.

[cassws]

Ah so helpful!! Thank you @schloerke . I thought there may be some knitr intercepting going on... and I neglected to use deparse() in my first go-around. That makes a lot of sense.

I'll proceed keeping those features in mind. I noticed that tabs weren't functioning as I expected when writing Python code, so I may have to tackle a bit of the browser UI stuff along the way in the context of input. More soon!

[schloerke]

deparse shouldn't be necessary as we're only passing a value (vs wanting to know the function body). Sorry for unclear example.

---

Thanks! Feel free to push a "broken" PR and ask questions.

[cassws]

Hi @schloerke - had time to return to this today & a little more progress! We can now use exercise.engine to specify a language for a given chunk. As a test case, the code:

```{r r-not-python, exercise=TRUE, exercise.lines = 5}
# if running in R, this should return 10
# if running in Python, this should return FALSE and 5
x = 5
x <-10
print(x)
```

evaluates to 10, whereas

```{r python-not-r, exercise.engine="python", exercise=TRUE, exercise.lines = 5}
# if running in R, this should return 10
# if running in Python, this should return FALSE and 5
x = 5
x <-10
print(x)
```

evaluates to FALSE and 5 within the same document. So, no more hard-coding, at least for this syntactically-bilingual snippet!

Re: disabling pre/post checking and last line evaluation**

I couldn't easily move the engine assignment lines (219-221) as the restoring knitr options lines overwrote them. As I work-around, I set a language parameter for both checker calls just using exercise$options$exercise.engine for now (I think this would work?)

When I attempted to start disabling code checking or the last line evaluation specific code, e.g. by wrapping the segment you called out earlier in a giant is.null() statement, I wasn't successful.

Any advice for specifically which aspects of the checking to disable? I can continue poking around, but wanted to see if you had any quick guidance. Thank you!

[braunsb]

I am so happy to see these things happening! Thanks, @zoews and @schloerke !

[cassws]

Errors to overcome:

1. Error: Text to be written must be a length-one character vector (after clicking Run Code)

2. Red X box, hovering reveals "unexpected symbol" errors, won't allow clicking on Run Code

[schloerke]

Yes. Actually looking at this bug right now.

Can you pull in my branch so that we have a consistent tutorial to look at? It'll also be easier to collaborate too.

# pull in rstudio branch
git pull https://github.com/rstudio/learnr.git exercise_language

Allows us to run devtools::load_all(); run_tutorial("python", "learnr")

[kevinushey]

One of the larger issues here is that the Ace editor instances assume that the inner code is always R code, and so the code is always diagnosed as though it were R code. See:

learnr/inst/lib/tutorial/tutorial.js

Lines 4 to 16 in 6518724

	$(document).ready(function() {
	var tutorial = new Tutorial();
	// register autocompletion if available
	if (typeof TutorialCompleter !== "undefined")
	tutorial.$completer = new TutorialCompleter(tutorial);
	// register diagnostics if available
	if (typeof TutorialDiagnostics !== "undefined")
	tutorial.$diagnostics = new TutorialDiagnostics(tutorial);
	window.tutorial = tutorial;
	});

I think there needs to be some way of declaring the completion + diagnostics engines used for different programming languages (or just disabling them for now with Python code).

See also:

https://github.com/rstudio/learnr/blob/master/inst/lib/tutorial/tutorial-diagnostics.js
https://github.com/rstudio/learnr/blob/master/inst/lib/tutorial/tutorial-autocompletion.js

[schloerke]

@kevinushey Yes! Absolutely. Was also thinking of just disabling it for now.

For me, the blocker comes at evaluating/checking the code. If that can be solved, then we can update the UI.

[schloerke]

@zoews Fixed. Please pull again.

I should have some direction for you to look at here soon. Thank you for the help!

[schloerke]

@zoews I've added some more fixed. Please pull again.

Notes

• It really bugs me that we have to manually hand off each knitr chunk parameter to knitr. (There will always be another parameter that is created, so we will lose the current of manually passing them along battle.) Instead, I'd like for us to pass along all parameters given to the chunk. This may not work in the end as distinguishing between default and supplied chunks parameters might not be possible.
• We can worry about enabling/disabling the check handlers later.
• For now, we should put attention to the file that is created for exercise evaluation. Lines

  learnr/R/exercise.R

  Lines 229 to 254 in a2f48ca

  	# write the R code to a temp file (inclue setup code if necessary)
  	code <- c(exercise$setup, exercise$code)
  	exercise_r <- "exercise.R"
  	writeLines(code, con = exercise_r, useBytes = TRUE)
  	# spin it to an Rmd
  	cat("exercise.R:\n") # debug line
  	cat(readLines(exercise_r), sep = "\n") # debug line
  	exercise_rmd <- knitr::spin(hair = exercise_r,
  	knit = FALSE,
  	envir = envir,
  	format = "Rmd")
  	cat("exercise.Rmd:\n") # debug line
  	cat(readLines(exercise_rmd), sep = "\n") # debug line
  	# TODO - instead of sending raw R code to be interpeted as Rmd, create full Rmd chunks here using the contents of the original chunk.
  	# This would allow for dynamic options like the ones being supplied in `rmarkdown::knitr_options_html`
  	# create html_fragment output format with forwarded knitr options
  	knitr_options <- rmarkdown::knitr_options_html(
  	fig_width = exercise$options$fig.width,
  	fig_height = exercise$options$fig.height,
  	fig_retina = exercise$options$fig.retina,
  	# TODO - add extra options here. ex: `connection`
  	keep_md = FALSE
  	)

  I would be a happy camper if we can get a Rmd document created using chunks that are different languages. Such as r (global) setup chunk a python exercise.setup chunk and a sql exercise code chunk.
• Related to this, I would like to be able to use the engine directly (rather than using exercise.engine chunk parameter). I'll look into this issue.

[braunsb]

I'm following progress here with great interest. We have learnr modules available to a growing number of enthusiastic people. I do not have code-checking enabled for these reasons:

• I didn't have time to figure it out.
• I like people checking their own code against hints and solutions (gets them to focus on their answers multiple times).
• People can use exercise code chunks for playing around without generating error messages.
• People can find different, equally valid options for solving a given problem without generating error messages.

I'm pretty sure I could enable code checking of outcomes rather than of the code's text, but I like that people check their own work. It means they can see correct answers multiple times rather than just reviewing their errors and moving on right away when they get it right, without re-reading the correct code.

I should mention that most of our learners are highly self motivated. They are taking our classes because they really want to learn. There is no reason to have them "earn" anything other than knowledge. We gauge our success by how much learners think they have advanced towards meeting their own goals (have successfully completed an analysis on their own data sets, etc.).

In short, I'm fine with moving forward with multiple languages even if it means I can't check people's code.

Try http://www.a-mess.org/list/ for a list of our lessons. You're welcome to try them out and refer them to others, but WARNING: They will move to a new server as soon as we can make that happen. The link puts you on a a behind-the-scenes list that isn't meant as a landing page for the world, but you can tool around if you like to see the project @zoews and I are working on. The lessons include updates for swirl lessons and some statistics lessons based on the Biostatistics Handbook by John H. McDonald and the R Companion for it by Salvatore Mangiafico.

[cassws]

oh fantastic, thank you @schloerke! (and @kevinushey & @braunsb too!) I've pulled in changes from your branch and pushed them here as you can see.

FYI, if I try my two examples from earlier, I continue to see similar errors, but the wording is slightly different:

Was this your expectation that these chunks would still produce errors at this point? Or is this something strange continuing to happen?

In the meantime, as per your notes, I can look into the step where the chunks are created as an .Rmd for evaluation. I can see how that would be a helpful pattern to implement. Thanks for your good communication around this and please feel free to direct me to how I can be helpful!

[schloerke]

1. The error of <text>:3.5: unexpected ... is caused by knitr::spin. knitr::spin only understands R code. So if we can just bypass it to make exercise_rmd (and Rmd file with chunks), it should solve that issue.

2. This error is caused by the ace editor. I like @kevinushey's idea to disable the ace editor checking if we know it's a non-R language exercise. (Fixing the UI can happen later)

---

Thanks for your good communication around this and please let me know how I can be helpful!

Np! This is a really useful/highly requested feature!!

Being able to pull each other's branches solves a lot of issues. Thank you!

---

A more explicit example of the goal of how exercise_rmd lines should look using multiple languages... (Basically a direct copy of the original text. IDK where to find the original text, if it exists. ) The envir currently being passed to the knitr::spin call will need to be passed to rmarkdown::render when the file is executed.

```{python sql-not-r-setup}
x = 5
x <-10
print(x)
```

```{sql sql-not-r, connection=mammals, output.var='surveys'}
SELECT *
FROM `surveys`
LIMIT 10
```

[cassws]

@schloerke back working on this today FYI! I'll keep an eye on the exercise_language branch as I go. otherwise, I'll take a stab at no. 1 and 2 above right now

[cassws]

update: with @braunsb's contributions, we were able to circumvent the knitr::spin() call and manually create that .Rmd file. as a result, we can now execute Python code with Python syntax as per point no. 1 above:

Yay! Next on the list:

1. Disable Ace Editor calls (OR pass in language parameter?)
2. As per @braunsb, go from an !is.null() pattern for dealing with R vs non-R code chunks to explicitly setting R as engine. This will allow us to also specify supported or unsupported languages as we make more progress, and also prevent confusion for folks who DO set exercise.engine to R (which won't run with NULL === R logic)

I'll spend some time on these today.

@schloerke Thank you!

[cassws]

To be explicit... I want the unevaluated options being supplied here:

learnr/R/knitr-hooks.R

Line 49 in 3b230e1

knitr::opts_hooks$set(tutorial = function(options) {

which would then be stored just after here:

learnr/R/knitr-hooks.R

Line 195 in 3b230e1

preserved_options$engine <- "R"

This allows for sql chunks to keep the connection = connection text to be supplied to the Rmd chunk being created.

Hmm yes I can definitely see the idea of circumventing that pre-rendering layer to pass along connection, among other benefits.

However, I'm finding that I can't access connection var via the options in the opts_hook$set... function call you referenced starting at line 49. I printed out options at the start and end of that function. But even when running the {sql .... connections = ... } chunk from the new tutorial, I didn't see connection passing through.

I also tried passing along apreserved_options$grab_bag based on the contents of options at line 195, but same outcome - didn't find connection.

Is connection being tossed out earlier than any of these steps, or alternatively is at accessible from another object? I think I may be missing something -- I know we chatted about knitr throwing out parameters earlier in this PR thread. Thanks @schloerke !

[schloerke]

Oops! My fault. I meant the options from

learnr/R/knitr-hooks.R

Line 118 in 3b230e1

knitr::knit_hooks$set(tutorial = function(before, options, envir) {

.

I am looking at them using

        # other preserved_options  ~ line 195
        preserved_options$engine <- "R"
        str(options)
        browser()

...
$tutorial
[1] TRUE

$exercise.checker
function(...) {
    cat("REGULAR!!")
    vals <- list(...)
    cat("dots...\n")
    str(vals)
    cat("envir_result...\n")
    str(as.list(vals$envir_result))
    cat("envir_prep...\n")
    str(as.list(vals$envir_prep))
  }

$max.print
[1] 1000

$label
[1] "sql-not-r"

$exercise
[1] TRUE

$connection
<SQLiteConnection>
  Path: /Users/barret/Documents/git/rstudio/learnr/learnr/inst/tutorials/python/data_raw/portal_mammals.sqlite
  Extensions: TRUE

$output.var
[1] "surveys"

$out.width.px
[1] 624

$out.height.px
[1] 384

$code
[1] "SELECT *"       "FROM `surveys`" "LIMIT 10"      

$params.src
[1] "sql-not-r, exercise = TRUE, connection=mammals, output.var='surveys', engine=\"sql\""

attr(,"class")
[1] "knitr_strict_list"

[schloerke]

Is connection being tossed out earlier than any of these steps, or alternatively is at accessible from another object?

Yes. This is the sneaky part that learnr hides from its users... shiny-prerendered documents write everything to a file, which is then re-read at run time. The connection object is a s4 object, not an a basic value (atomics, vectors, and list).

Learnr is not able to serialize parameters freely. Ex:

dput(options$connection)
#> new("SQLiteConnection", ptr = <pointer: 0x7f8eee4ae4d0>, dbname = "./inst/tutorials/python/data_raw/portal_mammals.sqlite", 
#>     loadable.extensions = TRUE, flags = 70L, vfs = "", ref = <environment>, 
#>     bigint = "integer64")

The pointer above can not be recreated at runtime. So the connection object can not be recreated.

---

Hopefully what we'll be able to do is to capture the original parameters supplied and use them to create the exercise rmd file such as

exercise.Rmd

```{r setup, include=FALSE}
library(learnr)
tutorial_options(
  exercise.checker = function(...) {
    cat("REGULAR!!")
    vals <- list(...)
    cat("dots...\n")
    str(vals)
    cat("envir_result...\n")
    str(as.list(vals$envir_result))
    cat("envir_prep...\n")
    str(as.list(vals$envir_prep))
  }
)

# altered from https://datacarpentry.org/R-ecology-lesson/05-r-and-databases.html
if (!dir.exists("data_raw")) {
  dir.create("data_raw")
}
if (!file.exists("data_raw/portal_mammals.sqlite")) {
  download.file(url = "https://ndownloader.figshare.com/files/2292171",
                destfile = "data_raw/portal_mammals.sqlite", mode = "wb")
}
mammals <- DBI::dbConnect(RSQLite::SQLite(), "data_raw/portal_mammals.sqlite")
```

```{sql sql-not-r, connection=mammals, output.var='surveys'}
SELECT *
FROM `surveys`
LIMIT 10
```

The code above would just be a direct copy of the setup chunk, plus the original parameters of the exercise chunk (minus any learnr specific ones) with the body of the user submitted code.

[schloerke]

@yihui Is it possible to capture the unevaluated parameters of an rmarkdown chunk? I'm trying to pass through the parameters without the user having to do anything custom. Thank you!!

[cassws]

hi @schloerke I'm with you on using the pattern: pass along serielized/unevaluated parameters -> (bypass shiny pre-rendering) -> combine with code + setup to output chunks serially into .rmd file -> execute as we're already doing. I'm also not sure how to achieve that while e.g. resolving that sql connection pointer issue, so I'll pause here to see if @yihui is able to weigh in.

@schloerke in case this pattern is a showstopper/takes more upstream changes to implement/etc before SQL is supported: would you consider accepting the PR at current scope and moving this set of changes to a new PR? I'm working on the project that @braunsb mentioned and we would love to start using Python chunks in our lessons in the short term. if that's acceptable I can keep checking into that other PR as well. thank you!

[yihui]

@schloerke Sorry this thread is too long and I'm not sure if I get what you need, but connection can be a character string: https://github.com/yihui/knitr/blob/33d69c3/R/engine.R#L528-L529 You can definitely serialize a character string. Will that help?

[schloerke]

@yihui that should work for now.

I’ll catch you at the conf to hash out some ideas. Thank you, @yihui !

[yihui]

Sounds good!

[cassws]

Hey @schloerke -- happy 2020! I tested out supporting connection as a character string, and it works! I added it to preserved_options and changed the chunk parameters in the python test to be a string, and now that test has SQL, R, and Python running side by side.

This of course kicks the can down the road re: implementing a no-evaluation pattern for the chunks, but given that it works now with all three languages, wanted to present it as one possible option.

[braunsb]

@zoews this is remarkable! At the risk of sounding greedy—any chance of adding Bash?

[schloerke]

@zoews This is all great work. Unfortunately, I do not want to give it the full green light yet.

I'm happy to merge this PR into a standing "multiple engine" branch that you (and others) can make more PRs/improvements. Does this work for you?

Current thoughts:

• The exercise parameters still do not scale for other languages (ex: bash) without manual support.
  • Hopefully Yihui and I can figure this one out.
  • It may be that we just move all parameters like exercise.NAME to the generated Rmd as NAME.
• I would like to separate out each language into an S3 method for exercise_evaluate. This way if a new language comes along, they can have their own evaluation mechanism that doesn't depend upon the learnr being updated. (Ex: an R package, learnr.bash, with a function of learnr.bash::exercise_evaluate.bash which checks bash code.) .
  • I'm not fully sold on this idea either, but it opens up the door for secondary packages.
  • I do not know if S3 dispatch would work as expected within a forked evaluator.
• There are security concerns by using languages other than R.

[cassws]

@schloerke that makes a lot of sense! I think merging into a "multiple engine" branch, and giving more thought to the concerns you listed, is a great next step. Count me in.

In the meantime, our team will be building from my forked repo in order to deploy Python + other lessons in our modular learning environment in the next few weeks. I imagine this will provoke some further ideas/tweaks that I can propagate back to the multiple engine branch.

Thank you!

[schloerke]

Notes for me to look at... str(attr(knitr::knit_code$get(CHUNK_NAME), "chunk_opts"))

[vnijs]

Having learnr work with R, SQL, and Python would be amazing! @zoews Do you now have this working in your fork? Could you share some examples perhaps?

[vnijs]

Found it: https://github.com/zoews/learnr/blob/master/inst/tutorials/python/python.Rmd

Related question: Is it possible to change the mode to "python" for syntax highlighting and even the ace theme? Something like, the below would be great.

{python python-not-r, exercise=TRUE, exercise.lines = 5, mode = "python", theme = "cobalt"}

Would also love vim keybindings and keyboard short-cuts to run code, etc. as is available in https://github.com/trestletech/shinyAce

[schloerke]

@vnijs ... baby steps. There are still a lot of rough edges to fix. But, there is no reason to not generalize language support. We will need to ship multiple Ace languages, but will be achievable.

For vim support, feel free to make a new issue.

For theme support, can you make a new issue? A DOM data-value will need to be stored and these lines will need to be altered to look at the exercise theme DOM data value.

learnr/inst/rmarkdown/templates/tutorial/resources/tutorial-format.htm

Lines 232 to 240 in e72c1f9

	$if(ace-theme)$
	<script>
	$$(document).ready(function () {
	$$(".ace_editor").each(function(i, el) {
	window.ace.edit(el).setTheme("ace/theme/$ace-theme$")
	});
	});
	</script>
	$endif$

We're very aware of getting multi-language support working. Our push right now is to make sure it can execute in a safe/reproducible environment. Ex: See #345

[vnijs]

Thanks for the quick response @schloerke. I expect there is a lot of interest in tools like leanr as most school in the US have already determined that summer classes will be "virtual" and there will be a lot of international students with issues getting visas on time to be on campus in fall.

To get the extra components for python, sql, and vim I expect you would just need to add the below to update-ace.R (see also newly created issues as you suggested). Not sure where you store these files in the github repo. Do you graft them together into ace.js?

If there is anything (non-JS) I can do to help out please let me now. I'm planning to use some form of learnr (R-python-sql) + gradethis + submitr, hopefully in the form of a website of tutorials, for summer and fall classes so I have some motivation :)

"keybinding-vim.js",
"mode-sql.js",
"mode-python.js"

https://github.com/rstudio/learnr/blob/7a28826b9a6caaa3257602f75d626291cb8bce8b/tools/update-ace.R

[dputhier]

Dear all,
I'm really pleased to see that learnr could provide a support for additional languages (I'm particularly interested in Bash and Python support). Do you had any idea about the schedule for this feature being potentially integrated into the master branch.
I have already performed some tests for bash tutorials using this branch and I may be interested in going further if it may help.
Sorry if this highly selfish question and congrats for this job.

[schloerke]

@dputhier Yes, this feature is exciting!

@vnijs Good catch on the ace editor! I very am happy to showcase tutorials/vignettes of how to organize packages for teaching. A "real life" tutorial with all three packages would be amazing!

@nischalshrestha (and I) is currently working on a new approach (and also allowing for chained setup chunks; another big request!), learning from @zoews's branch. (We will be copying over a lot of the UI improvements already solved in this branch.)

When @nischalshrestha's branch is ready, I'll be sure to tag everyone interested! 😃😃

[schloerke]

Closing this PR in favor of #390 and #397. (I will try to cherry-pick as much as I can from this PR in #397.)

Thank you, @zoews!! 🙌 🙌

[cassws]

hey @schloerke just seeing this -- absolutely! so thrilled to see the new PRs -- long live polyglot learnr!