Add Quarto and Jupyter (R kernel) rendering support#39
Conversation
Agent-Logs-Url: https://github.com/yihui/gglite/sessions/a9d17d3e-6908-4419-a1f3-ff9e18a63a07 Co-authored-by: yihui <163582+yihui@users.noreply.github.com>
Co-authored-by: yihui <163582+yihui@users.noreply.github.com>
…to workflow Agent-Logs-Url: https://github.com/yihui/gglite/sessions/a9d17d3e-6908-4419-a1f3-ff9e18a63a07 Co-authored-by: yihui <163582+yihui@users.noreply.github.com>
| repr_html.g2 = function(obj, ...) { | ||
| html = c( | ||
| '<!DOCTYPE html>', '<html>', '<head>', | ||
| '<meta charset="utf-8">', | ||
| cdn_scripts(), | ||
| '</head>', '<body>', | ||
| chart_html(obj), | ||
| '</body>', '</html>' | ||
| ) | ||
| paste(html, collapse = '\n') | ||
| } |
There was a problem hiding this comment.
this function is highly similar to print.g2(); refactor and reuse!
There was a problem hiding this comment.
Done in f4f29ed. Extracted g2_html_page(body) helper that constructs the full HTML page from a chart body. Both print.g2 and repr_html.g2 now delegate to it — repr_html.g2 is reduced to a one-liner: function(obj, ...) g2_html_page(chart_html(obj)).
| register_repr_html = function() { | ||
| registerS3method('repr_html', 'g2', repr_html.g2, envir = asNamespace('repr')) | ||
| registerS3method('repr_text', 'g2', repr_text.g2, envir = asNamespace('repr')) | ||
| } | ||
|
|
||
| register_knit_print = function() { | ||
| registerS3method('knit_print', 'g2', knit_print.g2, envir = asNamespace('knitr')) | ||
| } | ||
|
|
||
| .onLoad = function(...) { | ||
| if (isNamespaceLoaded('knitr')) register_knit_print() | ||
| setHook(packageEvent('knitr', 'onLoad'), function(...) register_knit_print()) | ||
| if (isNamespaceLoaded('repr')) register_repr_html() | ||
| setHook(packageEvent('repr', 'onLoad'), function(...) register_repr_html()) | ||
| } |
There was a problem hiding this comment.
too much repetition; refactor and simplify, e.g., write a register_methods(pkg, generic) function so you can
register_methods(c('knitr', 'repr', 'repr'), c('knit_print', 'repr_html', 'repr_text'))There was a problem hiding this comment.
Done in cf2be2b. Added register_methods(pkgs, generics) that loops over parallel vectors, looks up each method by name from the gglite namespace, and both registers immediately (if the package is already loaded) and installs an onLoad hook. .onLoad is now:
register_methods(
c('knitr', 'repr', 'repr'),
c('knit_print', 'repr_html', 'repr_text')
)| on: | ||
| schedule: | ||
| - cron: '0 0 * * 0' # weekly on Sunday at midnight UTC | ||
| workflow_dispatch: |
There was a problem hiding this comment.
the workflow should also run when we make changes to .ipynb or .qmd
There was a problem hiding this comment.
Added in cf2be2b. The workflow now also triggers on push and pull_request when tests/test-gglite.qmd or tests/test-gglite.ipynb change.
| - name: Browser-test Quarto output | ||
| run: | | ||
| python - << 'EOF' | ||
| import asyncio | ||
| from playwright.async_api import async_playwright | ||
| import os | ||
|
|
||
| async def test(): | ||
| async with async_playwright() as p: | ||
| browser = await p.chromium.launch() | ||
| page = await browser.new_page() | ||
|
|
||
| g2_errors = [] | ||
| page.on("pageerror", lambda e: g2_errors.append(str(e))) | ||
|
|
||
| html = os.path.abspath("tests/test-gglite.html") | ||
| await page.goto(f"file://{html}") | ||
| await page.wait_for_timeout(6000) | ||
|
|
||
| canvases = await page.query_selector_all("canvas") | ||
| print(f"Quarto: {len(canvases)} canvas elements") | ||
| g2_errs = [e for e in g2_errors if "G2" in e or "chart" in e.lower()] | ||
| if g2_errs: | ||
| raise AssertionError(f"G2 errors: {g2_errs}") | ||
| assert len(canvases) >= 3, f"Expected >= 3 canvases, got {len(canvases)}" | ||
| print("Quarto test PASSED") | ||
|
|
||
| await page.screenshot(path="/tmp/quarto-test.png", full_page=True) | ||
| await browser.close() | ||
|
|
||
| asyncio.run(test()) | ||
| EOF | ||
|
|
||
| - name: Browser-test Jupyter output (iframe simulation) | ||
| run: | | ||
| python - << 'EOF' | ||
| import asyncio, json, os, tempfile | ||
| from playwright.async_api import async_playwright | ||
|
|
||
| # Load the executed notebook and build an iframe-based test page | ||
| with open("/tmp/test-gglite-executed.ipynb") as f: | ||
| nb = json.load(f) | ||
|
|
||
| # Collect all text/html cell outputs (charts) | ||
| chart_htmls = [] | ||
| for cell in nb["cells"]: | ||
| for out in cell.get("outputs", []): | ||
| if "text/html" in out.get("data", {}): | ||
| chart_htmls.append("".join(out["data"]["text/html"])) | ||
|
|
||
| print(f"Found {len(chart_htmls)} chart outputs in notebook") | ||
| assert len(chart_htmls) >= 3, f"Expected >= 3 chart outputs, got {len(chart_htmls)}" | ||
|
|
||
| # Write each chart as a separate file for iframe loading | ||
| tmp_dir = tempfile.mkdtemp() | ||
| iframe_tags = [] | ||
| for i, html in enumerate(chart_htmls): | ||
| path = os.path.join(tmp_dir, f"chart{i}.html") | ||
| with open(path, "w") as f: | ||
| f.write(html) | ||
| iframe_tags.append( | ||
| f'<iframe src="file://{path}" width="800" height="520" ' | ||
| f'frameborder="0" id="frame{i}"></iframe>' | ||
| ) | ||
|
|
||
| # Wrapper page | ||
| wrapper_path = os.path.join(tmp_dir, "wrapper.html") | ||
| with open(wrapper_path, "w") as f: | ||
| f.write("<!DOCTYPE html><html><body>\n") | ||
| f.write("\n".join(iframe_tags)) | ||
| f.write("\n</body></html>") | ||
|
|
||
| async def test(): | ||
| async with async_playwright() as p: | ||
| browser = await p.chromium.launch() | ||
| page = await browser.new_page() | ||
|
|
||
| errors = [] | ||
| page.on("pageerror", lambda e: errors.append(str(e))) | ||
|
|
||
| await page.goto(f"file://{wrapper_path}") | ||
| await page.wait_for_timeout(8000) | ||
|
|
||
| iframe_canvases = 0 | ||
| for iframe_el in await page.query_selector_all("iframe"): | ||
| frame = await iframe_el.content_frame() | ||
| if frame: | ||
| cs = await frame.query_selector_all("canvas") | ||
| iframe_canvases += len(cs) | ||
|
|
||
| print(f"Jupyter: {iframe_canvases} canvas elements across iframes") | ||
| g2_errs = [e for e in errors if "G2" in e] | ||
| if g2_errs: | ||
| raise AssertionError(f"G2 errors in Jupyter output: {g2_errs}") | ||
| assert iframe_canvases >= 3, \ | ||
| f"Expected >= 3 canvases, got {iframe_canvases}" | ||
| print("Jupyter test PASSED") | ||
|
|
||
| await page.screenshot(path="/tmp/jupyter-test.png", full_page=True) | ||
| await browser.close() | ||
|
|
||
| asyncio.run(test()) | ||
| EOF |
There was a problem hiding this comment.
write these python scripts into files and run them, instead of inlining them in the workflow
There was a problem hiding this comment.
Done in cf2be2b. Created tests/test-quarto.py and tests/test-jupyter.py as standalone scripts. The workflow now runs:
- run: python tests/test-quarto.py tests/test-gglite.html
- run: python tests/test-jupyter.py /tmp/test-gglite-executed.ipynb| # knit session (opts_knit is restored between documents). This avoids the | ||
| # disk-based dependency requirement that htmltools::htmlDependency imposes, | ||
| # which would cause Quarto to error. | ||
| key = 'gglite.scripts_added' |
There was a problem hiding this comment.
move it outside the function so we can reuse it elsewhere like in tests without using a hard-coded string, e.g.,
.knitr.flag = 'gglite.scripts_added'
There was a problem hiding this comment.
Done in cf2be2b. Moved it to a package-level constant: .knitr.flag = 'gglite.scripts_added'. knit_print.g2 now references .knitr.flag instead of a hard-coded string, and tests also use it.
| (grepl('unpkg.com', out1)) | ||
| (!grepl('unpkg.com', out2)) | ||
| # Reset for other tests | ||
| knitr::opts_knit$set('gglite.scripts_added' = NULL) |
There was a problem hiding this comment.
| knitr::opts_knit$set('gglite.scripts_added' = NULL) | |
| knitr::opts_knit$delete(.knitr.flag) |
There was a problem hiding this comment.
Done in cf2be2b: knitr::opts_knit$delete(.knitr.flag).
| assert('repr methods are registered when repr is loaded', { | ||
| loadNamespace('repr') |
There was a problem hiding this comment.
run tests conditionally:
| assert('repr methods are registered when repr is loaded', { | |
| loadNamespace('repr') | |
| if (xfun::loadable('repr')) assert('repr methods are registered when repr is loaded', { |
There was a problem hiding this comment.
Done in cf2be2b: if (xfun::loadable('repr')) assert('repr methods...', { ... }).
|
|
||
| ```{r} | ||
| #| label: scatter | ||
| g2(mtcars, x = 'mpg', y = 'hp') |> mark_point() |
There was a problem hiding this comment.
use the formula interface instead of character strings for variable names when possible, e.g.,
g2(mtcars, hp ~ mpg) |> mark_point()
instead of
g2(mtcars, x = 'mpg', y = 'hp') |> mark_point()
also drop marks that can be automatically inferred, e.g.,
g2(mtcars, hp ~ mpg)
instead of
g2(mtcars, hp ~ mpg) |> mark_point()
write these rules in copilot instructions so future examples follow them
also apply these rules to the examples you created
There was a problem hiding this comment.
Done in cf2be2b. Both .qmd and .ipynb examples now use the formula interface and drop explicit auto-inferable marks:
- Scatter:
g2(mtcars, hp ~ mpg)(wasg2(mtcars, x = 'mpg', y = 'hp') |> mark_point()) - Histogram:
g2(mtcars, ~ mpg)(replaces the broken bar-chart example) - Time series:
g2(sunspot.year)(unchanged)
Also added rules to the copilot instructions: prefer formula interface, drop marks that can be auto-inferred.
| "outputs": [ | ||
| { | ||
| "data": { | ||
| "text/html": [ |
There was a problem hiding this comment.
is it possible to make these plots show on github? while viewing https://github.com/yihui/gglite/blob/copilot/verify-quarto-and-jupyter-support/tests/test-gglite.ipynb, the plots are stripped off
There was a problem hiding this comment.
Unfortunately no — GitHub's notebook viewer strips all <script> tags for security reasons, so interactive JavaScript charts can't render in GitHub's static viewer regardless of the isolated: true metadata. The charts work in JupyterLab/Jupyter Notebook and on nbviewer.org (which allows trusted notebooks), but not in GitHub's built-in preview. Adding base64 PNG screenshots as image/png fallback outputs would make GitHub show something, but would roughly triple the file size.
|
|
||
| ```{r} | ||
| #| label: bar | ||
| g2(mtcars, x = 'cyl') |> mark_interval() |
There was a problem hiding this comment.
this example doesn't work and the plot is blank; how is it missed by your testing?
cyl is a numeric variable; you can't use interval marks on a single numeric variable
btw, while testing, i discovered a serious bug for the automatic histograms for single numeric variables, e.g., g2(mtcars, ~cyl) generates a histogram but the order of bars is wrong: on the x axis, we see tick labels 6, 4, 8, 7, 5, 9 instead of 4, 5, 6, 7, 8, 9; this bug is not unique to this example, e.g., g2(penguins, ~ bill_len) also shows wrong order; besides, for histograms, there shouldn't be gaps between bars, and you may set white borders on the bars as the visual separator between bars
There was a problem hiding this comment.
Fixed in cf2be2b:
-
The broken
g2(mtcars, x = 'cyl') |> mark_interval()example is replaced withg2(mtcars, ~ mpg)(histogram — no explicit mark needed). -
The histogram ordering bug is fixed by switching the auto-mark from
type = 'interval'totype = 'rect'. G2'sintervalmark forces aband(categorical) scale for x, so bins were displayed in data-appearance order. Therectmark uses quantitativex/x1values frombinXand renders them on a linear scale in the correct numeric order. Addedstyle = list(stroke = 'white')for visual separation between bars (no gaps).
…tes, workflow Agent-Logs-Url: https://github.com/yihui/gglite/sessions/e0111d51-e0cb-4cc5-a3c2-349f8230f7f4 Co-authored-by: yihui <163582+yihui@users.noreply.github.com>
…-based waits in tests Agent-Logs-Url: https://github.com/yihui/gglite/sessions/e0111d51-e0cb-4cc5-a3c2-349f8230f7f4 Co-authored-by: yihui <163582+yihui@users.noreply.github.com>
| ^tests/test-gglite\.qmd$ | ||
| ^tests/test-gglite\.ipynb$ | ||
| ^tests/test-quarto\.py$ | ||
| ^tests/test-jupyter\.py$ |
There was a problem hiding this comment.
just ignore all *.qmd, *.ipynb, and *.py under tests/ without listing their basenames in case we need to add more tests in future
There was a problem hiding this comment.
Done in ca0b4f4. Replaced the four specific entries with three wildcard patterns: ^tests/.*\.qmd$, ^tests/.*\.ipynb$, ^tests/.*\.py$.
| paths: | ||
| - 'tests/test-gglite.qmd' | ||
| - 'tests/test-gglite.ipynb' | ||
| pull_request: | ||
| paths: | ||
| - 'tests/test-gglite.qmd' | ||
| - 'tests/test-gglite.ipynb' |
There was a problem hiding this comment.
use wildcards in paths: *.qmd, *.ipynb, and *.py
There was a problem hiding this comment.
Done in ca0b4f4. Paths now use tests/*.qmd, tests/*.ipynb, tests/*.py.
| jupyter nbconvert --to notebook --execute tests/test-gglite.ipynb \ | ||
| --output /tmp/test-gglite-executed.ipynb |
There was a problem hiding this comment.
when running on the main branch and the output differs from original .ipynb, send a PR to update the original
There was a problem hiding this comment.
Done in ca0b4f4 (refined in 4c23667). Added a step that runs only when github.ref == 'refs/heads/main': normalizes the executed notebook to /tmp/test-gglite-normalized.ipynb (strips execution counts/timestamps via tests/normalize-notebook.py), diffs it against the committed file, and if different, commits to a new branch (auto/update-notebook-<run_id>) and opens a PR.
| uses: actions/upload-artifact@HEAD | ||
| with: | ||
| name: browser-screenshots | ||
| path: /tmp/*.png |
There was a problem hiding this comment.
also upload outputs from quarto and jupyter
There was a problem hiding this comment.
Done in ca0b4f4. The upload artifact step now collects /tmp/*.png (screenshots), tests/test-gglite.html (Quarto output), and /tmp/test-gglite-executed.ipynb (executed notebook).
| ```r | ||
| g2(mtcars, ~ mpg) # histogram | ||
| g2(mtcars, ~ cyl) # histogram (cyl is numeric in mtcars) | ||
| ``` |
There was a problem hiding this comment.
add an examplesof using formula in other arguments, e.g., color, size, etc (see PR #36)
There was a problem hiding this comment.
Done in ca0b4f4. Added examples showing color = ~ Species, color = ~ cyl, size = ~ wt, and shape = ~ Species.
| register_knit_print = function() { | ||
| registerS3method('knit_print', 'g2', knit_print.g2, envir = asNamespace('knitr')) | ||
| register_methods = function(pkgs, generics) { | ||
| stopifnot(length(pkgs) == length(generics)) |
There was a problem hiding this comment.
no need to verify; this function is called by ourselves, not user-facing, and we'll ensure lengths are equal
| stopifnot(length(pkgs) == length(generics)) |
There was a problem hiding this comment.
Done in ca0b4f4. Removed the stopifnot line.
| hook = function(...) { | ||
| registerS3method( | ||
| generic, 'g2', | ||
| get(method_name, envir = asNamespace('gglite')), |
There was a problem hiding this comment.
avoid default inheritance
| get(method_name, envir = asNamespace('gglite')), | |
| asNamespace('gglite')[[paste0(generic, '.g2')]], |
There was a problem hiding this comment.
Done in ca0b4f4: asNamespace('gglite')[[paste0(generic, '.g2')]].
| stopifnot(length(pkgs) == length(generics)) | ||
| for (i in seq_along(pkgs)) local({ | ||
| pkg = pkgs[[i]]; generic = generics[[i]] | ||
| method_name = paste0(generic, '.g2') |
There was a problem hiding this comment.
| method_name = paste0(generic, '.g2') |
There was a problem hiding this comment.
Done in ca0b4f4. Removed the method_name local variable; the expression is now inlined directly in the registerS3method() call.
| }) | ||
|
|
||
| if (xfun::loadable('repr')) assert('repr methods are registered when repr is loaded', { | ||
| loadNamespace('repr') |
There was a problem hiding this comment.
xfun::loadable() will load the package
| loadNamespace('repr') |
There was a problem hiding this comment.
Done in ca0b4f4. Removed the if (xfun::loadable('repr')) wrapper — the assert runs unconditionally.
…ister_methods, formula docs Agent-Logs-Url: https://github.com/yihui/gglite/sessions/42090aea-3171-40d2-b40b-07840c78eada Co-authored-by: yihui <163582+yihui@users.noreply.github.com>
…e, safe temp file compare Agent-Logs-Url: https://github.com/yihui/gglite/sessions/42090aea-3171-40d2-b40b-07840c78eada Co-authored-by: yihui <163582+yihui@users.noreply.github.com>
3b88a34 to
fc2d75e
Compare
gglitecharts were silently broken in Quarto and produced no output in Jupyter notebooks via IRkernel.Root causes
knit_print.g2usedhtmltools::htmlDependency(src = c(href = ''))to deduplicate CDN script injection. Quarto's pipeline rejects non-disk-based dependencies with a hard error.repr_html.g2orrepr_text.g2methods. IRkernel uses thereprpackage for MIME-bundle display; it also skips all output (including HTML) whentext/plainis empty.Changes
R/render.Rg2_html_page(body): New helper that constructs a complete HTML page (CDN scripts in<head>, chart body); shared betweenprint.g2andrepr_html.g2..knitr.flag: Package-level constant'gglite.scripts_added'for CDN deduplication, usable in tests without hard-coding the string.knit_print.g2: Dropshtmltools::htmlDependency. CDN scripts are now inlined once per document via aknitr::opts_knitflag (automatically cleared when opts_knit is restored at document end). Works in both R Markdown and Quarto.repr_html.g2: Returns a complete HTML page viag2_html_page(). JupyterLab renders this in a sandboxed iframe.repr_text.g2: Returns a brief label (e.g."G2 chart (point; 32 rows)") so IRkernel emits a non-emptytext/plainMIME entry — required for any rich display to be sent.register_methods(pkgs, generics): DRY helper that registers S3 methods in external namespaces usingasNamespace('gglite')[[paste0(generic, '.g2')]](immediately if loaded, viasetHookotherwise). Replaces the oldregister_repr_html()/register_knit_print()pair..onLoad: Simplified to a singleregister_methods(c('knitr', 'repr', 'repr'), c('knit_print', 'repr_html', 'repr_text'))call.type = 'rect'instead oftype = 'interval'. G2'sintervalmark forces a categorical band scale for x, causing bins to display in data-appearance order. Therectmark uses quantitativex/x1encodings frombinXwith a linear scale, giving correct numeric ordering. Addedstyle = list(stroke = 'white')for visual bar separation with no gaps.DESCRIPTIONrepradded toSuggests; version bumped to 0.0.22.Tests and examples
tests/test-gglite.qmd— scatter, histogram, and time-series examples using the formula interface (g2(mtcars, hp ~ mpg),g2(mtcars, ~ mpg)) with auto-inferred marks.tests/test-gglite.ipynb— same three charts with embeddedtext/htmloutputs.tests/test-quarto.py— standalone Playwright script for browser-testing the Quarto HTML output.tests/test-jupyter.py— standalone Playwright script for browser-testing notebook HTML outputs in iframes.tests/normalize-notebook.py— helper that strips execution counts and timestamps from an executed notebook for clean diffs..Rbuildignorepatterns (tests/.*\.qmd$,tests/.*\.ipynb$,tests/.*\.py$);tests/*.htmladded to.gitignore..knitr.flag),repr_text.g2, andrepr_html.g2..github/copilot-instructions.mdg2(mtcars, hp ~ mpg)), drop explicit marks thatauto_mark()can infer, with examples showingcolor = ~ Species,size = ~ wt, andshape = ~ Speciesfor other aesthetic channels..github/workflows/test-quarto-jupyter.ymlWeekly (Sunday midnight UTC) +
workflow_dispatch+push/pull_requestontests/*.qmd,tests/*.ipynb, andtests/*.pychanges. The job:tests/test-gglite.qmdvia Quarto.tests/test-gglite.ipynbviajupyter nbconvert.tests/test-quarto.pyandtests/test-jupyter.py— asserts canvas elements render and no G2 errors appear.mainand the executed notebook outputs differ from the committed file, automatically opens a PR to updatetests/test-gglite.ipynb.