MUCollective / multiverse

R package for creating explorable multiverse analysis
https://mucollective.github.io/multiverse/
GNU General Public License v3.0
62 stars 5 forks source link

Compiling multiverse notebooks does not output code correctly #101

Closed abhsarma closed 2 months ago

abhsarma commented 2 years ago

Consider the following multiverse:

First we define a branch with a transformation and then print it

```{multiverse default-m-1, inside = M}
df = hurricane_data %>%
   mutate(damage = branch(damage_transform,
            "no_transform" ~ identity(dam),
            "log_transform" ~ log(dam)
        ))

df$damage

Next we print the the same variable in a distinct multiverse code block

````r
```{multiverse default-m-2, inside = M}
df$damage


The first one is going to show the correct output but the second one is not. This is because how [knitr handles R execution](https://github.com/yihui/knitr/blob/694f999aad2f24e231d1ff850db2c60a19ca5d28/R/block.R#L171), it assumes the environment to be `knit_global()` --- thus in the first code block df is being (re-)defined and printed N times, but in the second one it is just taking the last definition (which in this case is the log transformed data.

One way to solve this would be to make another PR to `knitr` to add an optional `env` argument to the `eng_r` function. Some potential issues would be if `eng_r` is performing some lookup of previously defined variables in the `globalenv()` which it seems like it is (but not sure)

I am not sure if there's a simpler solution to this that I am missing?
mjskay commented 2 years ago

Hmm yeah that's annoying. I assume the short-term fix is our own implementation of eng_r() in the multiverse package again? You could do that for now and then once you're sure everything's working do a knitr PR.

abhsarma commented 2 years ago

Yeah that's what I did and it seems to solve things for now, but I'll check for more examples to see if this is sufficient to fix the problem

abhsarma commented 2 years ago

weird, seems like figures are handled differently. In the following code block, the first line produces the correct output, the second line does not:

```{multiverse default-m-2, inside = M}
df$damage
hist(df$damage)
mjskay commented 2 years ago

is this related to all the crap we had to do to get figures working interactively way back when? I recall digging into this then but I don't remember the solution

abhsarma commented 2 years ago

From what I could understand, I don't think so? I think the problem right now is that the correct data variable cannot be accessed. The problem previously was that the figure/plot outputs were not shown, and the solution we had implemented previously (if I remember correctly) used evaluate and it just needed the new_device argument to be set to FALSE. When the current eng_r is executed, that is specified already.

mjskay commented 2 years ago

K I don't think I have enough context to understand the issue at this point. The old problem I had to solve had to do with how figure output was handled differently from console output, and that was the whole reason for using evaluate() and whatnot.

Best advice I can give here without seeing output is "have you stepped through it with a debugger?"

abhsarma commented 2 years ago

I created a fork of knitr which fixes these issues---the file from knitting produces the correct output. The problem seems to be coming from these lines, which I have commented out for now---but I am not sure what this does or how to get around the problem without simply removing this.

Also unable to test the forked project :/

@mjskay any thoughts?

abhsarma commented 2 years ago

Created a fork of knitr which potentially solves this problem. Need to check if knitr tests pass before submitting a PR

abhsarma commented 2 months ago

As discussed in #121 , this issue has been addressed by distinguishing between compiling as an EMAR vs compiling as a regular html_document, and the previous commit