In-chunk options support #82

maelle · 2022-12-06T14:53:57Z

For now this writes all in-chunk options back to YAML format so if we go with this the README will need a note about this.

maelle · 2022-12-08T14:48:28Z

R/to_xml.R

+
+  code <- strsplit(xml2::xml_text(code_block), "\n")[[1]]
+  inchunk_info <- knitr::partition_chunk(info[["language"]], code)
+  xml2::xml_text(code_block) <- paste0(inchunk_info$code, "\n")


do not forget this newline 🔥

One of the things I'm wondering about 🤔 is it worth it for us to strip these comments from the source?

what do you mean? I think it makes sense because the user might change the options via the XML so we need to write the in-chunk options again.

That's a good point. I was thinking a situation where the user may want to know how many lines of code they have including options. But you are right. It would be more complex to retain the chunk option formatting in the first place.

should this go into a feature where we transform code chunks info into a tibble? I mean adding a column with number of code lines.

maelle · 2022-12-08T14:49:11Z

tests/testthat/_snaps/chunk-options/chunk-options.Rmd

+```{r, message=FALSE}
+#| echo: FALSE
+#| fig.width: 10
+#| fig.cap: This is a long caption.


maybe that's somewhere where I need to add more YAML-ing as the absence of quote could be problematic?

I think we can use the yaml package to leverage the yamling because the yaml we will encounter can often be complex and nested:

see https://fosstodon.org/@zkamvar/109423877203504343:

#| fig.alt: #| - scatter plot of 100 #| random points from #| a normal distribution. #| - approximately normal #| histogram from the same #| 100 points. #| - Q-Q plot against the #| normal distribution showing #| a near diagonal line. x <- rnorm(100) plot(1:100, x) hist(x) qqnorm(x)

and https://quarto.org/docs/computations/julia.html#multiple-outputs:

#| label: fig-plots #| fig-cap: Multiple Plots #| fig-subcap: #| - "Plot 1" #| - "Plot 2" #| layout-ncol: 2 using Plots display(plot(sin, x -> sin(2x), 0, 2)) display(plot(x -> sin(4x), y -> sin(5y), 0, 2))

I think it is solved? see the julia chunk below

maelle · 2022-12-08T14:49:38Z

tests/testthat/_snaps/to_md/to_md-works-for-Rmd.Rmd

@@ -33,7 +33,7 @@ plot(pressure)

 Non-RMarkdown blocks are also considered

-```{julia, info=bash}
+```{julia}


actually I am not sure why it used to be "info=bash"?

I just found why:

I was trying to test the integrity of the object (e.g. that it was copied before writing instead of transformed in place). I did not consider the fact that I was overwriting a markdown code block within the chunks:

tinkr/tests/testthat/test-to_md.R

Lines 98 to 110 in 60d3573

# One block with info

info_attr <- xml2::xml_attr(blocks, "info")

expect_equal(sum(!is.na(info_attr)), 1)

xml2::xml_set_attr(blocks, "language", "julia")

# save back and have a look

to_md(yaml_xml_list, newmd)

expect_snapshot_file(newmd)

# Still one block with info after writing (the process has not clobbered things)

info_attr <- xml2::xml_attr(blocks, "info")

expect_equal(sum(!is.na(info_attr)), 1)

That being said, I think the behaviour of having the 'info' propagate does actually make sense because it's not unheard of to have custom chunk options.

I don't get why my code changed this, though.

@zkamvar why was there originally the "info=bash"?

zkamvar

Thank you thank you thank you for tackling this! You brought up a good point about what we want the output to be. This PR produces yaml, but we may be able to choose one method and then use knitr::convert_chunk_header() to mix and match after that.

What do you think?

zkamvar · 2022-12-09T15:41:39Z

R/to_xml.R

+
+  code <- strsplit(xml2::xml_text(code_block), "\n")[[1]]
+  inchunk_info <- knitr::partition_chunk(info[["language"]], code)
+  xml2::xml_text(code_block) <- paste0(inchunk_info$code, "\n")


One of the things I'm wondering about 🤔 is it worth it for us to strip these comments from the source?

inst/extdata/chunk-options.Rmd

zkamvar · 2022-12-09T15:47:19Z

R/to_xml.R

+    names(inchunk_options) <- paste0(names(inchunk_options), "-inchunk")
+    info <- c(info, inchunk_options)


In this part, we need to include this process of quoting the characters so that we can properly parse them when converting back to YAML or chunk options:

tinkr/R/utils.R

Lines 73 to 84 in 60d3573

# Step 2: find the parameters that are characters because we need to add

# quotes around them (as all parameters are coerced as characters)

are_characters <- purrr::map_lgl(result, is.character)

# Step 3: flatten all params into a character vector

result <- unlist(result)

# Step 4: add quotes around the params that are characters

not_forbidden <- !names(result) %in% c("language", "name")

needs_quoting <- are_characters & not_forbidden

result[needs_quoting] <- shQuote(result[needs_quoting], type = "cmd")

It would be worth splitting this part off into a function.

Of course this will not solve our problems yet because there is also the problem of nested options.

We deal with this in our default chunk options by taking advantage of R syntax. and evaluating the chunk option into an alist of atomic vectors (character, integer, logical, language) that are all happily length 1. From there, we can flatten the list into characters that XML options understand and then quote the characters to indicate that the were characters in their previous lives.

The difficulty is that YAML will evaluate the R expressions for us. Let's take this parsed YAML block from the julia example:

l <- list( `label-inchunk` = c(label = "fig-plots"), `fig.cap-inchunk` = "Multiple Plots", `fig.subcap-inchunk` = c("Plot 1", "Plot 2"), `fig.alt-inchunk` = c( "A blue arc: labelled as y1 that starts at 0,0 and ends at 0.9,-0.75", "A blue curve: labelled as y1 twisted like a pretzel" ), `layout-ncol-inchunk` = 2L ) l #> $`label-inchunk` #> label #> "fig-plots" #> #> $`fig.cap-inchunk` #> [1] "Multiple Plots" #> #> $`fig.subcap-inchunk` #> [1] "Plot 1" "Plot 2" #> #> $`fig.alt-inchunk` #> [1] "A blue arc: labelled as y1 that starts at 0,0 and ends at 0.9,-0.75" #> [2] "A blue curve: labelled as y1 twisted like a pretzel" #> #> $`layout-ncol-inchunk` #> [1] 2

To overcome this, we can use capture.output along with dput to construct an alist and then parse that into a list.

eval(parse(text = paste0("a", paste(capture.output(dput(l)), collapse = "")))) #> $`label-inchunk` #> c(label = "fig-plots") #> #> $`fig.cap-inchunk` #> [1] "Multiple Plots" #> #> $`fig.subcap-inchunk` #> c("Plot 1", "Plot 2") #> #> $`fig.alt-inchunk` #> c("A blue arc: labelled as y1 that starts at 0,0 and ends at 0.9,-0.75", #> "A blue curve: labelled as y1 twisted like a pretzel") #> #> $`layout-ncol-inchunk` #> [1] 2

^{Created on 2022-12-09 with reprex v2.0.2}

With these options, we can then use purrr::map(eval) and then yaml::as.yaml() to produce the yaml string for writing back into the chunk

R/to_xml.R

Co-authored-by: Zhian N. Kamvar <[email protected]>

maelle · 2022-12-13T15:09:16Z

or, in the XML, we store the inchunk options as... YAML string? 🤔

It might help us with the parsing.
Now it might make users' life a bit more difficult if we don't provide some functionality to go with this?

maelle · 2022-12-15T12:26:17Z

It'd be nice to have a function operating on code chunks that would present chunk options as a tibble, with for each option a row with

name
value (list)
whether it's defined inside or outside the chunk.

Then there'd be a function to carry edits to this tibble to the code chunk XML.

@zkamvar how would that sound? what would this feature be, code chunks as objects?

maelle · 2022-12-15T12:45:47Z

R/to_xml.R

+  inchunk_options <- inchunk_info$options
+
+  inchunk_options <- if (!is.null(inchunk_options) > 0) {
+    yaml::as.yaml(inchunk_options)


now it's a bit annoying that there's such different treatment for in- and out- chunk options, it feels a bit wrong maybe.

This will help parse expressions... maybe

Suggested change

yaml::as.yaml(inchunk_options)

yaml::as.yaml(inchunk_options,

handlers = list(expression = function(x) paste("!expr: ", paste(as.character(x), sep = "; "))))

@zkamvar could you please explain this?

This is to address expressions that originate from YAML, but I'm now realising that I'm actually having a hard time getting a good example, so I have to rethink this.

(x <- list(A = 1, B = "two", C = str2expression("C <- 1 + 2\nC"))) #> $A #> [1] 1 #> #> $B #> [1] "two" #> #> $C #> expression(C <- 1 + 2, C) eval(x$C) # C evaluates to 3 #> [1] 3 try(yaml::as.yaml(x)) # conversion fails because C is an expression #> Error in yaml::as.yaml(x) : Unknown emitter error expr_handler <- function(x) paste("!expr:", paste(as.character(x), collapse = "; ")) yaml::as.yaml(x, handlers = list(expression = expr_handler)) #> [1] "A: 1.0\nB: two\nC: '!expr: C <- 1 + 2; C'\n"

^{Created on 2023-01-12 with reprex v2.0.2}

maelle · 2022-12-15T12:47:17Z

one might first need a method to get all code chunks? 🤔

maelle · 2022-12-15T15:17:59Z

In #28 you had mentioned the extraction of features such as code chunks @zkamvar

R/to_xml.R

zkamvar · 2022-12-17T00:10:15Z

one might first need a method to get all code chunks? 🤔

This is a good point. We've been able to get away with parsing the code chunks because they've always been generally linear and not very complex. With the new nested complexity that YAML introduces, It would definitely make sense for us to have options to store and access the chunk options consistently.

I'm wondering if we should just be opinionated on how code chunks are treated on output (e.g. users can choose to use curly, hashpipe-list, or hashpipe-yaml, but no mixing).

Co-authored-by: Zhian N. Kamvar <[email protected]>

maelle · 2023-01-13T13:21:21Z

@zkamvar what would be a good target for this PR? Happy to do more work on it / follow-up PRs 😁

zkamvar · 2023-03-17T20:27:59Z

@zkamvar what would be a good target for this PR? Happy to do more work on it / follow-up PRs grin

Ugh, I'm so sorry. We were doing this when things got really hectic for me and I had to drop it. I'm going to come back to this probably after May (that's when we transition all of our lessons)

maelle · 2023-03-21T09:49:55Z

No worry and good luck with the lessons transition! 🚀

maelle added 14 commits December 6, 2022 15:35

add example

6d0a435

start adding code

e0bd291

Merge branch 'main' into inside-chunk-options

4c05ae9

outchunk_options

a9cc59f

inchunk

2380bb1

oops

c7847fe

stop for now

c4932ad

add test

b7821bb

tweaks

2562b49

tweak

987a549

tweaks

122db32

less failures

9f09480

less failures

ad51486

ah!

60d3573

maelle marked this pull request as ready for review December 8, 2022 14:44

maelle requested a review from zkamvar December 8, 2022 14:44

maelle changed the title ~~DRAFT in-chunk options support~~ In-chunk options support Dec 8, 2022

maelle commented Dec 8, 2022

View reviewed changes

zkamvar reviewed Dec 9, 2022

View reviewed changes

zkamvar and others added 4 commits December 12, 2022 08:36

Merge branch 'main' into inside-chunk-options

c59c72c

Update inst/extdata/chunk-options.Rmd

2384577

Co-authored-by: Zhian N. Kamvar <[email protected]>

Update R/to_xml.R

e1ab985

Co-authored-by: Zhian N. Kamvar <[email protected]>

WIP

e473fc1

maelle added 2 commits December 15, 2022 13:11

yaml 😈

db04ef3

oops

8b0c2ef

add 1 more example

50433bb

maelle commented Dec 15, 2022

View reviewed changes

zkamvar reviewed Dec 16, 2022

View reviewed changes

R/to_xml.R Outdated Show resolved Hide resolved

maelle and others added 2 commits January 12, 2023 12:31

Update R/to_xml.R

fe2e7af

Co-authored-by: Zhian N. Kamvar <[email protected]>

update snapshot

c1499d0

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

In-chunk options support #82

In-chunk options support #82

maelle commented Dec 6, 2022 •

edited

Loading

maelle Dec 8, 2022

zkamvar Dec 9, 2022

maelle Dec 13, 2022

zkamvar Dec 16, 2022

maelle Jan 13, 2023

maelle Dec 8, 2022

zkamvar Dec 9, 2022

maelle Jan 13, 2023

maelle Dec 8, 2022

maelle Dec 8, 2022

zkamvar Dec 9, 2022

maelle Dec 15, 2022

maelle Jan 13, 2023

zkamvar left a comment

zkamvar Dec 9, 2022

zkamvar Dec 9, 2022

zkamvar Dec 9, 2022

maelle commented Dec 13, 2022 •

edited

Loading

maelle commented Dec 15, 2022

maelle Dec 15, 2022

zkamvar Dec 17, 2022 •

edited

Loading

maelle Jan 12, 2023

zkamvar Jan 12, 2023

maelle commented Dec 15, 2022

maelle commented Dec 15, 2022

zkamvar commented Dec 17, 2022

maelle commented Jan 13, 2023

zkamvar commented Mar 17, 2023

maelle commented Mar 21, 2023

	# One block with info
	info_attr <- xml2::xml_attr(blocks, "info")
	expect_equal(sum(!is.na(info_attr)), 1)

	xml2::xml_set_attr(blocks, "language", "julia")

	# save back and have a look
	to_md(yaml_xml_list, newmd)
	expect_snapshot_file(newmd)

	# Still one block with info after writing (the process has not clobbered things)
	info_attr <- xml2::xml_attr(blocks, "info")
	expect_equal(sum(!is.na(info_attr)), 1)

		names(inchunk_options) <- paste0(names(inchunk_options), "-inchunk")
		info <- c(info, inchunk_options)

	# Step 2: find the parameters that are characters because we need to add
	# quotes around them (as all parameters are coerced as characters)
	are_characters <- purrr::map_lgl(result, is.character)

	# Step 3: flatten all params into a character vector
	result <- unlist(result)

	# Step 4: add quotes around the params that are characters
	not_forbidden <- !names(result) %in% c("language", "name")
	needs_quoting <- are_characters & not_forbidden
	result[needs_quoting] <- shQuote(result[needs_quoting], type = "cmd")

	yaml::as.yaml(inchunk_options)
	yaml::as.yaml(inchunk_options,
	handlers = list(expression = function(x) paste("!expr: ", paste(as.character(x), sep = "; "))))

In-chunk options support #82

Are you sure you want to change the base?

In-chunk options support #82

Conversation

maelle commented Dec 6, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zkamvar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maelle commented Dec 13, 2022 • edited Loading

maelle commented Dec 15, 2022

Choose a reason for hiding this comment

zkamvar Dec 17, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maelle commented Dec 15, 2022

maelle commented Dec 15, 2022

zkamvar commented Dec 17, 2022

maelle commented Jan 13, 2023

zkamvar commented Mar 17, 2023

maelle commented Mar 21, 2023

maelle commented Dec 6, 2022 •

edited

Loading

maelle commented Dec 13, 2022 •

edited

Loading

zkamvar Dec 17, 2022 •

edited

Loading