Let different cells use the same graphics device#749
Let different cells use the same graphics device#749ma1112 wants to merge 1 commit intoIRkernel:masterfrom
Conversation
This PR resolves issues/240, and as a side effect it also resolves issues reporting various behaviour when plotting with IRKernel, e.g. also issues/722. The goal of this PR is to let different cells use the same graphics device. I.e. when one plots in one cell e.g. with the `plot` command and then enhances the plot in the following cell e.g. with `lines`, the cumulative effect of those two commands should be visible in the output of the former cell. `IRKernel` calls `evaluate` to run R code cell by cell, and `evaluate` creates a new graphics device for each function call. See [the evaluate source code](https://github.com/r-lib/evaluate/blob/8d2bb1f353b878178b042d36ddcff2cca4a2be5e/R/evaluate.R#L77) With this change, `IRKernel` now creates its own graphics device and calls `evalute()` with `new_device = FALSE.` Also, in order to capture plots properly, `IRKernel` now executes `dev.control(displaylist = "enable")`, similarly to `evaluate` that executes this for each kernel it creates ([code](https://github.com/r-lib/evaluate/blob/8d2bb1f353b878178b042d36ddcff2cca4a2be5e/R/watchout.R#L9C5-L9C40)) With this change, if one creates a plot in one cell, `evaluate` will call `handle_graphics()` in all subsesquent cells, even if the cell does not plot anyhting. To handle that, guardrails are applied in order not to send plots multiple times to the UI, in a from of introducing the boolean variable `last_recorded_plot_sent`. Moreover, as a side effect, `IRKernel`'s own device settings are now taking effect, that has been hidden by `evalaute`'s default `new_device` option that has set the device to `pdf` for each execution. See the IRkernel code [here](https://github.com/IRkernel/IRkernel/blob/124f2347f15cfadeabc6738868e05e6087f4c456/R/environment_shadow.r#L121) and evaluate's code [here](https://github.com/r-lib/evaluate/blob/8d2bb1f353b878178b042d36ddcff2cca4a2be5e/R/watchout.R#L8).
There was a problem hiding this comment.
To be honest I do not really understand what is the reason for sending the plot here, as well as after we call evaluate(), so I tried keeping the same logic with the new re-useable graphics devices.
| } | ||
|
|
||
| if (is.null(prev) && !is.null(current)) { | ||
| return(!require_exact_length) |
There was a problem hiding this comment.
can you explain the logic here?
There was a problem hiding this comment.
if require_exact_length is set to false, the function behaves as the former version of plot_builds_upon. Specifically, in this branch is.null(prev) is true, so the function returns TRUE.
if require_exact_length is set to true, the goal of the function is to check if the two plots are exactly the same.
If prev is null but current is not null, they can not be the same and therefore we return FALSE.
I am open to suggestions on how to rewrite the function to make it more readable. Maybe renaming require_exact_length to require_exact_match or explicitly writing out the logic here would help more?
E.g.
if (is.null(prev) && !is.null(current)) {
if(require_exact_length) {
return FALSE
} else {
return TRUE
}
```
|
@flying-sheep I know you said you had little time to review PRs but I'm wondering if you might find some time to look at this PR, or at least set some expectations on when you might have a chance to take a look. |
2 participants
This PR resolves #240 , and as a side effect it also resolves issues reporting various behaviour when plotting with IRKernel, e.g. also #722.
The goal of this PR is to let different cells use the same graphics device. I.e. when one plots in one cell e.g. with the
plotcommand and then enhances the plot in the following cell e.g. withlines, the cumulative effect of those two commands should be visible in the output of the former cell.IRKernelcallsevaluateto run R code cell by cell, andevaluatecreates a new graphics device for each function call. See the evaluate source codeWith this change,
IRKernelnow creates its own graphics device and callsevalute()withnew_device = FALSE.Also, in order to capture plots properly,
IRKernelnow executesdev.control(displaylist = "enable"), similarly toevaluatethat executes this for each kernel it creates (code). This is needed to capture the effects of multiple plotting functions applied on the same device.By re-using the same graphics device, if one creates a plot in one cell,
evaluatewill callhandle_graphics()in all subsesquent cells, even if the cell does not plot anything. To handle that, guardrails are applied in order not to send plots multiple times to the UI, in a from of introducing the boolean variablelast_recorded_plot_sent.Moreover, as a side effect,
IRKernel's own device settings are now taking effect, that has been hidden byevalaute's defaultnew_deviceoption that has set the device topdffor each execution. See the IRkernel code here and evaluate's code here.