Fix dashboard showing spurious matplotlib repr text next to figures#14645
Merged
Conversation
…11150) Dashboard runs the Jupyter kernel with ipynb-shell-interactivity: all, so every top-level expression in a cell echoes its value. matplotlib/pandas plotting calls return objects (plt.title -> Text, plt.boxplot -> dict of Line2D) whose reprs then leak into the rendered output next to the figure — the html-vs-dashboard difference reported in #11150. HTML is affected too, just less: under last_expr only the final expression's repr echoes. The test asserts those reprs are suppressed in both formats while the figure still renders, and guards the intentional multi-output capability that 'all' gives dashboards (both non-last and last cell values survive). Written test-first: currently fails until the output discard heuristic is broadened to cover these reprs.
…put guard The second cell asserted that two scalar expressions both echo under shell-interactivity: all. That proved multiplicity but not why all is the dashboard default, and the trivial values stretched awkwardly in the card fill layout. Replace it with the documented motivation: in a dashboard one cell is one card, so a card holding both a summary table and a plot must emit both from a single cell. Under all both survive; under HTML's last_expr only the last expression (the graph) is kept and the table is dropped. Element-count assertions on the table and the plotly div capture that contrast directly.
Matches the per-directory convention used across smoke-all: keep .quarto scratch and intermediate .quarto_ipynb files out of git.
…igures Dashboards run the Jupyter kernel with ipynb-shell-interactivity: all, so every top-level expression in a cell echoes its value. matplotlib/pandas plotting calls return objects — plt.title() a Text, plt.boxplot() a dict of Line2D — whose text/plain reprs leaked into the rendered output as spurious text beside the figure (#11150). HTML saw a milder version under last_expr. The existing discard heuristic only caught single-line bracket-wrapped reprs and a few library prefixes, missing Text(...) (no leading bracket) and the multi-line boxplot/hist dict. Broaden it to also drop Text(...) and {...} collections of matplotlib artists, still only when the same cell emits an image, so the intentional multi-output-per-card capability that 'all' enables is preserved. Guarded by a unit test on the heuristic (contract inputs captured verbatim from a real executed notebook) plus the existing dashboard+html smoke-all regression.
…tput
The broadened heuristic matched any bracket-wrapped text across newlines and any repr starting with "Text(", so a cell that emitted a figure alongside a legitimate multi-line list/tuple — or a non-matplotlib value such as rich's Text('...') — had that output silently discarded.
Restore the single-line gate for the bracket match, suppress multi-line output only when it references a matplotlib object (the boxplot dict of Line2D), and require matplotlib's leading numeric coordinate for the Text(...) case. Adds unit negatives covering the multi-line-bracket and non-matplotlib Text( paths.
Collaborator
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
… figure
The multi-line branch of the discard heuristic already caught boxplot's dict-of-Line2D repr when IPython pretty-prints it across lines, but a short dict small enough to stay on one line (e.g. a single-entry boxplot/hist result) fell into the single-line branch, which only matched bracket wrappers starting with <, (, or [ — not {. Extend the single-line case to also discard a {...} repr that references a matplotlib object.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Dashboards run the Jupyter kernel with
ipynb-shell-interactivity: all, so every top-level expression in a cell echoes its value. matplotlib/pandas plotting calls return objects —plt.title()aText,plt.boxplot()a dict ofLine2D— whosetext/plainrepr leaked into the rendered output as spurious text beside the figure. HTML output showed a milder version underlast_expr.Root Cause
The discard heuristic in
isDiscardableTextExecuteResult(src/core/jupyter/jupyter.ts) only caught single-line bracket-wrapped reprs and a handful of library prefixes. It missedText(...)(no leading bracket), the multi-line dict returned byboxplot()/hist(), and a single-line dict small enough for IPython to keep on one line.Broadening the match without care risks discarding real output: a cell that emits a figure alongside a legitimate multi-line list/tuple, or a non-matplotlib value like rich's
Text('...'), would have that output silently dropped too.Fix
Text(...)only when it leads with matplotlib's numeric coordinate signature (Text(0.5, ...)), not anyText(...)repr.{...}dicts the same way, covering the case where the artifact dict is short enough to stay on one line.Guarded by unit tests on the heuristic (contract inputs captured from real executed notebooks) plus a dashboard+HTML smoke-all regression.
Fixes #11150