feat: Add a wrapper for quarto markdown rendering #3003
Conversation
This wrapper uses quarto to render R quarto markdown .qmd notebooks. R dependency handling with conda or renv is supported.
votti
changed the title
|
Do you need to specify all those dependencies? And why do you need a second environment? |
This simplifies the dependencies in the main conda environment. This required then to rewrite the tests to use a separate conda environment with renv installed in order to build and restore the renv.
|
Hi @fgvieira , @multiple environments:
|
|
@votti we need a version in the Don't think we need the If not needed, best to have a single env. You can always add |
| # resources CLI flag: --resources renv_lock=1 --set-resource-scopes renv_lock=global | ||
|
|
||
|
|
||
| rule renv_example_init: |
There was a problem hiding this comment.
Similar the renv testcase requires renv as an additional dependency which I would track in the separate renv.yaml.
Generally these should not need to be updated. If updating renv.yaml, the renv_dt.lock file needs to be adapted as well.
As discussed in the PR this does not need to be explicitly shown. Instead I have added this more expliiclty to the documentation.
votti
force-pushed
the
feature/quarto
branch
2 times, most recently
from
f337a87 to
b17f428
Compare
Before this would have raised an error `log` not defined
utils/quarto/meta.yaml
Outdated
| threads: lambda wc, threads: threads | ||
| mem: lambda wc, resources: resources.mem |
There was a problem hiding this comment.
Wouldn't it make more sense for the wrapper to use threads and mem specified under resources?
There was a problem hiding this comment.
I definitely think it would make sense and attempted to implement this, but technically I did not find a way to achieve it:
We currently pass parameters for the markdown rendering using the --P, --execute-param KEY:VALUE syntax of the quarto render command. This only accepts parameters that are actually defined in the markdown document to be rendered and fails otherwise. This behaviour is coded in the knit_params_get used under the hood: https://github.com/rstudio/rmarkdown/blob/4b2f3426c1f46a8d207b0d661fc51c2508ecdad7/R/params.R#L28-L34
I dont see any possiblity to easily change this behaviour.
Thus if we would pass threads and mem as a parameter automatically, these would require the explicit declaration of these parameters in the markdown rendered.
If the markdown rendered does not explicitly supports these parameters, rendering would fail.
Thus to me requiring all markdowns to be rendered using this wrapper to support these two resource parameters would not be a good solution, as it would limit the applicability of the wrapper.
Also it would not be a complete solution as supporting any other resources would still require a workaround as the one documented here as well.
Automatically passing any resource parameter would be even more brittle as additional resources may be passed depending on the snakemake configuration used.
There was a problem hiding this comment.
If I understand correctly, params/threads/mem need to be specified inside the .qmd file AND passed as command-line arguments (--P, --execute-param KEY:VALUE)?
There was a problem hiding this comment.
Yes that is how it works.
There was a problem hiding this comment.
That is a bit unpractical since the params of the rule might not match the ones in the .qmd file! On the other hand, it is also not possible to add more parameters.
How are parameters included in the .qmd file? Is it an upstream rule?
Would it make sense for the wrapper to overwrite (or check) the params in the .qmd file to ensure they match?
There was a problem hiding this comment.
Think of the qmd file like an extension to Rmarkdown or akin to a Jupyter notebook.
I imagine the typical use of the wrapper would be users to hand-write the .qmd notebook, like they would do in a Rmarkdown or Jupyter notebook.
The parameter are included when authoring the file. The defined parameters are then used in the report to eg customize titles or as parameters in functions.
Would it make sense for the wrapper to overwrite (or check) the params in the .qmd file to ensure they match?
@overwriting: I am not sure if this would be sensible: adding additional parameters would not typically not really having an effect. I would say is is really rare for people to write code that handles arbitrary additional parameters and thus typically these would simply be ignored.
@cheking to ensure they match: this is what quarto render does at runtime and the error message raised is quite understandable, so I would say this is already done.
There was a problem hiding this comment.
What about if the wrapper just read the parameters in the .qmd file and add those with --execute-param KEY:VALUE? Would that make sense?
There was a problem hiding this comment.
I think this could make sense and it should be 'easy' to implement using knitr:knit_params: https://rdrr.io/cran/knitr/man/knit_params.html
or by writing a python function that parses the 'front matter' of such documents based some regexp.
This definitely would be quite convenient to implicitly pass these parameters if appropriately named parameters exist in the document.
Personally I am not so sure if I like such an implicit parameter passing as I tend to prefer explicit over implicit for such cases. Implicitly passing these may make it difficult to detect errors. Eg if in the parameter in the quarto document is called 'thread' instead of 'threads' this would lead to a silent ignoring of the parameter.
Also it is not possible to see at a glance from the rule if the document is respecting the resources.
What would you think as an alternative of a parameter: 'params: resource_parameters' that allow the user to specify a list of strings with resources to be passed as parameters:
Eg:
params:
resource_parameters: ['threads', 'mem_mb']
What would you prefer?
There was a problem hiding this comment.
I now implemented that if a quarto params named resources_<resource_name> is defined
the corresponding snakemake resource parameter is automatically passed.
eg quarto param resources_mem_mb -> snakemake.resources.mem_mb will be
passed.
threads will also be passed if a quarto parameter resrouces_threads is present.
Would this work for you?
There was a problem hiding this comment.
But how would those quarto params be defined?
I am not very familiar with quarto but my concern is that, if the rule resources change then the same input won't work (since the resources are hard-coded inside the file).
As I see it, the only way to ensure it, is to always overwrite (or add) the resources in the .qmd file with those in snakemake.resources. If you think the current wrapper can do it, then I am happy. 😄
Co-authored-by: Filipe G. Vieira <1151762+fgvieira@users.noreply.github.com>
As suggested resources that match the name should be automatically passed. Tests added: - test if a custom resource added will be passed - test if the special resource 'threads' will be passed
If a quarto params named resources_<resource_name> is defined the corresponding snakemake resource parameter is automatically passed. eg quarto param resources_mem_mb -> snakemake.resources.mem_mb will be passed.
Accidentally the header of the notes section in meta.yaml was removed
|
The idea behind @johanneskoester what do you think of using |
|
I think it is worth adding any further missing R packages to conda first. While it is fine to support renv as an alternative to conda at some point in general, I would say it is not really needed here, isn't it? We autobump and lock conda packages here, supporting a different kind of environments for wrappers would only further increase the complexity without improving the user experience (since conda is capable to handle R packages just fine). |
|
I just checked some of the packages in the renv, and all of them are available on conda-forge. I did not check all of them, but maybe they are even all already available for adding them to the environment.yaml? |
|
This PR was marked as stale because it has been open for 6 months with no activity. |
📝 WalkthroughWalkthroughThis change introduces a new Quarto wrapper for Snakemake workflows, including the wrapper implementation, metadata, environment specifications, example Quarto documents, and comprehensive tests. The wrapper enables rendering of Quarto markdown reports with support for parameter passing, resource management, and R environment handling via both conda and renv. Multiple new test cases and a Snakemake workflow are added to validate the wrapper's functionality for various output formats and configurations. Existing tests are reformatted for readability. Explicit environment files ensure reproducible setups for both the main Quarto environment and renv-based workflows. Changes
Sequence Diagram(s)sequenceDiagram
participant Snakemake
participant Wrapper (wrapper.py)
participant Quarto
participant R Environment
Snakemake->>Wrapper (wrapper.py): Invoke with inputs, outputs, params, resources
Wrapper (wrapper.py)->>R Environment: Setup/activate (conda or renv)
Wrapper (wrapper.py)->>Quarto: Run `quarto render` with parameters
Quarto->>R Environment: Execute R code in .qmd
Quarto-->>Wrapper (wrapper.py): Output rendered report
Wrapper (wrapper.py)-->>Snakemake: Move/copy output to target location
Suggested reviewers
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (14)
utils/quarto/test/env/renv.linux-64.pin.txt (1)
93-93: Add missing newline at EOF.
Most linters and POSIX tools expect a trailing newline.utils/quarto/environment.linux-64.pin.txt (1)
138-138: Add missing newline at EOF.
Include a final newline to satisfy POSIX file conventions and linting tools.utils/quarto/environment.yaml (1)
5-5: Add trailing newline.
YAML linters flag missing newline at end-of-file; please add one.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 5-5: no new line character at the end of file
(new-line-at-end-of-file)
utils/quarto/test/env/renv.yaml (1)
6-6: Add trailing newline.
YAML files should end with a newline to comply with linters and POSIX standards.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 6-6: no new line character at the end of file
(new-line-at-end-of-file)
utils/quarto/meta.yaml (1)
53-53: Add missing newline at end of file
A newline is required at EOF to satisfy YAMLlint’snew-line-at-end-of-filerule.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 53-53: no new line character at the end of file
(new-line-at-end-of-file)
test.py (5)
6737-6748: Include "-F" flag for consistency
Most wrapper tests include the-F(force) flag to ensure outputs are overwritten and logs are reproducible. Please add"-F"to this Snakemake invocation.
6751-6762: Include "-F" flag for consistency
Add"-F"to match the pattern used by other wrapper tests when invoking Snakemake.
6765-6776: Include "-F" flag for consistency
Please append"-F"to the command array here to maintain uniform test behavior.
6779-6790: Include "-F" flag for consistency
To align with the existing tests, add"-F"to this Snakemake call.
6793-6804: Include "-F" flag for consistency
Add"-F"to this test invocation so it follows the same conventions as the other Quarto wrapper tests.utils/quarto/test/Snakefile (1)
1-1: Consider removing unused importThe
osmodule is imported but not used anywhere in this Snakefile.-import osutils/quarto/wrapper.py (3)
54-68: Fix type annotation and simplify list handlingThe type annotation for the
valueparameter is missing, and there's a simpler way to handle list conversion.-def add_parameter(name: str, value, param_list: list[str]) -> None: +def add_parameter(name: str, value: any, param_list: list[str]) -> None: """Add a parameter to the parameter list in place Args: name (str): The name of the parameter - value (str): The value of the parameter + value (any): The value of the parameter param_list (list[str]): The list of parameters Returns: None """ if isinstance(value, list): - value = ",".join(value) + value = ",".join(str(v) for v in value) param_list.append("-P {}:{}".format(name, value))
89-105: Optimize dictionary key lookupThe code uses
.keys()unnecessarily when checking for key presence.passed_resource_params = { f"{RESOURCE_PARAMS_PREFIX}{k}": v for k, v in resources.items() - if f"{RESOURCE_PARAMS_PREFIX}{k}" in params_header.keys() + if f"{RESOURCE_PARAMS_PREFIX}{k}" in params_header }🧰 Tools
🪛 Ruff (0.8.2)
95-95: Undefined name
snakemake(F821)
96-96: Undefined name
snakemake(F821)
100-100: Use
key in dictinstead ofkey in dict.keys()Remove
.keys()(SIM118)
127-144: Add stacklevel to warning and improve report path constructionThe warning about PDF output doesn't specify a stacklevel, and the report path construction could be clearer.
if report_path.suffix == ".pdf": warnings.warn( "PDF output is likely not supported yet, as tinytex not installable" - " via conda: https://github.com/conda-forge/quarto-feedstock/issues/26" + " via conda: https://github.com/conda-forge/quarto-feedstock/issues/26", + stacklevel=2 )🧰 Tools
🪛 Ruff (0.8.2)
140-140: No explicit
stacklevelkeyword argument found(B028)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
utils/quarto/test/env/renv_dt.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
test.py(2 hunks)utils/quarto/environment.linux-64.pin.txt(1 hunks)utils/quarto/environment.yaml(1 hunks)utils/quarto/meta.yaml(1 hunks)utils/quarto/test/Snakefile(1 hunks)utils/quarto/test/env/renv.linux-64.pin.txt(1 hunks)utils/quarto/test/env/renv.yaml(1 hunks)utils/quarto/test/extra_datatable.qmd(1 hunks)utils/quarto/test/full.qmd(1 hunks)utils/quarto/test/resources_custom.qmd(1 hunks)utils/quarto/test/resources_threads.qmd(1 hunks)utils/quarto/test/simple.qmd(1 hunks)utils/quarto/wrapper.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
`**/*.py`: Do not try to improve formatting. Do not suggest type annotations for functions that are defined inside of functions or methods. Do not suggest type annotation of the `s...
**/*.py: Do not try to improve formatting.
Do not suggest type annotations for functions that are defined inside of functions or methods.
Do not suggest type annotation of theselfargument of methods.
Do not suggest type annotation of theclsargument of classmethods.
Do not suggest return type annotation if a function or method does not contain areturnstatement.
utils/quarto/wrapper.pytest.py
`**/wrapper.py`: Do not complain about use of undefined variable called `snakemake`.
**/wrapper.py: Do not complain about use of undefined variable calledsnakemake.
utils/quarto/wrapper.py
🪛 YAMLlint (1.35.1)
utils/quarto/test/env/renv.yaml
[error] 6-6: no new line character at the end of file
(new-line-at-end-of-file)
utils/quarto/environment.yaml
[error] 5-5: no new line character at the end of file
(new-line-at-end-of-file)
utils/quarto/meta.yaml
[error] 53-53: no new line character at the end of file
(new-line-at-end-of-file)
🪛 Ruff (0.8.2)
utils/quarto/wrapper.py
70-70: Undefined name snakemake
(F821)
71-71: Undefined name snakemake
(F821)
72-72: Undefined name snakemake
(F821)
77-77: Undefined name snakemake
(F821)
81-81: Undefined name snakemake
(F821)
84-84: Undefined name snakemake
(F821)
95-95: Undefined name snakemake
(F821)
96-96: Undefined name snakemake
(F821)
100-100: Use key in dict instead of key in dict.keys()
Remove .keys()
(SIM118)
110-110: Undefined name snakemake
(F821)
112-112: Undefined name snakemake
(F821)
113-113: Undefined name snakemake
(F821)
140-140: No explicit stacklevel keyword argument found
(B028)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Summary
🔇 Additional comments (36)
utils/quarto/test/env/renv.linux-64.pin.txt (3)
1-4: Explicit environment header is correct.
The comments and@EXPLICITdirective follow Conda’s format for explicit environment files.
5-93: Pinned package list ensures reproducibility.
This exhaustive list locks down the Rrenvenvironment for Linux-64, guaranteeing identical installs across systems.
1-93:✅ Verification successful
Keep explicit file in sync with YAML definitions.
Ensure any dependency changes inrenv.yamlor the wrapper’sconda:directive are mirrored here to prevent drift.Run this script to compare the
r-baseandr-renvversions between the YAML and explicit file:
🏁 Script executed:
#!/bin/bash # Compare r-base/renv versions grep -E '^ *- r-base' utils/quarto/test/env/renv.yaml | sed 's/- //g' > yaml.txt grep -E 'r-base' utils/quarto/test/env/renv.linux-64.pin.txt | head -1 | sed 's|.*/||; s/-[rb]*//g; s/_.*//g' > pin.txt diff yaml.txt pin.txtLength of output: 347
🏁 Script executed:
#!/bin/bash set -eux # Locate the YAML and pin files in the repo RENV_YAML=$(fd -t f renv.yaml . | head -1) PIN_FILE=$(fd -t f '*.pin.txt' . | head -1) echo "Using renv.yaml: $RENV_YAML" echo "Using pin file : $PIN_FILE" # Extract r-base and r-renv versions from renv.yaml grep -E '^ *- (r-base|r-renv)' "$RENV_YAML" | sed 's/- //g' > yaml_versions.txt # Extract r-base and r-renv from the explicit pin file grep -E '(r-base|r-renv)' "$PIN_FILE" | sed 's|.*/||; s|_.*||' > pin_versions.txt # Show a unified diff if there are discrepancies diff -u yaml_versions.txt pin_versions.txt || trueLength of output: 975
🏁 Script executed:
#!/bin/bash set -eux # Directly compare r-base and r-renv between YAML and explicit pin file YAML_FILE="utils/quarto/test/env/renv.yaml" PIN_FILE="utils/quarto/test/env/renv.linux-64.pin.txt" echo "YAML: $YAML_FILE" echo "PIN : $PIN_FILE" # Extract versions grep -E '^ *- (r-base|r-renv)' "$YAML_FILE" | sed 's/- //g' > yaml_versions.txt grep -E '(r-base|r-renv)' "$PIN_FILE" | sed 's|.*/||; s|_.*||' > pin_versions.txt # Show diff diff -u yaml_versions.txt pin_versions.txt || trueLength of output: 749
r-base and r-renv versions are in sync
Ther-base(4.3.3) andr-renv(1.0.7) entries match betweenrenv.yamland the explicit pin file—no drift detected.utils/quarto/environment.linux-64.pin.txt (3)
1-4: Explicit environment header is correct.
The file header and@EXPLICITdirective are properly set for Linux-64.
5-137: Comprehensive pin list for Quarto environment.
Locking system libraries, R packages, andquarto-1.4.550ensures a reproducible rendering environment.
81-81:✅ Verification successful
Verify Quarto version alignment.
Confirm that the pinnedquarto-1.4.550matches the version expected by the wrapper’s metadata (r-quarto=1.4).
🏁 Script executed:
#!/bin/bash # Check r-quarto in meta.yaml vs explicit pin rg -n "r-quarto" utils/quarto/meta.yaml grep -m1 -oP "quarto-[0-9]+\.[0-9]+\.[0-9]+" utils/quarto/environment.linux-64.pin.txtLength of output: 266
🏁 Script executed:
#!/bin/bash set -e echo "1) Locating all meta.yaml files in the repo" find . -name meta.yaml echo -e "\n2) Searching for 'r-quarto' in all files" grep -R -n "r-quarto" . echo -e "\n3) Finding all environment pin files" find . -type f -name "environment*.pin.txt" echo -e "\n4) Extracting the first pinned Quarto version in those files" grep -R -n -m1 -oP "quarto-[0-9]+\.[0-9]+\.[0-9]+" .Length of output: 15010
🏁 Script executed:
#!/bin/bash set -e echo "1) Show package name and version in utils/quarto/meta.yaml" grep -R -nE "^(package:|name:|version:)" -n utils/quarto/meta.yaml -A 3 -B 1 echo -e "\n2) Show 'requirements:' block in utils/quarto/meta.yaml" grep -R -n "requirements:" -n utils/quarto/meta.yaml -A 10 -B 2 echo -e "\n3) Display the top of utils/quarto/environment.yaml" sed -n '1,50p' utils/quarto/environment.yaml echo -e "\n4) Locate pinned 'quarto-' entries in utils/quarto/environment.linux-64.pin.txt" grep -n "quarto-[0-9]" -n utils/quarto/environment.linux-64.pin.txt | head -n 10Length of output: 531
Quarto version alignment verified.
The wrapper’senvironment.yamlspecifiesr-quarto=1.4, and theenvironment.linux-64.pin.txtpinsquarto-1.4.550. These are consistent—no changes needed.utils/quarto/environment.yaml (1)
1-5: Minimalenvironment.yamlis correctly configured.
Channels (conda-forge,nodefaults) andr-quarto=1.4meet the wrapper’s dependency requirements.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 5-5: no new line character at the end of file
(new-line-at-end-of-file)
utils/quarto/test/env/renv.yaml (1)
1-6: Baserenv.yamlis properly defined.
It includes the correct channels and pinsr-base=4.3.3andr-renv=1.0.7for renv-based testing.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 6-6: no new line character at the end of file
(new-line-at-end-of-file)
utils/quarto/test/simple.qmd (2)
1-5: Front-matter is valid Quarto YAML.
Thetitle,author, andformat: htmldirectives correctly configure an HTML output.
7-7: Minimal document body is sufficient.
“Hello Quarto!” provides a basic test case to verify rendering.utils/quarto/test/extra_datatable.qmd (1)
1-15: Well-structured test document for dependency management.This Quarto document effectively demonstrates the handling of additional R package dependencies. The document clearly explains its purpose and provides a good test case for verifying that the dependency management system (either conda or renv) correctly installs the required packages.
utils/quarto/test/resources_threads.qmd (1)
1-15: Good resource passing validation example.This test document correctly validates that the "threads" resource is passed from Snakemake to the Quarto document. The error message is clear and informative, making it easy to diagnose issues if the parameter passing fails.
utils/quarto/test/full.qmd (1)
1-15: Comprehensive parameter and file operation example.This example effectively demonstrates more complex Quarto usage, including dynamic titling with parameters and file operations. It's a good test case for ensuring that the wrapper correctly passes multiple parameters and that the R environment can perform file system operations.
utils/quarto/test/resources_custom.qmd (1)
1-15: Effective custom resource validation test case.This document correctly checks for custom resource parameters, complementing the threads resource test. Having separate test cases for different types of resources is a good practice to ensure the wrapper handles all resource types correctly.
test.py (2)
6807-6818: Verify resource passing test
This test targets custom resource parameters but doesn’t pass any--resourcesflag to Snakemake. Ensure you explicitly supply the correct resource (e.g.,--resources mem_mb=<value>) or clarify how the test environment provides those resources.
6821-6832: Test varying thread counts
Using--cores 1only validates the default path. To fully test thread propagation, consider invoking with a non-default value (e.g.,--cores 4) so the wrapper passes a distinctthreadsparameter into the Quarto script.utils/quarto/test/Snakefile (13)
4-12: LGTM! Well-structured rule aggregationThe
allrule correctly aggregates all test outputs as targets, providing a clear entry point for running the complete test suite.
14-17: LGTM! Simple and effective input generatorThis rule efficiently creates test input files using wildcards, which is a good practice for parameterized testing.
19-32: LGTM! Comprehensive test rule for full renderingThis rule properly tests the rendering of a full Quarto document with both report and text output, demonstrating parameter passing via the lambda function.
34-43: LGTM! Simple HTML rendering testClean implementation for basic HTML rendering test case.
45-54: LGTM! DOCX format rendering testThis rule appropriately tests rendering to DOCX format, ensuring the wrapper handles different output formats correctly.
56-66: LGTM! Testing rendering without report outputGood test case that verifies the wrapper can handle scenarios where only data outputs (not reports) are needed.
68-79: LGTM! Custom resource passing testThis rule properly tests the ability to pass custom resources to the Quarto rendering process.
81-90: LGTM! Thread parameter passing testThis rule correctly tests passing thread parameters to the Quarto rendering process.
92-107: LGTM! Well-documented commented exampleThe commented-out example for conda environment provides valuable documentation for users who want to add R dependencies via conda, which aligns with the PR discussions about dependency management approaches.
109-113: LGTM! Proper wildcard constraintThe wildcard constraint is correctly defined to allow flexibility in renv directory paths, including empty paths.
115-119: LGTM! Good explanation of resource lockingClear documentation of the purpose of the
renv_lockresource and how to use it correctly.
121-139: LGTM! Well-implemented renv initializationThis rule correctly initializes and restores an R environment using renv with proper resource locking and logging.
160-174: LGTM! Proper Quarto rendering with renvThe rule correctly demonstrates rendering a Quarto document with an renv environment, with good documentation explaining the potential need to override the conda directive.
utils/quarto/wrapper.py (7)
1-11: LGTM! Well-structured header and importsThe file has proper attribution, licensing information, and appropriate imports.
13-27: LGTM! Clear and effective renv path parserThe
parse_renvfunction provides good path resolution logic with appropriate type hints and documentation.
70-73: LGTM! Proper input extractionThe code correctly extracts the necessary input paths with default fallbacks.
🧰 Tools
🪛 Ruff (0.8.2)
70-70: Undefined name
snakemake(F821)
71-71: Undefined name
snakemake(F821)
72-72: Undefined name
snakemake(F821)
75-87: LGTM! Well-structured parameter collectionParameters are correctly collected from inputs, params, and outputs, with appropriate exclusions.
🧰 Tools
🪛 Ruff (0.8.2)
77-77: Undefined name
snakemake(F821)
81-81: Undefined name
snakemake(F821)
84-84: Undefined name
snakemake(F821)
107-116: LGTM! Proper log path resolutionThe code correctly resolves log paths and handles the case when no log is provided.
🧰 Tools
🪛 Ruff (0.8.2)
110-110: Undefined name
snakemake(F821)
112-112: Undefined name
snakemake(F821)
113-113: Undefined name
snakemake(F821)
119-126: LGTM! Good temporary output handlingThe code correctly handles the case when no report output is specified by using a temporary directory.
145-159: LGTM! Well-structured shell executionThe shell command correctly copies the script, changes to the renv directory, renders the document, and moves the output to the final destination.
| description: Render quarto markdown documents | ||
| url: https://github.com/quarto-dev/quarto-r | ||
| authors: | ||
| -Vito Zanotelli |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix YAML list syntax for authors
Add a space after the hyphen to conform to YAML list syntax. Also verify whether the wrapper schema expects a singular author: key instead of authors:.
-authors:
- -Vito Zanotelli
+authors:
+ - Vito Zanotelli📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| -Vito Zanotelli | |
| authors: | |
| - Vito Zanotelli |
| rule restore_renv: | ||
| conda: | ||
| "env/renv.yaml" | ||
| input: | ||
| rprofile="{renv_dir}.Rprofile", | ||
| lock="{renv_dir}renv.lock", | ||
| output: | ||
| restored="{renv_dir}restored.Rprofile", | ||
| resources: | ||
| renv_lock=1, | ||
| log: | ||
| "logs/renv/{renv_dir}_restore.log", | ||
| shell: | ||
| """ | ||
| R -e 'source("{input.rprofile}", chdir=TRUE);renv::restore()' | ||
| echo 'source("{input.rprofile}", chdir=TRUE)' > {output.restored} | ||
| """ | ||
|
|
There was a problem hiding this comment.
Fix paths in restore_renv rule
There are missing path separators in the input paths, which could lead to errors when resolving file paths.
input:
- rprofile="{renv_dir}.Rprofile",
- lock="{renv_dir}renv.lock",
+ rprofile="{renv_dir}/.Rprofile",
+ lock="{renv_dir}/renv.lock",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| rule restore_renv: | |
| conda: | |
| "env/renv.yaml" | |
| input: | |
| rprofile="{renv_dir}.Rprofile", | |
| lock="{renv_dir}renv.lock", | |
| output: | |
| restored="{renv_dir}restored.Rprofile", | |
| resources: | |
| renv_lock=1, | |
| log: | |
| "logs/renv/{renv_dir}_restore.log", | |
| shell: | |
| """ | |
| R -e 'source("{input.rprofile}", chdir=TRUE);renv::restore()' | |
| echo 'source("{input.rprofile}", chdir=TRUE)' > {output.restored} | |
| """ | |
| rule restore_renv: | |
| conda: | |
| "env/renv.yaml" | |
| input: | |
| rprofile="{renv_dir}/.Rprofile", | |
| lock="{renv_dir}/renv.lock", | |
| output: | |
| restored="{renv_dir}restored.Rprofile", | |
| resources: | |
| renv_lock=1, | |
| log: | |
| "logs/renv/{renv_dir}_restore.log", | |
| shell: | |
| """ | |
| R -e 'source("{input.rprofile}", chdir=TRUE);renv::restore()' | |
| echo 'source("{input.rprofile}", chdir=TRUE)' > {output.restored} | |
| """ |
| def parse_qmd_header(fn_script): | ||
| """Parse the yml header of the a | ||
| quarto markdown file | ||
|
|
||
| Args: | ||
| fn_script (path): Path to the qmd script | ||
|
|
||
| Raises: | ||
| ValueError: qmd has not valid header | ||
|
|
||
| Returns: | ||
| dict: parsed header | ||
| """ | ||
| with open(fn_script, mode="r") as f: | ||
| first = f.readline() | ||
| if first != "---\n": | ||
| raise ValueError(".qmd file has no valid yaml header.") | ||
| line = f.readline() | ||
| yml_lines = [] | ||
| while line != "---\n": | ||
| yml_lines.append(line) | ||
| line = f.readline() | ||
| return yaml.safe_load("".join(yml_lines)) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for YAML parsing
The function parses the YAML header but doesn't handle potential exceptions from yaml.safe_load().
while line != "---\n":
yml_lines.append(line)
line = f.readline()
- return yaml.safe_load("".join(yml_lines))
+ try:
+ return yaml.safe_load("".join(yml_lines))
+ except yaml.YAMLError as e:
+ raise ValueError(f"Invalid YAML in .qmd header: {e}")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def parse_qmd_header(fn_script): | |
| """Parse the yml header of the a | |
| quarto markdown file | |
| Args: | |
| fn_script (path): Path to the qmd script | |
| Raises: | |
| ValueError: qmd has not valid header | |
| Returns: | |
| dict: parsed header | |
| """ | |
| with open(fn_script, mode="r") as f: | |
| first = f.readline() | |
| if first != "---\n": | |
| raise ValueError(".qmd file has no valid yaml header.") | |
| line = f.readline() | |
| yml_lines = [] | |
| while line != "---\n": | |
| yml_lines.append(line) | |
| line = f.readline() | |
| return yaml.safe_load("".join(yml_lines)) | |
| def parse_qmd_header(fn_script): | |
| """Parse the yml header of the a | |
| quarto markdown file | |
| Args: | |
| fn_script (path): Path to the qmd script | |
| Raises: | |
| ValueError: qmd has not valid header | |
| Returns: | |
| dict: parsed header | |
| """ | |
| with open(fn_script, mode="r") as f: | |
| first = f.readline() | |
| if first != "---\n": | |
| raise ValueError(".qmd file has no valid yaml header.") | |
| line = f.readline() | |
| yml_lines = [] | |
| while line != "---\n": | |
| yml_lines.append(line) | |
| line = f.readline() | |
| try: | |
| return yaml.safe_load("".join(yml_lines)) | |
| except yaml.YAMLError as e: | |
| raise ValueError(f"Invalid YAML in .qmd header: {e}") |
Quarto has become a frequently used markdown notebook authoring system.
This wrapper uses quarto to render R Quarto
.qmdnotebooks using the the Quarto params mechanism to pass parameters from snakemake.R dependency handling with
renvis supported.The
meta.ymlcontains a more detailed description of the wrapper.Test cases for all the major functionality and demonstrating the
renvintegration are included.QC
For all wrappers added by this PR,
input:andoutput:file paths in the resulting rule can be changed arbitrarily,threads: xstatement withxbeing a reasonable default,map_readsfor a step that maps reads),environment.yamlspecifications follow the respective best practices,environment.yamlpinning has been updated by runningsnakedeploy pin-conda-envs environment.yamlon a linux machine,input:oroutput:),Snakefiles and their entries are explained via comments (input:/output:/params:etc.),stderrand/orstdoutare logged correctly (log:), depending on the wrapped tool,tempfile.gettempdir()points to (see here; this also means that using any Pythontempfiledefault behavior works),meta.yamlcontains a link to the documentation of the respective tool or command,Snakefiles pass the linting (snakemake --lint),Snakefiles are formatted with snakefmt,Summary by CodeRabbit
New Features
Tests
Chores