Add quartonotebook module #4876
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
5be5f57 to
49187dc
Compare
49187dc to
2f87b27
Compare
Do not specify using the AMD64 architecture for the docker test profile, as this leads to problems with Pandoc (which Quarto uses) when emulating AMD64 on ARM64 systems. The docker images for this module can be built on both architectures, so always specifying one or the other is not necessary.
This reverts commit 839cae0. Getting PDF output to work turned out to be problematic due to issues with (1) differences between TinyTeX installations on AMD64/ARM64 architectures; and (2) getting Pandoc to work properly inside the docker containers. This might be solvable with a lot more work and troubleshooting, but removing the PDF-functionality for now since HTML is the output type expected to be the one desired by a vast majority of the envisioned module audience; the related RMARKDOWNNOTEBOOK and JUPYTERNOTEBOOK modules currently only support HTML output. Another possible solution is to use the new `typst` typesetting system introduced in Quarto 1.4 instead of *TeX, but this would preclude being able to use Conda (which currently doesn't have Quarto 1.4).
Disallow using the Conda or Mamba profiles for the QUARTONOTEBOOK module, as the environment created differs from that created with containers. The Conda version of Quarto does not work on ARM64 architectures due to Pandoc-related issues, but installing outside Conda works in a container-context. It is thus impossible to get the same environment in a container image and using Conda, if compatibility with both AMD64 and ARM64 architectures is desired (which it is). Hopefully the issues with Conda will be solved in the future.
94acf9b to
6c9a653
Compare
grst
reviewed
Feb 8, 2024
There was a problem hiding this comment.
Thanks for adding this! I wish quarto existed when I was working on the jupyternotebook and rmarkdown modules.
Just two minor comments. I also thought there was something broken with the dumping the parameters to yaml in more recent versions of nextflow, but the tests seem to pass.
modules/nf-core/quartonotebook/tests/with-parametrization.config
Outdated
Show resolved
Hide resolved
Add the `extension` module input that pipelines can use for Quarto
templates. This can be achieved e.g. by adding the `_extensions/`
directory with whatever extensions are desired into a pipeline's
`assets/` directory and creating a value channel like so:
`extensions = Channel.fromPath("[...]/_extensions").collect()`.
|
As I was working on your comments I realised that I had forgotten to add the ability to use Quarto extensions for the module! I have now added commits for this, as well as outputting the original report to better reflect how RMARKDOWNNOTEBOOK and JUPYTERNOTEBOOK works - this one already outputs the parameter YAML for parametrisation, so it felt natural to also give the original report for users who want to edit it directly. I have previously made a Quarto nf-core extension, which is here: https://github.com/fasterius/nf-core-quarto-template. We are already using it in the spatialtranscriptomics pipeline. |
grst
approved these changes
Feb 9, 2024
17 tasks
jch-13
pushed a commit
to jch-13/modules
that referenced
this pull request
Mar 19, 2024
* Add `main.nf` for quartonotebook
* Add environment files
* Add `meta.yml`
* Temporarily change `test_data_base` to for testing
* Add bare-bones nf-test test
* Abort when running with Conda profile on ARM64
* Add stub test; snapshot all outputs
* Add python, rmd and ipynb tests
* Add notebook parametrization
* Update Conda environment
* Add parametrization tests
* Add note about the container
* Add missing `papermill` mention in tools section
* Fix function names according to nf-core convention
* Add missing `${args}` to Quarto render command
* Change output to `${prefix}.html`
* Also allow PDF output; add PDF tests
* Fix nf-test configs
* Do not specify AMD64 for docker test profile
Do not specify using the AMD64 architecture for the docker test profile,
as this leads to problems with Pandoc (which Quarto uses) when emulating
AMD64 on ARM64 systems. The docker images for this module can be built
on both architectures, so always specifying one or the other is not
necessary.
* Revert "Also allow PDF output; add PDF tests"
This reverts commit 839cae0.
Getting PDF output to work turned out to be problematic due to issues
with (1) differences between TinyTeX installations on AMD64/ARM64
architectures; and (2) getting Pandoc to work properly inside the docker
containers. This might be solvable with a lot more work and
troubleshooting, but removing the PDF-functionality for now since HTML
is the output type expected to be the one desired by a vast majority of
the envisioned module audience; the related RMARKDOWNNOTEBOOK and
JUPYTERNOTEBOOK modules currently only support HTML output.
Another possible solution is to use the new `typst` typesetting system
introduced in Quarto 1.4 instead of *TeX, but this would preclude being
able to use Conda (which currently doesn't have Quarto 1.4).
* Update snapshot
* Disallow using the Conda/Mamba profile
Disallow using the Conda or Mamba profiles for the QUARTONOTEBOOK
module, as the environment created differs from that created with
containers. The Conda version of Quarto does not work on ARM64
architectures due to Pandoc-related issues, but installing outside Conda
works in a container-context. It is thus impossible to get the same
environment in a container image and using Conda, if compatibility with
both AMD64 and ARM64 architectures is desired (which it is). Hopefully
the issues with Conda will be solved in the future.
* Use `nf-core/test-datasets` for test data
* Move XDG variable definition to `main.nf`
* Add `extensions` input for Quarto templates
Add the `extension` module input that pipelines can use for Quarto
templates. This can be achieved e.g. by adding the `_extensions/`
directory with whatever extensions are desired into a pipeline's
`assets/` directory and creating a value channel like so:
`extensions = Channel.fromPath("[...]/_extensions").collect()`.
* Also output the original report
* Update snapshot
* Add note regarding disallowing the Conda profile
jennylsmith
pushed a commit
to RSC-RP/modules
that referenced
this pull request
Mar 20, 2024
* Add `main.nf` for quartonotebook
* Add environment files
* Add `meta.yml`
* Temporarily change `test_data_base` to for testing
* Add bare-bones nf-test test
* Abort when running with Conda profile on ARM64
* Add stub test; snapshot all outputs
* Add python, rmd and ipynb tests
* Add notebook parametrization
* Update Conda environment
* Add parametrization tests
* Add note about the container
* Add missing `papermill` mention in tools section
* Fix function names according to nf-core convention
* Add missing `${args}` to Quarto render command
* Change output to `${prefix}.html`
* Also allow PDF output; add PDF tests
* Fix nf-test configs
* Do not specify AMD64 for docker test profile
Do not specify using the AMD64 architecture for the docker test profile,
as this leads to problems with Pandoc (which Quarto uses) when emulating
AMD64 on ARM64 systems. The docker images for this module can be built
on both architectures, so always specifying one or the other is not
necessary.
* Revert "Also allow PDF output; add PDF tests"
This reverts commit 839cae0.
Getting PDF output to work turned out to be problematic due to issues
with (1) differences between TinyTeX installations on AMD64/ARM64
architectures; and (2) getting Pandoc to work properly inside the docker
containers. This might be solvable with a lot more work and
troubleshooting, but removing the PDF-functionality for now since HTML
is the output type expected to be the one desired by a vast majority of
the envisioned module audience; the related RMARKDOWNNOTEBOOK and
JUPYTERNOTEBOOK modules currently only support HTML output.
Another possible solution is to use the new `typst` typesetting system
introduced in Quarto 1.4 instead of *TeX, but this would preclude being
able to use Conda (which currently doesn't have Quarto 1.4).
* Update snapshot
* Disallow using the Conda/Mamba profile
Disallow using the Conda or Mamba profiles for the QUARTONOTEBOOK
module, as the environment created differs from that created with
containers. The Conda version of Quarto does not work on ARM64
architectures due to Pandoc-related issues, but installing outside Conda
works in a container-context. It is thus impossible to get the same
environment in a container image and using Conda, if compatibility with
both AMD64 and ARM64 architectures is desired (which it is). Hopefully
the issues with Conda will be solved in the future.
* Use `nf-core/test-datasets` for test data
* Move XDG variable definition to `main.nf`
* Add `extensions` input for Quarto templates
Add the `extension` module input that pipelines can use for Quarto
templates. This can be achieved e.g. by adding the `_extensions/`
directory with whatever extensions are desired into a pipeline's
`assets/` directory and creating a value channel like so:
`extensions = Channel.fromPath("[...]/_extensions").collect()`.
* Also output the original report
* Update snapshot
* Add note regarding disallowing the Conda profile
|
@fasterius any links to the issue with pandoc on arm64 with amd64 emulation? Trying to use Seqera containers in #5561 |
I haven't check into it for a long time now, so don't know if the issue is solved now or not. Does it work using Seqera containers? |
|
It seems to? The tests pass at least. Let me know if there's something else to look for! If you don't have any objections I'll merge the other PR and we can report if anyone has any issues. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds a new QUARTONOTEBOOK module, which renders parametrised Quarto documents to HTML. This modules re-uses some code from the RMARKDOWNNOTEBOOK and JUPYTERNOTEBOOK modules, namely part of the code that parametrizes the reports. It re-uses some of the test datasets from those modules, but also adds its own test data; Quarto can render R Markdown as well as Jupyter notebooks.
A difference between this module and its notebook-relatives is how the parametrisation is done. This module does not output a parametrised report with the relevant parameters changed from the default, but rather outputs a parametrised
params.ymlfile, which is equivalent to the hidden.params.ymlin the other modules. The main reason for this is that Quarto is built with multi-language support (Python, R, Julia and Observable, of which the former two are tested for) and has an option (--execute-params) that allows taking parameters from a YAML file in a language-agnostic way usingpapermill(JUPYTERNOTEBOOK also usespapermill). The way that the other modules do it yields a preferable output from a user-perspective, but is harder to achieve without language-specific code - I did attempt this, but failed, so if anybody has any good ideas on how to achieve this I'm interested!PR checklist
Closes #4812
versions.ymlfile.label