Merge pull request #60 from lsst-sqre/tickets/DM-40865

DM-40865: Fix rendering string values in parameters cell

CHANGELOG.md

@@ -8,7 +8,16 @@ Collect fragments into this file with: scriv collect --version X.Y.Z
 
 <!-- scriv-insert-here -->
 
+<a id='changelog-0.9.2'></a>
+
+## 0.9.2 (2023-09-21)
+
+### 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.
+
 <a id='changelog-0.9.1'></a>
+
 ## 0.9.1 (2023-07-31)
 
 ### Bug fixes

src/timessquare/domain/page.py

@@ -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)
@@ -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
 

tests/data/demo.ipynb

@@ -23,7 +23,8 @@
    "source": [
     "A = 2\n",
     "y0 = 4\n",
-    "lambd = 1"
+    "lambd = 1\n",
+    "title = \"hello world\""
    ]
   },
   {
@@ -103,6 +104,11 @@
      "default": 0,
      "description": "Y-axis offset",
      "type": "number"
+    },
+    "title": {
+     "default": "hello world",
+     "description": "A string value",
+     "type": "string"
     }
    }
   }

tests/domain/page_test.py

@@ -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
@@ -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

tests/handlers/v1/pages_test.py

@@ -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
@@ -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
@@ -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.