Skip to content

Commit

Permalink
fix(R microkernel): Capture plots and avoid recursive print calls
Browse files Browse the repository at this point in the history
  • Loading branch information
nokome committed Dec 6, 2021
1 parent 3a24073 commit cfb5a42
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 7 deletions.
24 changes: 23 additions & 1 deletion rust/kernel-r/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ mod tests {
stencila_schema::Node,
KernelTrait,
};
use test_utils::{assert_json_eq, serde_json::json, skip_ci};
use test_utils::{assert_json_eq, print_logs, serde_json::json, skip_ci};

async fn skip_or_kernel() -> Result<MicroKernel> {
if skip_ci("Failing on Linux and MacOS CIs") {
Expand Down Expand Up @@ -249,4 +249,26 @@ mod tests {

Ok(())
}

#[tokio::test]
async fn encode_plots() -> Result<()> {
let mut kernel = match skip_or_kernel().await {
Ok(kernel) => kernel,
Err(..) => return Ok(()),
};

print_logs();

for code in ["plot(1)", "hist(rnorm(1000), breaks=30)"] {
let (outputs, messages) = kernel.exec(code).await?;
assert_json_eq!(messages, json!([]));
let image = match &outputs[0] {
Node::ImageObject(dt) => dt.clone(),
_ => bail!("unexpected type {:?}", outputs[0]),
};
assert!(image.content_url.starts_with("data:image/png;base64,"));
}

Ok(())
}
}
16 changes: 10 additions & 6 deletions rust/kernel-r/src/r-codec.r
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
if (!suppressPackageStartupMessages(require("jsonlite", quietly=TRUE)))
install.packages("jsonlite")
for (pkg in c("jsonlite", "base64enc")) {
if (!suppressPackageStartupMessages(require(pkg, character.only = TRUE, quietly=TRUE))) {
install.packages(pkg, quiet = TRUE)
suppressPackageStartupMessages(require(pkg, character.only = TRUE, quietly=TRUE))
}
}

# Decode JSON to a R value
decode_value <- function(json) {
Expand All @@ -23,7 +27,7 @@ convert_value <- function(value, options = list()) {
} else if (inherits(value, "table")) {
# The functions `summary` and `table` return class "table" results
# Currently, just "print" them. In the future, we may convert these to Datatables.
paste(utils::capture.output(print(value)), collapse = "\n")
paste(utils::capture.output(base::print(value)), collapse = "\n")
} else if (is.data.frame(value)) {
# Decode to a Datatable
convert_data_frame(value)
Expand All @@ -42,7 +46,7 @@ convert_value <- function(value, options = list()) {
value
} else {
warning(paste("Default conversion for R type:", typeof(value), ", class:", class(value)))
paste(utils::capture.output(print(value)), collapse = "\n")
paste(utils::capture.output(base::print(value)), collapse = "\n")
}
}

Expand All @@ -68,12 +72,12 @@ convert_plot <- function(value, options = list(), format = "png") {
units = "cm",
res = 150
)
print(value)
base::print(value)
grDevices::dev.off()

list(
type = unbox("ImageObject"),
contentUrl = unbox(paste0("data:image/", format, ";base64,", base64enc::base64encode(filename)))
contentUrl = unbox(paste0("data:image/", format, ";base64,", base64encode(filename)))
)
}

Expand Down
19 changes: 19 additions & 0 deletions rust/kernel-r/src/r-kernel.r
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ info <- function(msg) message(msg, "CodeInfo")
warning <- function(msg) message(msg, "CodeWarning")
error <- function(error, type = "RuntimeError") message(error$message, type)

# Default graphics device to avoid window popping up or `Rplot.pdf` polluting
# local directory. Recording must be enabled for print devices.
png(tempfile())
dev.control("enable")

stdin <- file("stdin", "r")
while (TRUE) {
code <- readLines(stdin, n=1)
Expand All @@ -28,6 +33,20 @@ while (TRUE) {
error(compiled, "SyntaxError")
} else {
value <- tryCatch(eval(compiled), message=info, warning=warning, error=error)

if (!withVisible(value)$visible) {
value <- NULL
}

rec_plot <- recordPlot()
if (!is.null(rec_plot[[1]])) {
value <- rec_plot
# Clear the existing device and create a new one
dev.off()
png(tempfile())
dev.control("enable")
}

if (!is.null(value)) {
last_line <- tail(strsplit(unescaped, "\\n")[[1]], n=1)
assignment <- grepl("^\\s*\\w+\\s*(<-|=)\\s*", last_line)
Expand Down

0 comments on commit cfb5a42

Please sign in to comment.