Adding a few knitr helpers#6
Conversation
73dcf83 to
d979556
Compare
llrs-roche
left a comment
There was a problem hiding this comment.
Thanks for the smaller PR!
I left some comments here and there but haven't run the code yet (will update add more comments once I do).
Now that I think of it, if we export it we should be able to maintain these functions well. We will need examples and tests for them.
|
All really great feedback. Will implement all your suggestions.
This in particular is something I was debating. I'd prefer not to require internal functions in the chunks of the report, but I also see these functions as "internal" (at least subject to change and evolution, I don't really want to sign us up for maintaining a stable API for these) So I guess I see this package as a convenience for bundling packages functions used in the template, rather than a public-facing package for broader use. I'm not exactly sure how to manage function exports for that use case. |
…itr/rmarkdown is used
dgkf
left a comment
There was a problem hiding this comment.
@llrs-roche I think I addressed all your feedback.
- added tests
- added a note about use of exported helpers (but would be happy to revise and just keep them as non-exported if you think that would be better)
- updated docs and added examples
llrs-roche
left a comment
There was a problem hiding this comment.
Locally checks failed:
Error (test-utils-knitr.R:127:3): knitr_mutable_header can modify yaml frontmatter during runtime
<badUnicodeHex/parseError/error/condition>
Error: '\U' used without hex digits in character string (<text>:14:19)
Error (test-utils-knitr.R:112:5): knitr_update_options accepts complex R objects through quarto::quarto_render
Error in `quarto::quarto_render(example_qmd, execute_params = params_expr, quiet = TRUE)`: ! Error running quarto CLI from R.
Caused by error in `quarto::quarto_render()`:
x Error returned by quarto CLI.
i Rerun with `quiet = FALSE` to see the full error message.
Error (test-utils-knitr.R:71:5): knitr_update_options accepts complex R objects through rmarkdown::render
<badUnicodeHex/parseError/error/condition>
Error: '\U' used without hex digits in character string (<text>:14:19)
Failure (test-report.R:12:5): Run test for the report / should be generated in all formats
Expected `package_report(...)` not to throw any errors.
Actually got a <rlang_error> with message:
[ FAIL 5 | WARN 0 | SKIP 0 | PASS 33 ]
Maybe because I'm on a Windows and
quarto::check_newer_version()
## ℹ You are using the latest stable version of Quarto: 1.8.27.
|
Maybe we need to have also windows checks? It is not the first time something works on someone else computer but not on my windows |
|
Okay, got the ubuntu checks passing. Since the others are also failing on main, I propose that we focus on fixing the other OSes in a follow-up PR. @llrs-roche let me know if that works for you. |
llrs-roche
left a comment
There was a problem hiding this comment.
Agreed! Let's move forward even if the package is broken at Windows and macOS at the moment.
Feel free to merge
Changes broken out from #5
Adds the two
knitrhelpers,knitr_mutable_headerandknitr_loggerwith some minimal changes to the report to accommodate them.Affected files got formatted by
air, so I suggest checking the ignore whitespace changes box while reviewing.