quarto-dev / quarto-cli

Open-source scientific and technical publishing system built on Pandoc.
https://quarto.org
Other
3.7k stars 301 forks source link

Unexpected behavior with parameters when using Jupyter #9189

Open tnederlof opened 5 months ago

tnederlof commented 5 months ago

Bug description

When specifying parameters via the CLI (for a document using Jupyter), if no default value is specified, the document will not render. If a single (even unrelated) parameter default is specified then it works which is unexpected behavior.

Steps to reproduce

For example running the follow results in an error as key1 is not found (get error: NameError: name 'key1' is not defined)

CLI command: quarto render test.qmd -P key1:value2

test.qmd

---
title: "Test"
format:
  html: default
jupyter: python3
---

```{python}
#| tags: [parameters]
print(key1)

However, the following code works (specifying a single default even if its a different key produces the desired functionality):

CLI command: `quarto render test.qmd -P key1:value2`

`test.qmd`

title: "Test" format: html: default jupyter: python3

#| tags: [parameters]
key5 = "value5"
print(key1)

### Expected behavior

I expect to not need to specify default values if I am passing them in through the CLI. Even if default values are required, it should require defaults for all values.

### Actual behavior

Currently, any single default value being specified causes the document to render. This key/value default can be entirely unrelated to the key/value actually being access in the document but it still allows it to run which is unexpected.

### Your environment

OS: Mac OS 14.3.1 

### Quarto check output

Quarto 1.4.542 [✓] Checking versions of quarto binary dependencies... Pandoc version 3.1.11: OK Dart Sass version 1.69.5: OK Deno version 1.37.2: OK [✓] Checking versions of quarto dependencies......OK [✓] Checking Quarto installation......OK Version: 1.4.542 Path: /Applications/quarto/bin

[✓] Checking tools....................OK TinyTeX: v2022.08 Chromium: (not installed)

[✓] Checking LaTeX....................OK Using: TinyTex Path: /Users/trevornederlof/Library/TinyTeX/bin/universal-darwin Version: 2022

[✓] Checking basic markdown render....OK

[✓] Checking Python 3 installation....OK Version: 3.10.4 Path: /Users/trevornederlof/code/sample/api/.venv/bin/python3 Jupyter: 5.7.2 Kernels: python3

[✓] Checking Jupyter engine render....OK

[✓] Checking R installation...........OK Version: 4.2.2 Path: /Library/Frameworks/R.framework/Resources LibPaths:

[✓] Checking Knitr engine render......OK

cscheid commented 5 months ago

What's happening here is that Quarto expects parameter cells to be non-empty. This is documented.

There's no way for Quarto to determine if all defaults have been provided because papermill works (afaict) by variable injection, and there's no registry of all required values and their types.

Ideally, Quarto would check for the presence of an empty code cell and issue a warning. But an empty parameters cell is not a valid Quarto parameterized document.

tnederlof commented 5 months ago

@cscheid ah that makes sense, it was hard for me to tell which part was papermill vs quarto. The documentation does show the parameter cells to be non-empty but to my eyes it looks like defining defaults for each variable. It may help others to say even if you don't want to set default values something needs to be in the cell (at least I didn't take the docs to mean the cell needs to be non-empty).

cscheid commented 5 months ago

It may help others to say even if you don't want to set default values

We say: "To parameterize a document, designate a cell with the tag parameters and provide appropriate default values". We don't allow the lack of default values; that's the point.

tnederlof commented 5 months ago

Thank you, @cscheid that makes sense now, default values are the intended way to use this feature, so I will do that. I started getting confused when I realized that you don't have to set default values, any random value would allow it to still run with parameters that never had a default value specified.

cscheid commented 5 months ago

We should do a better job here of emitting a warning, for sure. But the intended usage is for there to always be some values, in order for the documents to not need parameterization in order to render.