Skip to content

Commit

Permalink
Fix rendering string values in parameters cell
Browse files Browse the repository at this point in the history
Fix how strings are rendered in the parameters cell of notebooks.
Previously string parameters were missing Python quotes. Now parameters
are passed to Jinja in their `repr` string forms to be proper Python
code.
  • Loading branch information
jonathansick committed Sep 21, 2023
1 parent 3520873 commit 780d23c
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 7 deletions.
3 changes: 3 additions & 0 deletions changelog.d/20230921_133245_jsick_DM_40865.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
### Bug fixes

- Fix how strings are rendered in the parameters cell of notebooks. Previously string parameters were missing Python quotes. Now parameters are passed to Jinja in their `repr` string forms to be proper Python code.
10 changes: 7 additions & 3 deletions src/timessquare/domain/page.py
Original file line number Diff line number Diff line change
Expand Up @@ -461,8 +461,12 @@ def render_parameters(
JSON-encoded notebook source.
"""
# Build Jinja render context
jinja_env = jinja2.Environment(autoescape=True)
jinja_env.globals.update({"params": values})
# Turn off autoescaping to avoid escaping the parameter values
jinja_env = jinja2.Environment(autoescape=False) # noqa: S701
value_code_strings = {
name: repr(value) for name, value in values.items()
}
jinja_env.globals.update({"params": value_code_strings})

# Read notebook and render cell-by-cell
notebook = PageModel.read_ipynb(self.ipynb)
Expand All @@ -483,7 +487,7 @@ def render_parameters(
cell.source = template.render()

# Modify notebook metadata to include values
if "times-square" not in notebook.metadata.keys():
if "times-square" not in notebook.metadata:
notebook.metadata["times-square"] = {}
notebook.metadata["times-square"]["values"] = values

Expand Down
8 changes: 7 additions & 1 deletion tests/data/demo.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@
"source": [
"A = 2\n",
"y0 = 4\n",
"lambd = 1"
"lambd = 1\n",
"title = \"hello world\""
]
},
{
Expand Down Expand Up @@ -103,6 +104,11 @@
"default": 0,
"description": "Y-axis offset",
"type": "number"
},
"title": {
"default": "hello world",
"description": "A string value",
"type": "string"
}
}
}
Expand Down
6 changes: 4 additions & 2 deletions tests/domain/page_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,9 @@ def test_render_parameters() -> None:
title="Demo",
uploader_username="testuser",
)
rendered = page.render_parameters(values={"A": 2, "y0": 1.0, "lambd": 0.5})
rendered = page.render_parameters(
values={"A": 2, "y0": 1.0, "lambd": 0.5, "title": "Demo"}
)
rendered_nb = PageModel.read_ipynb(rendered)

# Check that the markdown got rendered
Expand All @@ -110,7 +112,7 @@ def test_render_parameters() -> None:

# Check that the first code cell got replaced
assert rendered_nb["cells"][1]["source"] == (
"# Parameters\nA = 2\nlambd = 0.5\ny0 = 1.0"
"# Parameters\nA = 2\nlambd = 0.5\ntitle = 'Demo'\ny0 = 1.0"
)

# Check that the second code cell was unchanged
Expand Down
9 changes: 8 additions & 1 deletion tests/handlers/v1/pages_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,18 @@ async def test_pages(client: AsyncClient, respx_mock: respx.Router) -> None:
"description": "Amplitude",
"default": 4,
},
"y0": {"type": "number", "description": "Y-axis offset", "default": 0},
"lambd": {
"type": "number",
"minimum": 0,
"description": "Wavelength",
"default": 2,
},
"title": {
"default": "hello world",
"description": "A string value",
"type": "string",
},
"y0": {"type": "number", "description": "Y-axis offset", "default": 0},
}

# List page summaries
Expand Down Expand Up @@ -121,6 +126,7 @@ async def test_pages(client: AsyncClient, respx_mock: respx.Router) -> None:
"A": 4,
"y0": 0,
"lambd": 2,
"title": "hello world",
}

# Render the page template with some parameters set
Expand All @@ -140,6 +146,7 @@ async def test_pages(client: AsyncClient, respx_mock: respx.Router) -> None:
"A": 2,
"y0": 0,
"lambd": 2,
"title": "hello world",
}

# Try to get HTML rendering; should be unavailable right now.
Expand Down

0 comments on commit 780d23c

Please sign in to comment.