From bdf594c1febbc42b0084f1c665f7c74f1eb38f85 Mon Sep 17 00:00:00 2001 From: seem Date: Mon, 27 Jul 2026 21:54:45 +0200 Subject: [PATCH 01/26] `current_execute_request` helper --- .../python_files/posit/positron/execute_request.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/extensions/positron-python/python_files/posit/positron/execute_request.py b/extensions/positron-python/python_files/posit/positron/execute_request.py index cdbf6fb93e2..d2f6dd5f4c0 100644 --- a/extensions/positron-python/python_files/posit/positron/execute_request.py +++ b/extensions/positron-python/python_files/posit/positron/execute_request.py @@ -4,7 +4,7 @@ # import logging -from typing import Optional, Union +from typing import Optional, Union, cast from ._vendor.pydantic import ( BaseModel, @@ -80,3 +80,13 @@ def from_message(cls, message: dict) -> "PositronExecuteRequest": except ValidationError: logger.debug("Failed to parse positron execute request", exc_info=True) return cls() + + +def current_execute_request() -> PositronExecuteRequest: + """The Positron `execute_request` currently being handled, if any.""" + # Imported lazily to avoid a circular import. + from .positron_ipkernel import PositronIPyKernel + + kernel = cast("PositronIPyKernel", PositronIPyKernel.instance()) + execute_request_message = kernel.get_parent("shell") + return PositronExecuteRequest.from_message(execute_request_message) From e9e33f783e5d0a668e7783d5985cd38200c9bfdb Mon Sep 17 00:00:00 2001 From: seem Date: Tue, 28 Jul 2026 19:04:53 +0200 Subject: [PATCH 02/26] add `PositronExecuteRequest.figure_size` --- .../python_files/posit/positron/execute_request.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/extensions/positron-python/python_files/posit/positron/execute_request.py b/extensions/positron-python/python_files/posit/positron/execute_request.py index d2f6dd5f4c0..cfae1c69fbd 100644 --- a/extensions/positron-python/python_files/posit/positron/execute_request.py +++ b/extensions/positron-python/python_files/posit/positron/execute_request.py @@ -58,6 +58,13 @@ class PositronExecuteRequest(BaseModel): None, description="Output area device pixel ratio, e.g. 1.0 or 2.0" ) + @property + def figure_size(self) -> Optional[tuple[float, float]]: + """The figure size in inches, if specified.""" + if self.fig_width is not None and self.fig_height is not None: + return (self.fig_width, self.fig_height) + return None + @classmethod def from_message(cls, message: dict) -> "PositronExecuteRequest": """Parse from a Jupyter shell message.""" From 7688d299c2705761f06cb8bade96ea8783555024 Mon Sep 17 00:00:00 2001 From: seem Date: Tue, 28 Jul 2026 17:09:11 +0200 Subject: [PATCH 03/26] move console matplotlib backend --- extensions/positron-python/.gitignore | 2 +- .../positron/matplotlib_backend/__init__.py | 4 + .../console.py} | 106 +++++++++++++----- .../posit/positron/positron_ipkernel.py | 15 +-- .../tests/test_matplotlib_backend/__init__.py | 4 + .../test_console.py} | 44 +++++++- 6 files changed, 126 insertions(+), 49 deletions(-) create mode 100644 extensions/positron-python/python_files/posit/positron/matplotlib_backend/__init__.py rename extensions/positron-python/python_files/posit/positron/{matplotlib_backend.py => matplotlib_backend/console.py} (82%) create mode 100644 extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/__init__.py rename extensions/positron-python/python_files/posit/positron/tests/{test_plots.py => test_matplotlib_backend/test_console.py} (94%) diff --git a/extensions/positron-python/.gitignore b/extensions/positron-python/.gitignore index cf75d9cbe29..78c269b907a 100644 --- a/extensions/positron-python/.gitignore +++ b/extensions/positron-python/.gitignore @@ -51,7 +51,7 @@ dist/** l10n/ tags # --- Start Positron --- -python_files/posit/positron/tests/images +python_files/posit/positron/tests/test_matplotlib_backend/images python_files/posit/positron/_vendor/** scripts/*.js resources/pet/** diff --git a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/__init__.py b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/__init__.py new file mode 100644 index 00000000000..1d40d5998be --- /dev/null +++ b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/__init__.py @@ -0,0 +1,4 @@ +# +# Copyright (C) 2026 Posit Software, PBC. All rights reserved. +# Licensed under the Elastic License 2.0. See LICENSE.txt for license information. +# diff --git a/extensions/positron-python/python_files/posit/positron/matplotlib_backend.py b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/console.py similarity index 82% rename from extensions/positron-python/python_files/posit/positron/matplotlib_backend.py rename to extensions/positron-python/python_files/posit/positron/matplotlib_backend/console.py index 3a11a7c33a3..6ac10862bdf 100644 --- a/extensions/positron-python/python_files/posit/positron/matplotlib_backend.py +++ b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/console.py @@ -1,5 +1,5 @@ # -# Copyright (C) 2024 Posit Software, PBC. All rights reserved. +# Copyright (C) 2024-2026 Posit Software, PBC. All rights reserved. # Licensed under the Elastic License 2.0. See LICENSE.txt for license information. # """ @@ -24,22 +24,20 @@ import matplotlib import matplotlib.pyplot as plt +from IPython.core.getipython import get_ipython from matplotlib.backend_bases import FigureManagerBase from matplotlib.backends.backend_agg import FigureCanvasAgg -from .execute_request import PositronExecuteRequest +from ..execute_request import PositronExecuteRequest if TYPE_CHECKING: from matplotlib.figure import Figure - from .plot_comm import PlotSize + from ..plot_comm import PlotSize logger = logging.getLogger(__name__) - -# Enable interactive mode (i.e. redraw after every plotting command). -# This is expected to run when the backend is selected. See the note at the top of the file. -matplotlib.interactive(True) # noqa: FBT003 +BACKEND_NAME = "module://positron.matplotlib_backend.console" # High-level libraries that build on matplotlib. A figure produced by one of these is @@ -80,7 +78,7 @@ def _detect_plotting_library() -> str: return "matplotlib" -def detach_library_figures() -> None: +def _detach_library_figures() -> None: """ Detach high-level library figures (e.g. seaborn) from matplotlib's global registry. @@ -95,17 +93,20 @@ def detach_library_figures() -> None: Plain matplotlib figures are intentionally left in the registry to preserve Positron's cross-cell figure persistence (updating or re-showing a plot across cells). """ - from matplotlib._pylab_helpers import Gcf + try: + from matplotlib._pylab_helpers import Gcf - for manager in list(Gcf.get_all_fig_managers()): - if ( - isinstance(manager, FigureManagerPositron) - and manager.plotting_library in _DETACH_AFTER_CELL_KINDS - ): - # Removes the manager from the registry and invokes our no-op `destroy`. - # Pass the manager rather than its number: numbers freed here can be reused - # by matplotlib, so destroying by number could hit a different figure. - Gcf.destroy(manager) + for manager in list(Gcf.get_all_fig_managers()): + if ( + isinstance(manager, FigureManagerPositron) + and manager.plotting_library in _DETACH_AFTER_CELL_KINDS + ): + # Removes the manager from the registry and invokes our no-op `destroy`. + # Pass the manager rather than its number: numbers freed here can be reused + # by matplotlib, so destroying by number could hit a different figure. + Gcf.destroy(manager) + except Exception: + logger.exception("Error detaching high-level library figures") class FigureManagerPositron(FigureManagerBase): @@ -128,8 +129,8 @@ class FigureManagerPositron(FigureManagerBase): canvas: FigureCanvasPositron def __init__(self, canvas: FigureCanvasPositron, num: int | str): - from .plot_comm import PlotOrigin, PlotRange - from .positron_ipkernel import PositronIPyKernel + from ..plot_comm import PlotOrigin, PlotRange + from ..positron_ipkernel import PositronIPyKernel super().__init__(canvas, num) @@ -342,7 +343,10 @@ def _hash_buffer_rgba(self) -> str: return hashlib.sha1(self.buffer_rgba()).hexdigest() -_library_gca_redirect_installed = False +# The original `plt.gca` saved when we install our redirect, so `deactivate` can restore it, +# and the wrapper we installed, so `deactivate` only restores when ours is still in place. +_original_gca = None +_installed_gca = None def _install_library_gca_redirect() -> None: @@ -362,14 +366,14 @@ def _install_library_gca_redirect() -> None: and only fires across libraries, so plain matplotlib figures remain reusable (preserving cross-cell persistence). See https://github.com/posit-dev/positron/issues/8898. """ - global _library_gca_redirect_installed - if _library_gca_redirect_installed: + global _original_gca, _installed_gca + if _installed_gca is not None: return import matplotlib.pyplot as plt from matplotlib._pylab_helpers import Gcf - original_gca = plt.gca + _original_gca = plt.gca def gca(*args, **kwargs): manager = Gcf.get_active() @@ -381,15 +385,61 @@ def gca(*args, **kwargs): # Drawing library differs from the active figure's library: start fresh so # the existing figure isn't overwritten. return plt.figure().gca() - return original_gca(*args, **kwargs) + return _original_gca(*args, **kwargs) - plt.gca = gca - _library_gca_redirect_installed = True + plt.gca = _installed_gca = gca -_install_library_gca_redirect() +def _uninstall_library_gca_redirect() -> None: + """Restore the original `plt.gca`, undoing `_install_library_gca_redirect`.""" + global _original_gca, _installed_gca + if _installed_gca is None: + return + + import matplotlib.pyplot as plt + + # Only restore if our redirect is still the installed `gca`; something else may have + # patched over it, in which case we leave that patch alone. + if plt.gca is _installed_gca: + plt.gca = _original_gca + + _original_gca = None + _installed_gca = None # Fulfill the matplotlib backend API. FigureCanvas = FigureCanvasPositron FigureManager = FigureManagerPositron + + +def activate() -> None: + shell = get_ipython() + if shell is None: + logger.warning("No IPython shell found; matplotlib console backend not activated") + return + + # Enable interactive mode (i.e. redraw after every plotting command). + matplotlib.interactive(True) # noqa: FBT003 + + # Detach high-level plotting library figures (e.g. seaborn) so that re-running + # plotting code starts a fresh figure instead of drawing onto the previous one. + # See https://github.com/posit-dev/positron/issues/8898. + shell.events.register("post_execute", _detach_library_figures) + + _install_library_gca_redirect() + + +def deactivate() -> None: + shell = get_ipython() + if shell is None: + logger.warning("No IPython shell found; matplotlib console backend not deactivated") + return + + shell.events.unregister("post_execute", _detach_library_figures) + + _uninstall_library_gca_redirect() + + +# This is expected to run when the backend is selected. See the note at the top of the file. +if matplotlib.get_backend() == BACKEND_NAME: + activate() diff --git a/extensions/positron-python/python_files/posit/positron/positron_ipkernel.py b/extensions/positron-python/python_files/posit/positron/positron_ipkernel.py index ba1e88bc71c..fd482682f99 100644 --- a/extensions/positron-python/python_files/posit/positron/positron_ipkernel.py +++ b/extensions/positron-python/python_files/posit/positron/positron_ipkernel.py @@ -397,19 +397,6 @@ def _handle_post_run_cell(self, result: ExecutionResult) -> None: except Exception: logger.exception("Error polling variables") - # Detach high-level plotting library figures (e.g. seaborn) so that re-running - # plotting code starts a fresh figure instead of drawing onto the previous one. - # See https://github.com/posit-dev/positron/issues/8898. Only act if matplotlib - # has already imported our backend: importing it here would run its module-level - # setup code, so we reuse the already-loaded module instead. - if "positron.matplotlib_backend" in sys.modules: - try: - from .matplotlib_backend import detach_library_figures - - detach_library_figures() - except Exception: - logger.exception("Error detaching plotting library figures") - def _add_editor_dir_to_sys_path(self) -> str | None: """ Add the directory of the executed file to sys.path. @@ -787,7 +774,7 @@ def init_gui_pylab(self): # Matplotlib uses the MPLBACKEND environment variable to determine the backend to use. # It imports the backend module when it's first needed. if self.session_mode != SessionMode.NOTEBOOK and not os.environ.get("MPLBACKEND"): - os.environ["MPLBACKEND"] = "module://positron.matplotlib_backend" + os.environ["MPLBACKEND"] = "module://positron.matplotlib_backend.console" return super().init_gui_pylab() diff --git a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/__init__.py b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/__init__.py new file mode 100644 index 00000000000..1d40d5998be --- /dev/null +++ b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/__init__.py @@ -0,0 +1,4 @@ +# +# Copyright (C) 2026 Posit Software, PBC. All rights reserved. +# Licensed under the Elastic License 2.0. See LICENSE.txt for license information. +# diff --git a/extensions/positron-python/python_files/posit/positron/tests/test_plots.py b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_console.py similarity index 94% rename from extensions/positron-python/python_files/posit/positron/tests/test_plots.py rename to extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_console.py index 4129de80d6e..352537145f0 100644 --- a/extensions/positron-python/python_files/posit/positron/tests/test_plots.py +++ b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_console.py @@ -13,12 +13,13 @@ import pytest from PIL import Image +from positron.matplotlib_backend.console import activate, deactivate from positron.plot_comm import PlotRenderFormat, PlotSize, PlotUnit from positron.plots import PlotsService from positron.positron_ipkernel import PositronIPyKernel, _CommTarget -from .conftest import DummyComm, PositronShell -from .utils import ( +from ..conftest import DummyComm, PositronShell +from ..utils import ( comm_close_message, json_rpc_request, json_rpc_response, @@ -34,9 +35,14 @@ @pytest.fixture(autouse=True) def setup_positron_matplotlib_backend() -> None: + prev = matplotlib.get_backend() # The backend is set in the kernel app, which isn't currently available in our tests, # so set it here too. - matplotlib.use("module://positron.matplotlib_backend") + matplotlib.use("module://positron.matplotlib_backend.console") + activate() + yield + deactivate() + matplotlib.use(prev) @pytest.fixture(autouse=True) @@ -560,7 +566,9 @@ def test_mpl_seaborn_figure_detached_after_cell( """ # Create the figure as if it were produced by seaborn (detection is exercised # separately in test_mpl_detect_library_walks_call_stack). - monkeypatch.setattr("positron.matplotlib_backend._detect_plotting_library", lambda: "seaborn") + monkeypatch.setattr( + "positron.matplotlib_backend.console._detect_plotting_library", lambda: "seaborn" + ) plot_comm = _create_mpl_plot(shell, plots_service) # Running any cell triggers the post-cell detach. @@ -590,7 +598,9 @@ def test_mpl_seaborn_does_not_overwrite_existing_matplotlib_figure( assert plt.get_fignums() == [1] # Simulate a seaborn call resolving its axes via plt.gca(). - monkeypatch.setattr("positron.matplotlib_backend._detect_plotting_library", lambda: "seaborn") + monkeypatch.setattr( + "positron.matplotlib_backend.console._detect_plotting_library", lambda: "seaborn" + ) axes = plt.gca() # gca returned axes on a new figure (2), leaving the matplotlib figure (1) intact. @@ -644,7 +654,7 @@ def test_mpl_detect_library_walks_call_stack() -> None: See https://github.com/posit-dev/positron/issues/8898. """ - from positron.matplotlib_backend import _detect_plotting_library + from positron.matplotlib_backend.console import _detect_plotting_library # A plain call site is attributed to matplotlib. assert _detect_plotting_library() == "matplotlib" @@ -656,6 +666,28 @@ def test_mpl_detect_library_walks_call_stack() -> None: assert namespace["call"](_detect_plotting_library) == "seaborn" +def test_mpl_deactivate_restores_gca() -> None: + """`deactivate` restores the original `plt.gca` that `activate` patched over.""" + import matplotlib.pyplot as plt + + from positron.matplotlib_backend import console + + # The autouse fixture already activated, installing our redirect. + assert console._installed_gca is not None # noqa: SLF001 + installed_gca = plt.gca + original_gca = console._original_gca # noqa: SLF001 + + console.deactivate() + + assert console._installed_gca is None # noqa: SLF001 + assert plt.gca is original_gca + assert plt.gca is not installed_gca + + # Re-activate so later tests keep the redirect installed and the fixture's teardown + # `deactivate` (a no-op once uninstalled) stays balanced. + console.activate() + + def test_mpl_shutdown(shell: PositronShell, plots_service: PlotsService) -> None: plot_comms = [_create_mpl_plot(shell, plots_service) for _ in range(2)] From e1abd4af9ce828bfd21a7355df5e815ef2949146 Mon Sep 17 00:00:00 2001 From: seem Date: Fri, 24 Jul 2026 17:22:35 +0200 Subject: [PATCH 04/26] python: respect quarto figure size settings --- .../positron/matplotlib_backend/console.py | 8 +- .../positron/matplotlib_backend/notebook.py | 205 ++++++++++++++++++ .../posit/positron/positron_ipkernel.py | 18 +- .../test_matplotlib_backend/test_notebook.py | 190 ++++++++++++++++ .../python_files/posit/positron/utils.py | 8 + 5 files changed, 424 insertions(+), 5 deletions(-) create mode 100644 extensions/positron-python/python_files/posit/positron/matplotlib_backend/notebook.py create mode 100644 extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_notebook.py diff --git a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/console.py b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/console.py index 6ac10862bdf..3ac937f0525 100644 --- a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/console.py +++ b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/console.py @@ -3,7 +3,7 @@ # Licensed under the Elastic License 2.0. See LICENSE.txt for license information. # """ -The matplotlib backend for Positron. +Positron's matplotlib backend for console sessions. NOTE: DO NOT DIRECTLY IMPORT THIS MODULE! @@ -343,8 +343,7 @@ def _hash_buffer_rgba(self) -> str: return hashlib.sha1(self.buffer_rgba()).hexdigest() -# The original `plt.gca` saved when we install our redirect, so `deactivate` can restore it, -# and the wrapper we installed, so `deactivate` only restores when ours is still in place. +# The original and installed `plt.gca`, so `deactivate` can undo our changes. _original_gca = None _installed_gca = None @@ -413,6 +412,7 @@ def _uninstall_library_gca_redirect() -> None: def activate() -> None: + """Activate the Positron matplotlib console backend.""" shell = get_ipython() if shell is None: logger.warning("No IPython shell found; matplotlib console backend not activated") @@ -430,6 +430,7 @@ def activate() -> None: def deactivate() -> None: + """Deactivate the Positron matplotlib console backend.""" shell = get_ipython() if shell is None: logger.warning("No IPython shell found; matplotlib console backend not deactivated") @@ -440,6 +441,7 @@ def deactivate() -> None: _uninstall_library_gca_redirect() +# If we are the selected backend, activate. # This is expected to run when the backend is selected. See the note at the top of the file. if matplotlib.get_backend() == BACKEND_NAME: activate() diff --git a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/notebook.py b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/notebook.py new file mode 100644 index 00000000000..345dbdf2caa --- /dev/null +++ b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/notebook.py @@ -0,0 +1,205 @@ +# +# Copyright (C) 2026 Posit Software, PBC. All rights reserved. +# Licensed under the Elastic License 2.0. See LICENSE.txt for license information. +# +""" +Positron's matplotlib backend for notebook sessions. + +This backend is designed to match Jupyter's `matplotlib_inline` backend, except where deviating +would provide a better user experience in Positron. + +NOTE: DO NOT DIRECTLY IMPORT THIS MODULE! + +This module assumes that it is only ever imported by matplotlib when it sets its backend. +Given that, it doesn't check whether matplotlib is installed in the user's environment, +and runs code on import e.g. to enable matplotlib interactive mode. This is the same approach +taken by IPython's matplotlib-inline backend, and seems to be the only way to run code when +the backend is set by matplotlib. +""" + +from __future__ import annotations + +import io +import logging +from binascii import b2a_base64 +from typing import TYPE_CHECKING, Literal + +import matplotlib +import matplotlib.pyplot as plt +from IPython.core.getipython import get_ipython +from IPython.display import display +from matplotlib._pylab_helpers import Gcf +from matplotlib.backend_bases import FigureManagerBase +from matplotlib.backends.backend_agg import FigureCanvasAgg +from matplotlib.figure import Figure + +from ..execute_request import PositronExecuteRequest, current_execute_request +from ..utils import png_pixel_size + +if TYPE_CHECKING: + from IPython.core.formatters import PNGFormatter + from IPython.core.interactiveshell import InteractiveShell + +logger = logging.getLogger(__name__) + +BACKEND_NAME = "module://positron.matplotlib_backend.notebook" + + +def new_figure_manager( + num: int | str, + *args, + FigureClass=Figure, # noqa: N803 + # 3-tuple with unit is only available in matplotlib >= 3.11. + figsize: tuple[int, int] | tuple[float, float, Literal["in", "cm", "px"]] | None = None, + **kwargs, +) -> FigureManagerPositronNotebook: + """Called by matplotlib when a new figure is created.""" + # Get the current execute request. + execute_request = current_execute_request() + + # Prefer the user's explicit figsize (e.g. plt.figure(figsize=...) or plt.subplots(figsize=...)) + # over the execute request's figure size. + figsize = figsize or execute_request.figure_size + figure = FigureClass(*args, figsize=figsize, **kwargs) + + # Also provide the execute request to the figure manager. + manager: FigureManagerPositronNotebook = FigureCanvasPositronNotebook.new_manager(figure, num) + manager.set_execute_request(execute_request) + + return manager + + +class FigureManagerPositronNotebook(FigureManagerBase): + canvas: FigureCanvasPositronNotebook + + def show(self): + """Called by matplotlib when a figure is shown via `plt.show()` or `figure.show()`.""" + display(self.canvas.figure) + + @classmethod + def pyplot_show(cls, *, block: bool | None = None) -> None: + """Called by by matplotlib when a user calls `plt.show()`.""" + try: + super().pyplot_show(block=block) + finally: + # Close all figures after showing them. + if Gcf.get_all_fig_managers(): + plt.close("all") + + def set_execute_request(self, execute_request: PositronExecuteRequest): + """Set the current execute request.""" + self.execute_request = execute_request + self.canvas.set_execute_request(execute_request) + + +class FigureCanvasPositronNotebook(FigureCanvasAgg): + manager_class = FigureManagerPositronNotebook + + def set_execute_request(self, execute_request: PositronExecuteRequest) -> None: + """Set the current execute request.""" + # Set the device pixel ratio to the execute request's value, if provided. + if (pixel_ratio := execute_request.output_pixel_ratio) is not None: + self._set_device_pixel_ratio(pixel_ratio) + + +def _display_figure(fig: Figure, *, format_="png") -> tuple[str, dict] | None: + """Render a figure to its Jupyter display wire format.""" + # NOTE: This implementation must match `IPython.core.pylabtools.print_figure` otherwise + # users will get different results in Positron vs other notebook editors. + + # Don't display empty figures; mirrors IPython. + if not fig.axes and not fig.lines: + return None + + # Render the figure to bytes. + canvas = fig.canvas + with io.BytesIO() as figure_buffer: + canvas.print_figure( + figure_buffer, + format=format_, + # Must pass `fig.dpi` otherwise `print_figure` renders at a pixel ratio of 1. + dpi=fig.dpi, + # Tight bbox mirrors IPython. + bbox_inches="tight", + ) + data = figure_buffer.getvalue() + + # Decode bytes to string. + if format_ == "svg": + decoded = data.decode("utf-8") + else: + decoded = b2a_base64(data, newline=False).decode("ascii") + + # Prepare figure metadata. + metadata = {} + + # If the canvas has a custom device pixel ratio, include the intended pixel size in the metadata. + if (ratio := canvas.device_pixel_ratio) != 1: + w, h = png_pixel_size(data) + metadata["width"] = int(w) / ratio + metadata["height"] = int(h) / ratio + + return decoded, metadata + + +# Fulfil the matplotlib backend API. +FigureCanvas = FigureCanvasPositronNotebook +FigureManager = FigureManagerPositronNotebook + + +def _get_png_formatter(shell: InteractiveShell) -> PNGFormatter: + """Get the shell's PNG formatter.""" + return shell.display_formatter.formatters["image/png"] + + +def _show_figures(): + """Post execute hook to show all figures and log errors.""" + try: + return FigureManagerPositronNotebook.pyplot_show() + except Exception: + logger.exception("Error showing figures in post execute hook") + + +def activate() -> None: + """Activate the Positron matplotlib notebook backend.""" + shell = get_ipython() + if shell is None: + logger.warning("No IPython shell found; matplotlib notebook backend not activated") + return + + # I think the only part of enable_matplotlib_integration that we really need + # is to set interactive mode and to register the flush hook (below). + matplotlib.interactive(True) # noqa: FBT003 + + # Register a hook to show all figures after cell execution. + shell.events.register("post_execute", _show_figures) + + # Register our formatter for matplotlib Figure objects. + _get_png_formatter(shell).for_type(Figure, _display_figure) + + +def deactivate() -> None: + """Deactivate the Positron matplotlib notebook backend.""" + shell = get_ipython() + if shell is None: + logger.warning("No IPython shell found; matplotlib notebook backend not deactivated") + return + + # Unregister the post execute hook. + shell.events.unregister("post_execute", _show_figures) + + # Unregister our formatter for matplotlib Figure objects. + png_formatter = _get_png_formatter(shell) + try: + if png_formatter.lookup(Figure) is _display_figure: + # It's still our formatter, remove it. + png_formatter.pop(Figure) + except KeyError: + # `lookup` raises if there's no formatter for the type, nothing to do. + pass + + +# If we are the selected backend, activate. +# This is expected to run when the backend is selected. See the note at the top of the file. +if matplotlib.get_backend() == BACKEND_NAME: + activate() diff --git a/extensions/positron-python/python_files/posit/positron/positron_ipkernel.py b/extensions/positron-python/python_files/posit/positron/positron_ipkernel.py index fd482682f99..824d31daf12 100644 --- a/extensions/positron-python/python_files/posit/positron/positron_ipkernel.py +++ b/extensions/positron-python/python_files/posit/positron/positron_ipkernel.py @@ -768,13 +768,27 @@ def init_control(self, context): self.control_thread.join = with_logging(self.control_thread.join) return result + def init_extensions(self): + # Include Positron built-in extensions in default_extensions, which: + # 1. Always loads, regardless of user config, and + # 2. Loads before user-configured extensions. + self.default_extensions = [ + # Parent default_extensions. + *self.default_extensions, + # Positron built-in extensions. + ] + super().init_extensions() + def init_gui_pylab(self): # Enable the Positron matplotlib backend if we're not in a notebook. # If we're in a notebook, use IPython's default backend via the super() call below. # Matplotlib uses the MPLBACKEND environment variable to determine the backend to use. # It imports the backend module when it's first needed. - if self.session_mode != SessionMode.NOTEBOOK and not os.environ.get("MPLBACKEND"): - os.environ["MPLBACKEND"] = "module://positron.matplotlib_backend.console" + if not os.environ.get("MPLBACKEND"): + if self.session_mode == SessionMode.NOTEBOOK: + os.environ["MPLBACKEND"] = "module://positron.matplotlib_backend.notebook" + else: + os.environ["MPLBACKEND"] = "module://positron.matplotlib_backend.console" return super().init_gui_pylab() diff --git a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_notebook.py b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_notebook.py new file mode 100644 index 00000000000..5deb14c36e3 --- /dev/null +++ b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_notebook.py @@ -0,0 +1,190 @@ +# +# Copyright (C) 2026 Posit Software, PBC. All rights reserved. +# Licensed under the Elastic License 2.0. See LICENSE.txt for license information. +# + +from __future__ import annotations + +import base64 +from typing import TYPE_CHECKING + +import matplotlib +import pytest +from IPython.utils.capture import RichOutput, capture_output + +from positron.matplotlib_backend.notebook import activate, deactivate +from positron.session_mode import SessionMode +from positron.utils import png_pixel_size + +from ..utils import run_with_metadata + +if TYPE_CHECKING: + from matplotlib.figure import Figure + + from positron.positron_ipkernel import PositronShell + + +DEFAULT_FIGSIZE: list[float] = matplotlib.rcParamsDefault["figure.figsize"] + + +@pytest.fixture(autouse=True) +def _setup(shell, monkeypatch): + """Configure the shell to match the notebook environment.""" + monkeypatch.setattr(shell, "session_mode", SessionMode.NOTEBOOK) + + +@pytest.fixture +def backend(shell): + """A fixture that configures matplotlib to use the Positron notebook backend.""" + prev = matplotlib.get_backend() + matplotlib.use("module://positron.matplotlib_backend.notebook") + activate() + + yield NotebookBackendFixture(shell) + + deactivate() + matplotlib.use(prev) + + +def _parse_png_output(output: RichOutput) -> tuple[bytes, dict]: + result = output._repr_png_() + data, metadata = result if len(result) == 2 else (result, {}) + png = base64.b64decode(data) + return png, metadata + + +class PlotResult: + def __init__(self, figure: Figure, png: bytes, metadata: dict): + self.figure = figure + self.png = png + self.metadata = metadata + + @property + def figure_size(self): + return self.figure.get_size_inches().tolist() + + @property + def png_pixel_size(self): + return png_pixel_size(self.png) + + +class NotebookBackendFixture: + def __init__(self, shell: PositronShell) -> None: + self.shell = shell + + def import_matplotlib(self) -> None: + self.shell.run_cell("import matplotlib.pyplot as plt").raise_error() + + def plot( + self, + *, + do_import=True, + figsize: tuple[float, float] | None = None, + meta: dict | None = None, + ): + # Create the code snippet. + lines = [] + if do_import: + lines.append("import matplotlib.pyplot as plt") + subplots_args = f"figsize={figsize}" if figsize is not None else "" + lines.append(f"fig, ax = plt.subplots({subplots_args})") + lines.append("ax.plot([0, 1], [0, 1])") + code = "\n".join(lines) + + # Run the code and capture the output. + with capture_output() as captured: + run_with_metadata(self.shell, code, meta) + + # Return the created plot. + assert len(captured.outputs) == 1 + png, metadata = _parse_png_output(captured.outputs[0]) + return PlotResult(figure=self.shell.user_ns["fig"], png=png, metadata=metadata) + + +def test_backend_activates(backend): # noqa: ARG001 + assert matplotlib.get_backend() == "module://positron.matplotlib_backend.notebook" + + +def test_figure_size_set_when_matplotlib_already_imported(backend): + """A figure made after matplotlib was imported in an earlier cell is resized.""" + # NOTE: We can't easily guarantee that matplotlib isn't imported since our tests use + # an in-process shell that shares imported modules with the test runner. + # This also makes it hard to test the case where matplotlib is imported + # in the same cell as the figure is created - so we test that manually for now. + backend.import_matplotlib() + result = backend.plot(do_import=False, meta={"fig-width": 8, "fig-height": 4}) + + assert result.figure_size == [8.0, 4.0] + + +def test_explicit_figsize_wins(backend): + """An explicit figsize in the cell beats the pending default.""" + result = backend.plot(figsize=(1, 1), meta={"fig-width": 8, "fig-height": 4}) + assert result.figure_size == [1.0, 1.0] + + +def test_figure_size_does_not_leak_to_later_cell(backend): + """A sized cell does not leak into a later unsized cell.""" + backend.plot(meta={"fig-width": 8, "fig-height": 4}) + assert backend.plot().figure_size == DEFAULT_FIGSIZE + + +@pytest.mark.parametrize( + "meta", + [ + # Lone dimension is a no-op. + {"fig-width": 1}, + {"fig-height": 1}, + # Non-positive dimension is a no-op. + {"fig-width": 0}, + {"fig-height": 0}, + {"fig-width": -1}, + ], +) +def test_figure_size_noop(backend, meta): + """A lone fig-height leaves the figure at the default size.""" + assert backend.plot(meta=meta).figure_size == DEFAULT_FIGSIZE + + +def test_pixel_ratio_scales_png_size_and_attaches_metadata(backend): + """A pixel ratio scales the pixels, not the figure's inch size.""" + base = backend.plot(meta={"fig-width": 8, "fig-height": 4, "output_pixel_ratio": 1}) + scaled = backend.plot(meta={"fig-width": 8, "fig-height": 4, "output_pixel_ratio": 2}) + + # The requested inch size is honored regardless of the pixel ratio. + assert base.figure_size == [8.0, 4.0] + assert scaled.figure_size == [8.0, 4.0] + # Physical pixels ~double ... + assert scaled.png_pixel_size[0] == pytest.approx(2 * base.png_pixel_size[0], abs=5) + assert scaled.png_pixel_size[1] == pytest.approx(2 * base.png_pixel_size[1], abs=5) + # ... while the reported logical size stays at the 1x pixel size. + assert scaled.metadata["width"] == pytest.approx(base.png_pixel_size[0], abs=5) + assert scaled.metadata["height"] == pytest.approx(base.png_pixel_size[1], abs=5) + + +def test_pixel_ratio_missing_is_noop(backend): + """ratio=1 produces the same output as no ratio at all.""" + result = backend.plot() + + assert "width" not in result.metadata + assert "height" not in result.metadata + + +def test_pixel_ratio_one_is_noop(backend): + """ratio=1 produces the same output as no ratio at all.""" + base = backend.plot() + scaled = backend.plot(meta={"output_pixel_ratio": 1}) + + assert scaled.figure_size == DEFAULT_FIGSIZE + assert scaled.png_pixel_size == base.png_pixel_size + assert "width" not in scaled.metadata + assert "height" not in scaled.metadata + + +@pytest.mark.parametrize("ratio", [0, -1]) +def test_pixel_ratio_zero_raises(backend, ratio): + with pytest.raises(ValueError): + backend.plot(meta={"output_pixel_ratio": ratio}) + + +# TODO: Test plt.show and other apis, display(fig)? diff --git a/extensions/positron-python/python_files/posit/positron/utils.py b/extensions/positron-python/python_files/posit/positron/utils.py index 2d119dd9e04..6d202f6b3bc 100644 --- a/extensions/positron-python/python_files/posit/positron/utils.py +++ b/extensions/positron-python/python_files/posit/positron/utils.py @@ -11,6 +11,7 @@ import logging import os import re +import struct import sys import threading import urllib.parse @@ -382,3 +383,10 @@ def get_command_uri(command: str, *args: str) -> str: def strip_ansi(text): """Strip ANSI escape sequences from text.""" return ansi_escape_re.sub("", text) + + +def png_pixel_size(data: bytes) -> tuple[int, int]: + """Read the (width, height) from a PNG's IHDR chunk.""" + ihdr = data.index(b"IHDR") + # The next 8 bytes after "IHDR" are the width and height, big-endian. + return struct.unpack(">ii", data[ihdr + 4 : ihdr + 12]) From 41c5add5a7e3623ef666f90fcd3383eaec7b527e Mon Sep 17 00:00:00 2001 From: seem Date: Wed, 29 Jul 2026 15:18:22 +0200 Subject: [PATCH 05/26] fix pyright errors and warnings --- .../posit/positron/execute_request.py | 9 +++-- .../positron/matplotlib_backend/console.py | 38 ++++++++----------- .../positron/matplotlib_backend/notebook.py | 23 +++++++---- .../posit/positron/positron_ipkernel.py | 13 +------ .../positron/tests/test_execute_request.py | 24 ++++++++++++ .../test_matplotlib_backend/test_notebook.py | 3 +- 6 files changed, 63 insertions(+), 47 deletions(-) diff --git a/extensions/positron-python/python_files/posit/positron/execute_request.py b/extensions/positron-python/python_files/posit/positron/execute_request.py index cfae1c69fbd..fd95be8963e 100644 --- a/extensions/positron-python/python_files/posit/positron/execute_request.py +++ b/extensions/positron-python/python_files/posit/positron/execute_request.py @@ -61,8 +61,9 @@ class PositronExecuteRequest(BaseModel): @property def figure_size(self) -> Optional[tuple[float, float]]: """The figure size in inches, if specified.""" - if self.fig_width is not None and self.fig_height is not None: - return (self.fig_width, self.fig_height) + w, h = self.fig_width, self.fig_height + if w is not None and h is not None and w > 0 and h > 0: + return (w, h) return None @classmethod @@ -71,7 +72,7 @@ def from_message(cls, message: dict) -> "PositronExecuteRequest": content = message.get("content", {}) positron = content.get("positron", {}) if isinstance(content, dict) else {} if not isinstance(positron, dict): - return cls() + return cls.parse_obj({}) try: return cls.parse_obj(positron) @@ -86,7 +87,7 @@ def from_message(cls, message: dict) -> "PositronExecuteRequest": return cls.parse_obj(cleaned) except ValidationError: logger.debug("Failed to parse positron execute request", exc_info=True) - return cls() + return cls.parse_obj({}) def current_execute_request() -> PositronExecuteRequest: diff --git a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/console.py b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/console.py index 3ac937f0525..7b728072360 100644 --- a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/console.py +++ b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/console.py @@ -126,7 +126,7 @@ class FigureManagerPositron(FigureManagerBase): The canvas for this figure. """ - canvas: FigureCanvasPositron + canvas: FigureCanvasPositron # type: ignore def __init__(self, canvas: FigureCanvasPositron, num: int | str): from ..plot_comm import PlotOrigin, PlotRange @@ -235,7 +235,7 @@ class FigureCanvasPositron(FigureCanvasAgg): The manager for this canvas. """ - manager: FigureManagerPositron + manager: FigureManagerPositron # type: ignore manager_class = FigureManagerPositron # type: ignore @@ -267,28 +267,21 @@ def draw(self, *, is_rendering=False) -> None: finally: # Do nothing if the canvas has not been rendered yet, to avoid an unnecessary update # since opening the comm will trigger a render from the frontend. - if not self._first_render_completed: - return # noqa: B012 - - # Do nothing if the canvas is currently being rendered, to avoid an infinite draw-render loop. - if is_rendering: - return # noqa: B012 - + if not self._first_render_completed or is_rendering: + pass # If the plot was closed after being opened, request an update to re-open the plot. - if self.manager.closed: + elif self.manager.closed: self.manager.update() - return # noqa: B012 - - # Check if the canvas contents have changed, and request an update if they have. - current_hash = self._hash_buffer_rgba() - logger.debug(f"Canvas: previous hash: {self._previous_hash[:6]}") - logger.debug(f"Canvas: current hash: {current_hash[:6]}") - if current_hash == self._previous_hash: - logger.debug("Canvas: hash is the same, no need to update") - return # noqa: B012 - - logger.debug("Canvas: hash changed, requesting an update") - self.manager.update() + else: + # Check if the canvas contents have changed, and request an update if they have. + current_hash = self._hash_buffer_rgba() + logger.debug(f"Canvas: previous hash: {self._previous_hash[:6]}") + logger.debug(f"Canvas: current hash: {current_hash[:6]}") + if current_hash == self._previous_hash: + logger.debug("Canvas: hash is the same, no need to update") + else: + logger.debug("Canvas: hash changed, requesting an update") + self.manager.update() def render(self, size: PlotSize | None, pixel_ratio: float, format_: str) -> bytes: # Set the device pixel ratio to the requested value. @@ -384,6 +377,7 @@ def gca(*args, **kwargs): # Drawing library differs from the active figure's library: start fresh so # the existing figure isn't overwritten. return plt.figure().gca() + assert _original_gca is not None return _original_gca(*args, **kwargs) plt.gca = _installed_gca = gca diff --git a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/notebook.py b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/notebook.py index 345dbdf2caa..b596b835564 100644 --- a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/notebook.py +++ b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/notebook.py @@ -22,10 +22,11 @@ import io import logging from binascii import b2a_base64 -from typing import TYPE_CHECKING, Literal +from typing import TYPE_CHECKING, Literal, cast import matplotlib import matplotlib.pyplot as plt +from IPython.core.formatters import DisplayFormatter from IPython.core.getipython import get_ipython from IPython.display import display from matplotlib._pylab_helpers import Gcf @@ -50,27 +51,32 @@ def new_figure_manager( *args, FigureClass=Figure, # noqa: N803 # 3-tuple with unit is only available in matplotlib >= 3.11. - figsize: tuple[int, int] | tuple[float, float, Literal["in", "cm", "px"]] | None = None, + figsize: tuple[float, float] | tuple[float, float, Literal["in", "cm", "px"]] | None = None, **kwargs, ) -> FigureManagerPositronNotebook: """Called by matplotlib when a new figure is created.""" # Get the current execute request. execute_request = current_execute_request() - # Prefer the user's explicit figsize (e.g. plt.figure(figsize=...) or plt.subplots(figsize=...)) - # over the execute request's figure size. + # Sizing precedence: + # 1. per-figure code (`plt.figure(figsize=...)` or `plt.subplots(figsize=...)`), + # which passes non-None figsize + # 2. cell execute request (`#| fig-width`` and `#| fig-height`) + # 3. matplotlib config (`plt.rcParams`) figsize = figsize or execute_request.figure_size figure = FigureClass(*args, figsize=figsize, **kwargs) # Also provide the execute request to the figure manager. - manager: FigureManagerPositronNotebook = FigureCanvasPositronNotebook.new_manager(figure, num) + manager: FigureManagerPositronNotebook = cast( + "FigureManagerPositronNotebook", FigureCanvasPositronNotebook.new_manager(figure, num) + ) manager.set_execute_request(execute_request) return manager class FigureManagerPositronNotebook(FigureManagerBase): - canvas: FigureCanvasPositronNotebook + canvas: FigureCanvasPositronNotebook # type: ignore def show(self): """Called by matplotlib when a figure is shown via `plt.show()` or `figure.show()`.""" @@ -93,13 +99,13 @@ def set_execute_request(self, execute_request: PositronExecuteRequest): class FigureCanvasPositronNotebook(FigureCanvasAgg): - manager_class = FigureManagerPositronNotebook + manager_class = FigureManagerPositronNotebook # type: ignore def set_execute_request(self, execute_request: PositronExecuteRequest) -> None: """Set the current execute request.""" # Set the device pixel ratio to the execute request's value, if provided. if (pixel_ratio := execute_request.output_pixel_ratio) is not None: - self._set_device_pixel_ratio(pixel_ratio) + self._set_device_pixel_ratio(pixel_ratio) # type: ignore def _display_figure(fig: Figure, *, format_="png") -> tuple[str, dict] | None: @@ -149,6 +155,7 @@ def _display_figure(fig: Figure, *, format_="png") -> tuple[str, dict] | None: def _get_png_formatter(shell: InteractiveShell) -> PNGFormatter: """Get the shell's PNG formatter.""" + assert isinstance(shell.display_formatter, DisplayFormatter) return shell.display_formatter.formatters["image/png"] diff --git a/extensions/positron-python/python_files/posit/positron/positron_ipkernel.py b/extensions/positron-python/python_files/posit/positron/positron_ipkernel.py index 824d31daf12..fe7ea06e63d 100644 --- a/extensions/positron-python/python_files/posit/positron/positron_ipkernel.py +++ b/extensions/positron-python/python_files/posit/positron/positron_ipkernel.py @@ -125,7 +125,7 @@ def _is_module_on_disk(name: str) -> bool: @magics_class class PositronMagics(Magics): - shell: PositronShell + shell: PositronShell # type: ignore # This will override the default `clear` defined in `ipykernel.zmqshell.KernelMagics`. @line_magic @@ -768,17 +768,6 @@ def init_control(self, context): self.control_thread.join = with_logging(self.control_thread.join) return result - def init_extensions(self): - # Include Positron built-in extensions in default_extensions, which: - # 1. Always loads, regardless of user config, and - # 2. Loads before user-configured extensions. - self.default_extensions = [ - # Parent default_extensions. - *self.default_extensions, - # Positron built-in extensions. - ] - super().init_extensions() - def init_gui_pylab(self): # Enable the Positron matplotlib backend if we're not in a notebook. # If we're in a notebook, use IPython's default backend via the super() call below. diff --git a/extensions/positron-python/python_files/posit/positron/tests/test_execute_request.py b/extensions/positron-python/python_files/posit/positron/tests/test_execute_request.py index 835fdcaad17..f52ae2c80c3 100644 --- a/extensions/positron-python/python_files/posit/positron/tests/test_execute_request.py +++ b/extensions/positron-python/python_files/posit/positron/tests/test_execute_request.py @@ -3,6 +3,8 @@ # Licensed under the Elastic License 2.0. See LICENSE.txt for license information. # +import pytest + from positron.execute_request import PositronExecuteRequest @@ -125,3 +127,25 @@ def test_unknown_keys_are_ignored() -> None: meta = PositronExecuteRequest.from_message(message) assert meta.fig_width == 6.4 + + +@pytest.mark.parametrize( + ("fig_width", "fig_height", "expected"), + [ + (6.4, 4.8, (6.4, 4.8)), + (None, 4.8, None), + (6.4, None, None), + (None, None, None), + (-1.0, 4.8, None), + (6.4, -1.0, None), + ], +) +def test_figure_size( + fig_width: float | None, fig_height: float | None, expected: tuple[float, float] | None +) -> None: + """The `figure_size` property returns a tuple of (width, height) if both are positive.""" + message = {"content": {"positron": {"fig-width": fig_width, "fig-height": fig_height}}} + + meta = PositronExecuteRequest.from_message(message) + + assert meta.figure_size == expected diff --git a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_notebook.py b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_notebook.py index 5deb14c36e3..1b0f8f9e4c3 100644 --- a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_notebook.py +++ b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_notebook.py @@ -48,6 +48,7 @@ def backend(shell): def _parse_png_output(output: RichOutput) -> tuple[bytes, dict]: result = output._repr_png_() + assert result is not None, "Expected a PNG output, but got None." data, metadata = result if len(result) == 2 else (result, {}) png = base64.b64decode(data) return png, metadata @@ -93,7 +94,7 @@ def plot( # Run the code and capture the output. with capture_output() as captured: - run_with_metadata(self.shell, code, meta) + run_with_metadata(code, meta) # Return the created plot. assert len(captured.outputs) == 1 From d886d5dd68f651c4e8241ecfaf1d746a2d72cfa4 Mon Sep 17 00:00:00 2001 From: seem Date: Wed, 29 Jul 2026 18:29:54 +0200 Subject: [PATCH 06/26] wip: support `%matplotlib` to switch backends --- .../positron/matplotlib_backend/__init__.py | 139 ++++++ .../positron/matplotlib_backend/console.py | 29 +- .../positron/matplotlib_backend/notebook.py | 37 +- .../posit/positron/positron_ipkernel.py | 109 ++++- .../test_matplotlib_backend/test_switching.py | 404 ++++++++++++++++++ .../METADATA | 17 + .../entry_points.txt | 3 + 7 files changed, 715 insertions(+), 23 deletions(-) create mode 100644 extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_switching.py create mode 100644 extensions/positron-python/python_files/posit/positron_matplotlib-0.1.0.dist-info/METADATA create mode 100644 extensions/positron-python/python_files/posit/positron_matplotlib-0.1.0.dist-info/entry_points.txt diff --git a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/__init__.py b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/__init__.py index 1d40d5998be..e17df8135fe 100644 --- a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/__init__.py +++ b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/__init__.py @@ -2,3 +2,142 @@ # Copyright (C) 2026 Posit Software, PBC. All rights reserved. # Licensed under the Elastic License 2.0. See LICENSE.txt for license information. # +""" +Positron's matplotlib backends. + +Positron ships one backend per session mode, each registered under its own short name +(`positron-console`, `positron-notebook`) via the `matplotlib.backend` entry point in +`python_files/posit/positron_matplotlib-.dist-info`. +This module holds the names they share and the switch-time lifecycle called by +`PositronShell.enable_matplotlib`. + +Keep this module free of matplotlib imports. It's imported by the kernel, which must +work in environments without matplotlib installed; the backend modules are imported +lazily, and only once they're actually needed. +""" + +from __future__ import annotations + +import importlib +import sys +from enum import Enum +from typing import Protocol, cast + +from ..session_mode import SessionMode + +_MODULE_PREFIX = "module://" + + +class Backend(Enum): + """ + One of Positron's matplotlib backends, and the names that select it. + + `short_name` is the name registered via the `matplotlib.backend` entry point; + `full_name` is matplotlib's `module://` spelling of `module_name`. Both select the + same backend: matplotlib resolves the short name through its backend registry + (matplotlib >= 3.9), while the `module://` spelling works on any version. + """ + + CONSOLE = ("positron.matplotlib_backend.console", "positron-console") + NOTEBOOK = ("positron.matplotlib_backend.notebook", "positron-notebook") + + def __init__(self, module_name: str, short_name: str) -> None: + self.module_name = module_name + self.short_name = short_name + + @classmethod + def for_session_mode(cls, session_mode: SessionMode) -> Backend: + """The backend for a session started in `session_mode`.""" + # BACKGROUND sessions get the console backend: they have no notebook to render + # figures into, and the console backend routes them to the plots pane. + return cls.NOTEBOOK if session_mode == SessionMode.NOTEBOOK else cls.CONSOLE + + @classmethod + def from_name(cls, backend: str) -> Backend | None: + """The Positron backend that `backend` selects, or None if `backend` isn't ours.""" + # Short names are case-insensitive in matplotlib's registry; `module://` names + # aren't, since everything after the prefix is an importable module path. + normalized = backend if backend.startswith(_MODULE_PREFIX) else backend.lower() + for candidate in cls: + if normalized in (candidate.full_name, candidate.short_name): + return candidate + return None + + @property + def full_name(self) -> str: + """Matplotlib's `module://` spelling of `module_name`; works on any matplotlib version.""" + return _MODULE_PREFIX + self.module_name + + @property + def preferred_name(self) -> str: + """ + The name to select this backend with. + + Prefers the short name (`positron-console` or `positron-notebook`): it's what + `%matplotlib` prints, and users can round-trip it into + `%matplotlib positron-console` or `matplotlib.use("positron-console")`. Falls + back to `full_name` when the entry point isn't discoverable, which is always + the case before matplotlib 3.9 since it has no backend registry. + """ + registry = _get_backend_registry() + # `list_all` also loads the entry points, which `switch_backend` relies on having + # been loaded to resolve the short name to a module. + if registry and self.short_name in registry.list_all(): + return self.short_name + return self.full_name + + def import_module(self) -> BackendModule: + """Import this Positron matplotlib backend module.""" + return cast("BackendModule", importlib.import_module(self.module_name)) + + +class BackendModule(Protocol): + """The lifecycle that each Positron matplotlib backend module implements.""" + + def activate(self) -> None: ... + + def deactivate(self) -> None: ... + + +def selects_module(backend: str, module_name: str) -> bool: + """ + Whether `backend` selects the Positron backend module `module_name`. + + True for the module's own `module://` name and its short name. Checking the short + name matters at self-activation time: when matplotlib switches to a short name, + `matplotlib.get_backend()` at module-import time can still report the short name + rather than the `module://` spelling it eventually settles into. + """ + candidate = Backend.from_name(backend) + return candidate is not None and candidate.module_name == module_name + + +def _get_backend_registry(): + """The matplotlib backend registry, or None if matplotlib < 3.9.""" + try: + from matplotlib.backends.registry import backend_registry + + return backend_registry + except ImportError: + # No backend registry before matplotlib 3.9. + return None + + +def configure_positron_support(backend: str) -> None: + """ + Activate or deactivate Positron's matplotlib backend for a switch to `backend`. + + Mirrors `matplotlib_inline.backend_inline.configure_inline_support`, the de facto + IPython interface for switch-time lifecycle: called on every backend switch, and + self-dispatching on whether the new backend is its own. Activating installs the + backend's shell hooks; deactivating removes them, so switching to another backend + (`%matplotlib qt`) no longer leaves them behind. + """ + target = Backend.from_name(backend) + for candidate in Backend: + if candidate is target: + candidate.import_module().activate() + elif candidate.module_name in sys.modules: + # Only deactivate a module that's already imported. Importing one just to + # deactivate it would pull in matplotlib and self-activate for nothing. + candidate.import_module().deactivate() diff --git a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/console.py b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/console.py index 7b728072360..32e5efe4a3f 100644 --- a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/console.py +++ b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/console.py @@ -16,6 +16,7 @@ from __future__ import annotations +import contextlib import hashlib import inspect import io @@ -29,6 +30,7 @@ from matplotlib.backends.backend_agg import FigureCanvasAgg from ..execute_request import PositronExecuteRequest +from . import selects_module if TYPE_CHECKING: from matplotlib.figure import Figure @@ -37,8 +39,6 @@ logger = logging.getLogger(__name__) -BACKEND_NAME = "module://positron.matplotlib_backend.console" - # High-level libraries that build on matplotlib. A figure produced by one of these is # attributed to that library (used for the plot's display name and, for seaborn, @@ -405,8 +405,17 @@ def _uninstall_library_gca_redirect() -> None: FigureManager = FigureManagerPositron +# True while this backend's shell hooks are installed, so that repeated activation +# (e.g. `%matplotlib inline` twice) doesn't register duplicate hooks. +_active = False + + def activate() -> None: - """Activate the Positron matplotlib console backend.""" + """Activate the Positron matplotlib console backend. Safe to call repeatedly.""" + global _active + if _active: + return + shell = get_ipython() if shell is None: logger.warning("No IPython shell found; matplotlib console backend not activated") @@ -422,20 +431,28 @@ def activate() -> None: _install_library_gca_redirect() + _active = True + def deactivate() -> None: - """Deactivate the Positron matplotlib console backend.""" + """Deactivate the Positron matplotlib console backend. Safe to call repeatedly.""" + global _active + _active = False + shell = get_ipython() if shell is None: logger.warning("No IPython shell found; matplotlib console backend not deactivated") return - shell.events.unregister("post_execute", _detach_library_figures) + # Suppress ValueError so that deactivating a backend that was never activated is a + # no-op. + with contextlib.suppress(ValueError): + shell.events.unregister("post_execute", _detach_library_figures) _uninstall_library_gca_redirect() # If we are the selected backend, activate. # This is expected to run when the backend is selected. See the note at the top of the file. -if matplotlib.get_backend() == BACKEND_NAME: +if selects_module(matplotlib.get_backend(), __name__): activate() diff --git a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/notebook.py b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/notebook.py index b596b835564..393b0e96a10 100644 --- a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/notebook.py +++ b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/notebook.py @@ -19,6 +19,7 @@ from __future__ import annotations +import contextlib import io import logging from binascii import b2a_base64 @@ -36,6 +37,7 @@ from ..execute_request import PositronExecuteRequest, current_execute_request from ..utils import png_pixel_size +from . import selects_module if TYPE_CHECKING: from IPython.core.formatters import PNGFormatter @@ -43,8 +45,6 @@ logger = logging.getLogger(__name__) -BACKEND_NAME = "module://positron.matplotlib_backend.notebook" - def new_figure_manager( num: int | str, @@ -167,8 +167,17 @@ def _show_figures(): logger.exception("Error showing figures in post execute hook") +# True while this backend's shell hooks are installed, so that repeated activation +# (e.g. `%matplotlib inline` twice) doesn't register duplicate hooks. +_active = False + + def activate() -> None: - """Activate the Positron matplotlib notebook backend.""" + """Activate the Positron matplotlib notebook backend. Safe to call repeatedly.""" + global _active + if _active: + return + shell = get_ipython() if shell is None: logger.warning("No IPython shell found; matplotlib notebook backend not activated") @@ -184,29 +193,37 @@ def activate() -> None: # Register our formatter for matplotlib Figure objects. _get_png_formatter(shell).for_type(Figure, _display_figure) + _active = True + def deactivate() -> None: - """Deactivate the Positron matplotlib notebook backend.""" + """Deactivate the Positron matplotlib notebook backend. Safe to call repeatedly.""" + global _active + _active = False + shell = get_ipython() if shell is None: logger.warning("No IPython shell found; matplotlib notebook backend not deactivated") return - # Unregister the post execute hook. - shell.events.unregister("post_execute", _show_figures) + # Unregister the post execute hook. Suppress ValueError so that deactivating a + # backend that was never activated is a no-op. + with contextlib.suppress(ValueError): + shell.events.unregister("post_execute", _show_figures) - # Unregister our formatter for matplotlib Figure objects. + # Unregister our formatter for matplotlib Figure objects. Note `lookup_by_type` and + # not `lookup`: the latter takes an instance, so it would never find our formatter. png_formatter = _get_png_formatter(shell) try: - if png_formatter.lookup(Figure) is _display_figure: + if png_formatter.lookup_by_type(Figure) is _display_figure: # It's still our formatter, remove it. png_formatter.pop(Figure) except KeyError: - # `lookup` raises if there's no formatter for the type, nothing to do. + # `lookup_by_type` raises if there's no formatter for the type, nothing to do. pass # If we are the selected backend, activate. # This is expected to run when the backend is selected. See the note at the top of the file. -if matplotlib.get_backend() == BACKEND_NAME: +if selects_module(matplotlib.get_backend(), __name__): activate() diff --git a/extensions/positron-python/python_files/posit/positron/positron_ipkernel.py b/extensions/positron-python/python_files/posit/positron/positron_ipkernel.py index fe7ea06e63d..5e3a80d6f5a 100644 --- a/extensions/positron-python/python_files/posit/positron/positron_ipkernel.py +++ b/extensions/positron-python/python_files/posit/positron/positron_ipkernel.py @@ -40,6 +40,7 @@ from .formatters.display_formatter import PositronDisplayFormatter from .help import HelpService, _distribution_to_modules, help # noqa: A004 from .lsp import LSPService +from .matplotlib_backend import Backend from .patch.bokeh import handle_bokeh_output, patch_bokeh_no_access from .patch.haystack import patch_haystack_is_in_jupyter from .patch.holoviews import set_holoviews_extension @@ -250,6 +251,31 @@ def _positron_exec_metadata(self, line: str) -> None: # noqa: ARG002 # keep reference to original showwarning original_showwarning = warnings.showwarning +_MATPLOTLIB_INLINE_MODULE = "matplotlib_inline.backend_inline" +_MATPLOTLIB_INLINE_BACKENDS = ("inline", f"module://{_MATPLOTLIB_INLINE_MODULE}") + + +def _needs_inline_support(shell: InteractiveShell, backend: str) -> bool: + """ + Whether matplotlib-inline's `configure_inline_support` should be called for `backend`. + + Only when matplotlib-inline is the backend being switched to, or when it's currently + active and needs tearing down. Calling it in any other case would instantiate + `InlineBackend` into `shell.configurables`, arming a traitlets observer that re-runs + `select_figure_formats` -- popping Positron's figure formatter -- on any later + `%config InlineBackend.*` assignment. + """ + if backend in _MATPLOTLIB_INLINE_BACKENDS: + return True + + # matplotlib-inline is active if its post execute hook is registered, whether by an + # earlier switch through here or by its backend module self-activating on import. + module = sys.modules.get(_MATPLOTLIB_INLINE_MODULE) + flush_figures = getattr(module, "flush_figures", None) + return flush_figures is not None and flush_figures in shell.events.callbacks.get( + "post_execute", [] + ) + class PositronShell(ZMQInteractiveShell): kernel: PositronIPyKernel @@ -340,6 +366,75 @@ def init_display_formatter(self): self.display_formatter = PositronDisplayFormatter(parent=self) self.configurables.append(self.display_formatter) # type: ignore IPython type annotation is wrong + def enable_matplotlib(self, gui=None): + """ + Enable interactive matplotlib and inline figure support. + + Overrides IPython so that bare `%matplotlib` and `%matplotlib inline` both + activate the Positron matplotlib backend that suits the session mode, and so + that switching to and from another backend (`%matplotlib qt`) installs and + removes its shell hooks. + + Mirrors `InteractiveShell.enable_matplotlib` as of IPython 9.14. The body is + inlined rather than delegated to `super()` because upstream unconditionally calls + `matplotlib_inline`'s `configure_inline_support`, which instantiates + `InlineBackend` into `shell.configurables` -- arming a traitlets observer that + pops Positron's figure formatter on any later `%config InlineBackend.*` + assignment. Drift on IPython upgrades is caught by the backend switch tests. + """ + from IPython.core import pylabtools as pt + + from .matplotlib_backend import configure_positron_support + + # 1. Select the matplotlib backend and gui event loop. + requested = gui.lower() if isinstance(gui, str) else gui + if requested in (None, "auto", "inline"): + # Positron's backend is both the session's default and its inline backend. + # This is a reroute through the full switch path rather than a no-op when + # already active, so that `%matplotlib inline` after `%matplotlib qt` + # switches back. See https://github.com/posit-dev/positron/issues/15116. + # + # An explicit `%matplotlib positron-console`/`positron-notebook` intentionally + # falls through to the `else` branch instead: `pt.find_gui_and_backend` below + # resolves those names through matplotlib's backend registry on matplotlib + # >= 3.9, same as any other named backend. + gui, backend = None, Backend.for_session_mode(self.session_mode).preferred_name + else: + gui, backend = pt.find_gui_and_backend(gui, self.pylab_gui_select) + if gui is not None: + # If we have our first gui selection, store it. + if self.pylab_gui_select is None: + self.pylab_gui_select = gui + # Otherwise if they are different, stay with the first one. + elif gui != self.pylab_gui_select: + print( + f"Warning: Cannot change to a different GUI toolkit: {gui}." + f" Using {self.pylab_gui_select} instead." + ) + gui, backend = pt.find_gui_and_backend(self.pylab_gui_select) + + # 2. Set up matplotlib for interactive use with that backend. + pt.activate_matplotlib(backend) + + # 3. Configure inline figure display: matplotlib-inline only when it's involved, + # and Positron's backend on every switch. Positron's goes last: tearing down + # matplotlib-inline re-runs `select_figure_formats`, which pops the figure + # formatter that activating Positron's backend then registers. + if _needs_inline_support(self, backend): + from matplotlib_inline.backend_inline import configure_inline_support + + configure_inline_support(self, backend) + configure_positron_support(backend) + + # 4. Enable the selected gui event loop, and fix %run to take plot updates into + # account. + self.enable_gui(gui) + self.magics_manager.registry["ExecutionMagics"].default_runner = pt.mpl_runner( + self.safe_execfile + ) + + return gui, backend + def _inspect(self, meth, oname, namespaces=None, **kw): # For `?name`, if the name isn't a live object but is an installed # module or a known PyPI distribution name (e.g. `scikit-learn`, @@ -769,15 +864,15 @@ def init_control(self, context): return result def init_gui_pylab(self): - # Enable the Positron matplotlib backend if we're not in a notebook. - # If we're in a notebook, use IPython's default backend via the super() call below. + # Enable the Positron matplotlib backend that matches the session mode. # Matplotlib uses the MPLBACKEND environment variable to determine the backend to use. - # It imports the backend module when it's first needed. + # It imports the backend module when it's first needed, which is when the backend + # activates itself. + # Use the `module://` spelling rather than a short name: it works on every + # matplotlib version, whereas the short names need the backend registry + # (matplotlib >= 3.9). if not os.environ.get("MPLBACKEND"): - if self.session_mode == SessionMode.NOTEBOOK: - os.environ["MPLBACKEND"] = "module://positron.matplotlib_backend.notebook" - else: - os.environ["MPLBACKEND"] = "module://positron.matplotlib_backend.console" + os.environ["MPLBACKEND"] = Backend.for_session_mode(self.session_mode).full_name return super().init_gui_pylab() diff --git a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_switching.py b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_switching.py new file mode 100644 index 00000000000..0e77b14bc6d --- /dev/null +++ b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_switching.py @@ -0,0 +1,404 @@ +# +# Copyright (C) 2026 Posit Software, PBC. All rights reserved. +# Licensed under the Elastic License 2.0. See LICENSE.txt for license information. +# +""" +Tests for switching matplotlib backends with `%matplotlib` in a Positron session. + +Exercises `PositronShell.enable_matplotlib`: bare `%matplotlib` and `%matplotlib inline` +keep the session's own Positron backend active, an explicit `positron-console` or +`positron-notebook` activates that flavor outright (even across session modes), another +backend tears Positron's hooks down, and switching back restores them. `agg` is the +"other" backend throughout so the tests run headless. Also covers `selects_module` and +`Backend`'s name-resolution helpers behind all of this. +""" + +from __future__ import annotations + +import contextlib +import sys +from typing import TYPE_CHECKING, Callable, Iterator, NamedTuple + +import matplotlib +import matplotlib.pyplot as plt +import pytest +from IPython.utils.capture import capture_output +from matplotlib.figure import Figure + +from positron import matplotlib_backend +from positron.matplotlib_backend import ( + Backend, + console, + notebook, + selects_module, +) +from positron.session_mode import SessionMode + +from ..utils import run_with_metadata + +if TYPE_CHECKING: + from types import ModuleType + + from positron.positron_ipkernel import PositronShell + +INLINE_MODULE_NAME = "matplotlib_inline.backend_inline" +INLINE_BACKEND_NAME = f"module://{INLINE_MODULE_NAME}" + +# The backend to switch away to. Headless, so these tests don't need a display. +OTHER_BACKEND_NAME = "agg" + + +class Flavor(NamedTuple): + """One of Positron's matplotlib backends, and how to detect that it's installed.""" + + session_mode: SessionMode + module: ModuleType + backend: Backend + # Whether the flavor's non-hook integration is installed: the console backend's + # `plt.gca` redirect, or the notebook backend's figure formatter. + integration_installed: Callable[[PositronShell], bool] + + +def _console_integration_installed(shell: PositronShell) -> bool: # noqa: ARG001 + return plt.gca is console._installed_gca # noqa: SLF001 + + +def _notebook_integration_installed(shell: PositronShell) -> bool: + formatter = shell.display_formatter.formatters["image/png"] + with contextlib.suppress(KeyError): + # `lookup_by_type` raises KeyError when no formatter is registered for the type. + return formatter.lookup_by_type(Figure) is notebook._display_figure # noqa: SLF001 + return False + + +CONSOLE = Flavor( + SessionMode.CONSOLE, + console, + Backend.CONSOLE, + _console_integration_installed, +) +NOTEBOOK = Flavor( + SessionMode.NOTEBOOK, + notebook, + Backend.NOTEBOOK, + _notebook_integration_installed, +) + + +def _other_flavor(flavor: Flavor) -> Flavor: + """The flavor that isn't `flavor`.""" + return NOTEBOOK if flavor is CONSOLE else CONSOLE + + +@pytest.fixture(params=[pytest.param(CONSOLE, id="console"), pytest.param(NOTEBOOK, id="notebook")]) +def flavor(request: pytest.FixtureRequest) -> Flavor: + """Each of Positron's matplotlib backends, one per session mode.""" + return request.param + + +@pytest.fixture +def positron_backend( + flavor: Flavor, shell: PositronShell, monkeypatch: pytest.MonkeyPatch +) -> Iterator[Flavor]: + """A session with the parametrized flavor's Positron backend active.""" + yield from _session_with_backend(flavor, shell, monkeypatch) + + +@pytest.fixture +def notebook_backend(shell: PositronShell, monkeypatch: pytest.MonkeyPatch) -> Iterator[Flavor]: + """A notebook session with Positron's notebook backend active.""" + yield from _session_with_backend(NOTEBOOK, shell, monkeypatch) + + +def _session_with_backend( + flavor: Flavor, shell: PositronShell, monkeypatch: pytest.MonkeyPatch +) -> Iterator[Flavor]: + """ + Fixture body: a session with `flavor`'s backend active, as it is at kernel startup. + + Restores matplotlib's backend, both flavors' hooks and any matplotlib-inline state + afterwards, since matplotlib and the shell are process-wide singletons. + """ + monkeypatch.setattr(shell, "session_mode", flavor.session_mode) + + # Entering a gui event loop needs a running kernel application, which tests don't have. + monkeypatch.setattr(shell, "enable_gui", lambda gui=None: None) # noqa: ARG005 + + previous_backend = matplotlib.get_backend() + matplotlib.use(flavor.backend.full_name) + flavor.module.activate() + + yield flavor + + console.deactivate() + notebook.deactivate() + _reset_matplotlib_inline(shell) + shell.kernel.plots_service.shutdown() + plt.close("all") + matplotlib.use(previous_backend) + + +def _reset_matplotlib_inline(shell: PositronShell) -> None: + """Undo any matplotlib-inline state so it doesn't leak into the next test.""" + module = sys.modules.get(INLINE_MODULE_NAME) + if module is None: + return + + from matplotlib_inline.config import InlineBackend + + with contextlib.suppress(ValueError): + shell.events.unregister("post_execute", module.flush_figures) + + saved_rc_params = shell.__dict__.pop("_saved_rcParams", None) + if saved_rc_params is not None: + matplotlib.rcParams.update(saved_rc_params) + + # `select_figure_formats` may have registered matplotlib-inline's figure formatters. + for formatter in shell.display_formatter.formatters.values(): + formatter.pop(Figure, None) + + shell.configurables[:] = [c for c in shell.configurables if not isinstance(c, InlineBackend)] + InlineBackend.clear_instance() + + # matplotlib-inline remembers the backend it last configured; forget it so the next + # test starts from the same state. + if hasattr(module.configure_inline_support, "current_backend"): + del module.configure_inline_support.current_backend + + +def _hook_count(shell: PositronShell, module_name: str) -> int: + """How many of `module_name`'s post execute hooks are registered on the shell.""" + return sum( + 1 + for callback in shell.events.callbacks["post_execute"] + if callback.__module__ == module_name + ) + + +def _state(shell: PositronShell, flavor: Flavor) -> dict: + """A snapshot of the backend state that a `%matplotlib` switch should manage.""" + return { + "backend": matplotlib.get_backend(), + "positron_hooks": _hook_count(shell, flavor.module.__name__), + "positron_integration": flavor.integration_installed(shell), + "inline_hooks": _hook_count(shell, INLINE_MODULE_NAME), + } + + +def _positron_active(flavor: Flavor) -> dict: + """The backend state after activating `flavor`'s Positron backend, inline uninvolved.""" + return { + "backend": flavor.backend.short_name, + "positron_hooks": 1, + "positron_integration": True, + "inline_hooks": 0, + } + + +def test_inline_keeps_positron_backend(shell: PositronShell, positron_backend: Flavor): + """`%matplotlib inline`, the boilerplate first cell of countless notebooks, is ours.""" + shell.run_cell("%matplotlib inline").raise_error() + + assert _state(shell, positron_backend) == _positron_active(positron_backend) + + +def test_inline_is_idempotent(shell: PositronShell, positron_backend: Flavor): + """Repeated `%matplotlib inline` registers the post execute hook exactly once.""" + shell.run_cell("%matplotlib inline").raise_error() + shell.run_cell("%matplotlib inline").raise_error() + + assert _state(shell, positron_backend) == _positron_active(positron_backend) + + +def test_inline_still_creates_positron_figures(shell: PositronShell, positron_backend: Flavor): + """Figures created after `%matplotlib inline` still go through Positron's backend.""" + shell.run_cell("%matplotlib inline").raise_error() + # Record the canvas type in the cell that creates the figure: the notebook backend + # closes figures after each cell, which drops the canvas. + shell.run_cell( + "import matplotlib.pyplot as plt\nfig, ax = plt.subplots()\ncanvas_type = type(fig.canvas)" + ).raise_error() + + assert shell.user_ns["canvas_type"] is positron_backend.module.FigureCanvas + + +def test_inline_keeps_figure_sizing(shell: PositronShell, notebook_backend: Flavor): # noqa: ARG001 + """`#| fig-width` and `#| fig-height` still size figures after `%matplotlib inline`.""" + shell.run_cell("%matplotlib inline").raise_error() + run_with_metadata( + "import matplotlib.pyplot as plt\nfig, ax = plt.subplots()", + {"fig-width": 8, "fig-height": 4}, + ) + + assert shell.user_ns["fig"].get_size_inches().tolist() == [8.0, 4.0] + + +def test_bare_magic_selects_positron(shell: PositronShell, positron_backend: Flavor): + """Bare `%matplotlib` selects Positron's backend, and prints a name users can reuse.""" + with capture_output() as captured: + shell.run_cell("%matplotlib").raise_error() + + expected = f"Using matplotlib backend: {positron_backend.backend.short_name}" + + assert captured.stdout.strip() == expected + assert _state(shell, positron_backend) == _positron_active(positron_backend) + + +def test_explicit_short_name_selects_own_flavor(shell: PositronShell, positron_backend: Flavor): + """`%matplotlib ` selects that flavor.""" + shell.run_cell(f"%matplotlib {positron_backend.backend.short_name}").raise_error() + + assert _state(shell, positron_backend) == _positron_active(positron_backend) + + +def test_explicit_short_name_selects_other_flavor_across_modes( + shell: PositronShell, positron_backend: Flavor +): + """ + `%matplotlib ` activates that flavor outright. + + Cross-mode selection is intentional now that each flavor has its own short name: + unlike the session-relative `positron` name it replaces, an explicit short name + always wins over the session's own mode. + """ + other = _other_flavor(positron_backend) + + shell.run_cell(f"%matplotlib {other.backend.short_name}").raise_error() + + assert _state(shell, other) == _positron_active(other) + # The flavor that was active coming in has been torn down. + assert _state(shell, positron_backend) == { + "backend": other.backend.short_name, + "positron_hooks": 0, + "positron_integration": False, + "inline_hooks": 0, + } + + +def test_positron_short_names_are_listed_backends(shell: PositronShell): + """Both of Positron's backends are first-class names, so they show up in `%matplotlib -l`.""" + with capture_output() as captured: + shell.run_cell("%matplotlib -l").raise_error() + + assert Backend.CONSOLE.short_name in captured.stdout + assert Backend.NOTEBOOK.short_name in captured.stdout + + +def test_other_backend_deactivates(shell: PositronShell, positron_backend: Flavor): + """Switching to another backend removes Positron's hooks instead of leaving them.""" + shell.run_cell(f"%matplotlib {OTHER_BACKEND_NAME}").raise_error() + + assert _state(shell, positron_backend) == { + "backend": OTHER_BACKEND_NAME, + "positron_hooks": 0, + "positron_integration": False, + "inline_hooks": 0, + } + + +def test_switching_back_reactivates(shell: PositronShell, positron_backend: Flavor): + """`%matplotlib inline` after another backend switches back instead of staying stuck.""" + shell.run_cell(f"%matplotlib {OTHER_BACKEND_NAME}").raise_error() + shell.run_cell("%matplotlib inline").raise_error() + + assert _state(shell, positron_backend) == _positron_active(positron_backend) + + +def test_deactivate_before_activate(shell: PositronShell, positron_backend: Flavor): + """Deactivating a backend that was never activated is a no-op, not an error.""" + positron_backend.module.deactivate() + positron_backend.module.deactivate() + + assert _state(shell, positron_backend) == { + "backend": positron_backend.backend.full_name, + "positron_hooks": 0, + "positron_integration": False, + "inline_hooks": 0, + } + + +def test_inline_backend_config_stays_inert(shell: PositronShell, positron_backend: Flavor): + """ + `%config InlineBackend.*` stays a no-op across switches that never target inline. + + Positron's backend never instantiates `InlineBackend` into `shell.configurables`, so + the traitlets observer that would re-run `select_figure_formats` -- popping Positron's + figure formatter -- is never armed. + """ + shell.run_cell("%matplotlib inline").raise_error() + shell.run_cell(f"%matplotlib {OTHER_BACKEND_NAME}").raise_error() + shell.run_cell("%matplotlib").raise_error() + + assert not [c for c in shell.configurables if type(c).__name__ == "InlineBackend"] + + shell.run_cell("%config InlineBackend.figure_format = 'svg'").raise_error() + + assert _state(shell, positron_backend) == _positron_active(positron_backend) + + +def test_matplotlib_inline_escape_hatch(shell: PositronShell, positron_backend: Flavor): + """The real matplotlib-inline backend stays reachable by its `module://` name.""" + shell.run_cell(f"%matplotlib {INLINE_BACKEND_NAME}").raise_error() + inline_state = _state(shell, positron_backend) + + shell.run_cell("%matplotlib inline").raise_error() + + assert inline_state == { + "backend": INLINE_BACKEND_NAME, + "positron_hooks": 0, + "positron_integration": False, + "inline_hooks": 1, + } + assert _state(shell, positron_backend) == _positron_active(positron_backend) + + +def test_selects_module_recognizes_own_names(flavor: Flavor): + """`selects_module` accepts a flavor's own short name and its `module://` name.""" + assert selects_module(flavor.backend.short_name, flavor.module.__name__) + assert selects_module(flavor.backend.full_name, flavor.module.__name__) + + +def test_selects_module_rejects_other_names(flavor: Flavor): + """`selects_module` rejects the other flavor's names and a foreign backend.""" + other = _other_flavor(flavor) + + assert not selects_module(other.backend.short_name, flavor.module.__name__) + assert not selects_module(other.backend.full_name, flavor.module.__name__) + assert not selects_module(OTHER_BACKEND_NAME, flavor.module.__name__) + + +def test_selects_module_short_name_case_insensitive(flavor: Flavor): + """Short names match case-insensitively, like matplotlib's backend registry.""" + assert selects_module(flavor.backend.short_name.upper(), flavor.module.__name__) + + +def test_selects_module_full_name_case_sensitive(flavor: Flavor): + """The path after `module://` is an importable module path, so case matters.""" + assert not selects_module(f"module://{flavor.module.__name__.upper()}", flavor.module.__name__) + + +@pytest.mark.parametrize( + ("session_mode", "expected"), + [ + (SessionMode.CONSOLE, Backend.CONSOLE), + (SessionMode.NOTEBOOK, Backend.NOTEBOOK), + # BACKGROUND sessions have no notebook to render into, so they get the console + # backend, which routes figures to the plots pane. + (SessionMode.BACKGROUND, Backend.CONSOLE), + ], +) +def test_backend_for_session_mode(session_mode: SessionMode, expected: Backend): + """Each session mode maps to the Positron backend that suits it.""" + assert Backend.for_session_mode(session_mode) is expected + + +def test_preferred_name_prefers_short_name(flavor: Flavor): + """The flavor's short name is preferred when matplotlib's backend registry knows it.""" + assert flavor.backend.preferred_name == flavor.backend.short_name + + +def test_preferred_name_falls_back_to_module_name(flavor: Flavor, monkeypatch: pytest.MonkeyPatch): + """Falls back to the `module://` name before matplotlib 3.9, which has no registry.""" + monkeypatch.setattr(matplotlib_backend, "_get_backend_registry", lambda: None) + + assert flavor.backend.preferred_name == flavor.backend.full_name diff --git a/extensions/positron-python/python_files/posit/positron_matplotlib-0.1.0.dist-info/METADATA b/extensions/positron-python/python_files/posit/positron_matplotlib-0.1.0.dist-info/METADATA new file mode 100644 index 00000000000..41838ceed88 --- /dev/null +++ b/extensions/positron-python/python_files/posit/positron_matplotlib-0.1.0.dist-info/METADATA @@ -0,0 +1,17 @@ +Metadata-Version: 2.1 +Name: positron-matplotlib +Version: 0.1.0 +Summary: Registers Positron's matplotlib backends under the names "positron-console" and "positron-notebook". + +This directory is checked into Positron, not produced by a package installer. + +matplotlib discovers named backends through the "matplotlib.backend" entry point +group, which it reads with importlib.metadata -- whose unit of discovery is a +*.dist-info directory on sys.path. This directory sits next to the kernel's +`positron` package, which is on sys.path before matplotlib loads, so the entry +points in entry_points.txt make `%matplotlib positron-console`, +`%matplotlib positron-notebook`, `matplotlib.use("positron-console")` and +`%matplotlib -l` work without installing anything. + +Do not publish this as a standalone distribution: Positron's matplotlib backends +only function inside a Positron session. diff --git a/extensions/positron-python/python_files/posit/positron_matplotlib-0.1.0.dist-info/entry_points.txt b/extensions/positron-python/python_files/posit/positron_matplotlib-0.1.0.dist-info/entry_points.txt new file mode 100644 index 00000000000..1ec0ccdbaf6 --- /dev/null +++ b/extensions/positron-python/python_files/posit/positron_matplotlib-0.1.0.dist-info/entry_points.txt @@ -0,0 +1,3 @@ +[matplotlib.backend] +positron-console = positron.matplotlib_backend.console +positron-notebook = positron.matplotlib_backend.notebook From 6f838b1b5e929cff104bc35474b2690340a210e0 Mon Sep 17 00:00:00 2001 From: seem Date: Wed, 29 Jul 2026 20:39:47 +0200 Subject: [PATCH 07/26] wip: set_matplotlib_formats support --- .../positron/matplotlib_backend/__init__.py | 21 ++ .../positron/matplotlib_backend/console.py | 9 +- .../positron/matplotlib_backend/formats.py | 210 ++++++++++++++++++ .../positron/matplotlib_backend/notebook.py | 78 +++---- .../posit/positron/positron_ipkernel.py | 19 +- .../test_matplotlib_backend/test_console.py | 20 ++ .../test_matplotlib_backend/test_formats.py | 205 +++++++++++++++++ .../test_matplotlib_backend/test_switching.py | 92 +++++++- .../posit/positron/tests/test_utils.py | 29 ++- .../python_files/posit/positron/utils.py | 26 +++ 10 files changed, 662 insertions(+), 47 deletions(-) create mode 100644 extensions/positron-python/python_files/posit/positron/matplotlib_backend/formats.py create mode 100644 extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_formats.py diff --git a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/__init__.py b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/__init__.py index e17df8135fe..d1345ffd13f 100644 --- a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/__init__.py +++ b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/__init__.py @@ -112,6 +112,27 @@ def selects_module(backend: str, module_name: str) -> bool: return candidate is not None and candidate.module_name == module_name +def register_with_legacy_ipython() -> None: + """ + Make Positron's short names visible to IPython versions without registry support. + + IPython 8.24 started resolving `%matplotlib ` and building `%matplotlib -l` + from matplotlib's backend registry (matplotlib >= 3.9), which discovers Positron's + backends through their entry points. Earlier IPython reads the static + `pylabtools.backends` table for both, so add the short names there. Uses the + `module://` spelling as the value so the entries resolve on any matplotlib version. + """ + from IPython.core import pylabtools as pt + + # The registry-based lister arrived in the same release that stopped reading the + # static table, so its presence tells the two resolution schemes apart. Touching + # `pt.backends` on newer IPython would also trip its deprecation warning. + if hasattr(pt, "_list_matplotlib_backends_and_gui_loops"): + return + for backend in Backend: + pt.backends.setdefault(backend.short_name, backend.full_name) + + def _get_backend_registry(): """The matplotlib backend registry, or None if matplotlib < 3.9.""" try: diff --git a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/console.py b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/console.py index 32e5efe4a3f..8a66a5a1de8 100644 --- a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/console.py +++ b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/console.py @@ -30,7 +30,7 @@ from matplotlib.backends.backend_agg import FigureCanvasAgg from ..execute_request import PositronExecuteRequest -from . import selects_module +from . import formats, selects_module if TYPE_CHECKING: from matplotlib.figure import Figure @@ -431,6 +431,10 @@ def activate() -> None: _install_library_gca_redirect() + # Intercept `set_matplotlib_formats`: it's a no-op here, since the Plots pane + # negotiates its own format via `canvas.render(format_=...)`. + formats.install_set_matplotlib_formats_patch() + _active = True @@ -451,6 +455,9 @@ def deactivate() -> None: _uninstall_library_gca_redirect() + # Undo the `set_matplotlib_formats` patch, unless the notebook flavor is still active. + formats.uninstall_set_matplotlib_formats_patch() + # If we are the selected backend, activate. # This is expected to run when the backend is selected. See the note at the top of the file. diff --git a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/formats.py b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/formats.py new file mode 100644 index 00000000000..56021962f96 --- /dev/null +++ b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/formats.py @@ -0,0 +1,210 @@ +# +# Copyright (C) 2026 Posit Software, PBC. All rights reserved. +# Licensed under the Elastic License 2.0. See LICENSE.txt for license information. +# +""" +Positron's support for `matplotlib_inline.backend_inline.set_matplotlib_formats`. + +That function (and its sibling `select_figure_formats`) operate entirely inside +matplotlib-inline/IPython, with no Positron code on the stack, so the only way to +intercept them is to patch `set_matplotlib_formats` itself. Both of Positron's +matplotlib backend flavors (console, notebook) install the *same* patch and leave it +dispatching at call time on whichever flavor is currently the active matplotlib +backend -- see `install_set_matplotlib_formats_patch` for why a shared, call-time +dispatch is needed instead of one patch per flavor. + +NOTE: only ever imported from a flavor's `activate`/`deactivate`, by which point +matplotlib is guaranteed to be importable. +""" + +from __future__ import annotations + +import logging +import sys +from functools import partial +from typing import TYPE_CHECKING, Callable + +import matplotlib +from IPython.core.getipython import get_ipython +from matplotlib.figure import Figure + +from . import Backend + +if TYPE_CHECKING: + from IPython.core.interactiveshell import InteractiveShell + +logger = logging.getLogger(__name__) + +# Formats accepted by `set_matplotlib_formats`, matching IPython's +# `select_figure_formats`. `retina`/`png2x` alias to `png`: Positron already renders at +# the frontend's device pixel ratio, so pixel-doubling is subsumed by that instead of +# IPython's dpi-doubling. `jpg` aliases to `jpeg`, the name Positron's formatter/mime +# tables use. +_FORMAT_ALIASES = {"retina": "png", "png2x": "png", "jpg": "jpeg"} +_MIME_BY_FORMAT = { + "png": "image/png", + "jpeg": "image/jpeg", + "svg": "image/svg+xml", + "pdf": "application/pdf", +} +# The formats IPython's `select_figure_formats` accepts; anything else is a `ValueError`. +_SUPPORTED_FORMATS = frozenset({"png", "retina", "png2x", "jpg", "jpeg", "svg", "pdf"}) + +# The formats currently selected; applied on the notebook backend's `activate()` so the +# user's choice survives a round trip through a non-Positron backend (e.g. +# `%matplotlib qt` then back to `%matplotlib inline`). +_selected_formats: set[str] = {"png"} + +# The (mime type, formatter callable) pairs registered by the last `select_figure_formats` +# call, so `pop_registered_formatters` pops exactly ours and nothing else. +_registered: list[tuple[str, Callable]] = [] + + +def _formatters(shell: InteractiveShell): + """The shell's mime-to-formatter mapping, asserting the display formatter exists.""" + assert shell.display_formatter is not None + return shell.display_formatter.formatters + + +def select_figure_formats(shell: InteractiveShell, formats, **print_figure_kwargs) -> None: + """ + Positron's mirror of `IPython.core.pylabtools.select_figure_formats`. + + Registers Positron's `_display_figure` (as a `partial` binding `format_` and any + `print_figure_kwargs`) on the display formatter for each requested format, so + figures keep going through Positron's DPR-aware rendering no matter which format + the user asked for. + + Unlike upstream, unsupported formats are validated before any formatter is + touched, so a typo (e.g. `'bmp'`) leaves a previously working selection in place + instead of clobbering it. + """ + # Imported lazily: `notebook` imports this module at top level, so importing it + # back here at module scope would be circular. By call time both modules are + # fully loaded. + from .notebook import _display_figure + + if isinstance(formats, str): + formats = {formats} + formats = set(formats) + + unsupported = formats - _SUPPORTED_FORMATS + if unsupported: + supported_str = ",".join(repr(f) for f in sorted(_SUPPORTED_FORMATS)) + unsupported_str = ",".join(repr(f) for f in sorted(unsupported)) + raise ValueError(f"supported formats are: {supported_str} not {unsupported_str}") + + # Pop `Figure` from every display formatter, mirroring upstream -- this also + # cleans up leftovers if IPython's own formatters somehow got installed (e.g. + # before this patch was in place). + for formatter in _formatters(shell).values(): + formatter.pop(Figure, None) + _registered.clear() + + for requested in formats: + normalized = _FORMAT_ALIASES.get(requested, requested) + mime = _MIME_BY_FORMAT[normalized] + callable_ = partial(_display_figure, format_=normalized, **print_figure_kwargs) + _formatters(shell)[mime].for_type(Figure, callable_) + _registered.append((mime, callable_)) + + _selected_formats.clear() + _selected_formats.update(formats) + + +def apply_selected_formats(shell: InteractiveShell) -> None: + """Register the currently selected formats. Called by the notebook backend's `activate`.""" + select_figure_formats(shell, _selected_formats) + + +def pop_registered_formatters(shell: InteractiveShell) -> None: + """Unregister the formatters `select_figure_formats` registered. Called by `deactivate`.""" + for mime, callable_ in _registered: + formatter = _formatters(shell)[mime] + try: + if formatter.lookup_by_type(Figure) is callable_: + # It's still ours, remove it. + formatter.pop(Figure) + except KeyError: + # `lookup_by_type` raises if there's no formatter for the type, nothing to do. + pass + _registered.clear() + + +# The original and installed `set_matplotlib_formats`, so `uninstall_...` can undo +# `install_...`. +_original_set_matplotlib_formats: Callable | None = None +_installed_set_matplotlib_formats: Callable | None = None + + +def install_set_matplotlib_formats_patch() -> None: + """ + Patch `matplotlib_inline.backend_inline.set_matplotlib_formats`. Safe to call repeatedly. + + The patch is shared by both flavors and dispatches at call time on + `matplotlib.get_backend()`, rather than being installed/uninstalled per flavor. + `configure_positron_support` iterates flavors in a fixed order, so on e.g. a + notebook -> console switch the new flavor's activate runs before the old flavor's + deactivate; a per-flavor install/uninstall would have that deactivate unpatch the + patch the activate just installed. + """ + global _original_set_matplotlib_formats, _installed_set_matplotlib_formats + if _installed_set_matplotlib_formats is not None: + return + + import matplotlib_inline.backend_inline as backend_inline + + original = backend_inline.set_matplotlib_formats + _original_set_matplotlib_formats = original + + def set_matplotlib_formats(*formats, **kwargs) -> None: + shell = get_ipython() + target = Backend.from_name(matplotlib.get_backend()) + if shell is not None and target is Backend.NOTEBOOK: + select_figure_formats(shell, formats, **kwargs) + elif target is Backend.CONSOLE: + # The Plots pane negotiates its own format via `canvas.render(format_=...)`; + # running `select_figure_formats` here would emit stray inline outputs + # alongside it. Silent no-op, matching the deliberate + # `%config InlineBackend.*` no-op precedent (see test_switching.py). + logger.debug("set_matplotlib_formats is a no-op in Positron's console backend") + else: + # A non-Positron backend is active (e.g. the user called + # `matplotlib.use('agg')` directly); defer to the original so its + # behavior is unaffected. + original(*formats, **kwargs) + + backend_inline.set_matplotlib_formats = set_matplotlib_formats + _installed_set_matplotlib_formats = set_matplotlib_formats + + +def uninstall_set_matplotlib_formats_patch() -> None: + """Restore the original `set_matplotlib_formats`. Safe to call repeatedly.""" + global _original_set_matplotlib_formats, _installed_set_matplotlib_formats + if _installed_set_matplotlib_formats is None: + return + + if _any_flavor_active(): + # Another flavor is still active; leave the shared patch installed for it. + return + + import matplotlib_inline.backend_inline as backend_inline + + # Only restore if our patch is still installed; something else may have patched + # over it since, in which case we leave that patch alone. + if backend_inline.set_matplotlib_formats is _installed_set_matplotlib_formats: + backend_inline.set_matplotlib_formats = _original_set_matplotlib_formats + + _original_set_matplotlib_formats = None + _installed_set_matplotlib_formats = None + + +def _any_flavor_active() -> bool: + """Whether either of Positron's matplotlib backend flavors is currently active.""" + for candidate in Backend: + # Only check a module that's already imported: one that was never imported + # was never active, and importing it now would self-activate it for nothing. + module = sys.modules.get(candidate.module_name) + if module is not None and getattr(module, "_active", False): + return True + return False diff --git a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/notebook.py b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/notebook.py index 393b0e96a10..d50f147c08b 100644 --- a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/notebook.py +++ b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/notebook.py @@ -23,11 +23,10 @@ import io import logging from binascii import b2a_base64 -from typing import TYPE_CHECKING, Literal, cast +from typing import Literal, cast import matplotlib import matplotlib.pyplot as plt -from IPython.core.formatters import DisplayFormatter from IPython.core.getipython import get_ipython from IPython.display import display from matplotlib._pylab_helpers import Gcf @@ -36,15 +35,15 @@ from matplotlib.figure import Figure from ..execute_request import PositronExecuteRequest, current_execute_request -from ..utils import png_pixel_size -from . import selects_module - -if TYPE_CHECKING: - from IPython.core.formatters import PNGFormatter - from IPython.core.interactiveshell import InteractiveShell +from ..utils import jpeg_pixel_size, png_pixel_size +from . import formats, selects_module logger = logging.getLogger(__name__) +# The pixel-size reader for each raster format's metadata. svg scales natively and +# pdf isn't rendered inline, so neither needs a pixel-size-derived `width`/`height`. +_PIXEL_SIZE_BY_FORMAT = {"png": png_pixel_size, "jpeg": jpeg_pixel_size} + def new_figure_manager( num: int | str, @@ -108,7 +107,9 @@ def set_execute_request(self, execute_request: PositronExecuteRequest) -> None: self._set_device_pixel_ratio(pixel_ratio) # type: ignore -def _display_figure(fig: Figure, *, format_="png") -> tuple[str, dict] | None: +def _display_figure( + fig: Figure, *, format_="png", **print_figure_kwargs +) -> tuple[str, dict] | None: """Render a figure to its Jupyter display wire format.""" # NOTE: This implementation must match `IPython.core.pylabtools.print_figure` otherwise # users will get different results in Positron vs other notebook editors. @@ -119,15 +120,18 @@ def _display_figure(fig: Figure, *, format_="png") -> tuple[str, dict] | None: # Render the figure to bytes. canvas = fig.canvas + kw = { + "format": format_, + # Must pass `fig.dpi` otherwise `print_figure` renders at a pixel ratio of 1. + "dpi": fig.dpi, + # Tight bbox mirrors IPython. + "bbox_inches": "tight", + } + # Explicit kwargs (e.g. `set_matplotlib_formats(..., pil_kwargs=...)`) win, matching + # upstream's `print_figure`. + kw.update(print_figure_kwargs) with io.BytesIO() as figure_buffer: - canvas.print_figure( - figure_buffer, - format=format_, - # Must pass `fig.dpi` otherwise `print_figure` renders at a pixel ratio of 1. - dpi=fig.dpi, - # Tight bbox mirrors IPython. - bbox_inches="tight", - ) + canvas.print_figure(figure_buffer, **kw) data = figure_buffer.getvalue() # Decode bytes to string. @@ -139,9 +143,12 @@ def _display_figure(fig: Figure, *, format_="png") -> tuple[str, dict] | None: # Prepare figure metadata. metadata = {} - # If the canvas has a custom device pixel ratio, include the intended pixel size in the metadata. - if (ratio := canvas.device_pixel_ratio) != 1: - w, h = png_pixel_size(data) + # If the canvas has a custom device pixel ratio, include the intended pixel size in + # the metadata. Only raster formats (png, jpeg) support this: svg scales natively, + # and pdf isn't rendered inline. + pixel_size = _PIXEL_SIZE_BY_FORMAT.get(format_) + if pixel_size is not None and (ratio := canvas.device_pixel_ratio) != 1: + w, h = pixel_size(data) metadata["width"] = int(w) / ratio metadata["height"] = int(h) / ratio @@ -153,12 +160,6 @@ def _display_figure(fig: Figure, *, format_="png") -> tuple[str, dict] | None: FigureManager = FigureManagerPositronNotebook -def _get_png_formatter(shell: InteractiveShell) -> PNGFormatter: - """Get the shell's PNG formatter.""" - assert isinstance(shell.display_formatter, DisplayFormatter) - return shell.display_formatter.formatters["image/png"] - - def _show_figures(): """Post execute hook to show all figures and log errors.""" try: @@ -190,8 +191,14 @@ def activate() -> None: # Register a hook to show all figures after cell execution. shell.events.register("post_execute", _show_figures) - # Register our formatter for matplotlib Figure objects. - _get_png_formatter(shell).for_type(Figure, _display_figure) + # Register our formatter(s) for matplotlib Figure objects, for the currently + # selected format(s) (`png` unless the user previously called + # `set_matplotlib_formats`). + formats.apply_selected_formats(shell) + + # Intercept `set_matplotlib_formats` so a user's call re-registers our formatters + # instead of IPython's. + formats.install_set_matplotlib_formats_patch() _active = True @@ -211,16 +218,11 @@ def deactivate() -> None: with contextlib.suppress(ValueError): shell.events.unregister("post_execute", _show_figures) - # Unregister our formatter for matplotlib Figure objects. Note `lookup_by_type` and - # not `lookup`: the latter takes an instance, so it would never find our formatter. - png_formatter = _get_png_formatter(shell) - try: - if png_formatter.lookup_by_type(Figure) is _display_figure: - # It's still our formatter, remove it. - png_formatter.pop(Figure) - except KeyError: - # `lookup_by_type` raises if there's no formatter for the type, nothing to do. - pass + # Unregister our formatter(s) for matplotlib Figure objects. + formats.pop_registered_formatters(shell) + + # Undo the `set_matplotlib_formats` patch, unless the console flavor is still active. + formats.uninstall_set_matplotlib_formats_patch() # If we are the selected backend, activate. diff --git a/extensions/positron-python/python_files/posit/positron/positron_ipkernel.py b/extensions/positron-python/python_files/posit/positron/positron_ipkernel.py index 5e3a80d6f5a..3e68610a352 100644 --- a/extensions/positron-python/python_files/posit/positron/positron_ipkernel.py +++ b/extensions/positron-python/python_files/posit/positron/positron_ipkernel.py @@ -40,7 +40,7 @@ from .formatters.display_formatter import PositronDisplayFormatter from .help import HelpService, _distribution_to_modules, help # noqa: A004 from .lsp import LSPService -from .matplotlib_backend import Backend +from .matplotlib_backend import Backend, register_with_legacy_ipython from .patch.bokeh import handle_bokeh_output, patch_bokeh_no_access from .patch.haystack import patch_haystack_is_in_jupyter from .patch.holoviews import set_holoviews_extension @@ -345,6 +345,10 @@ def init_magics(self): # Register Positron's custom magics. self.register_magics(PositronMagics) + # On IPython versions that don't read matplotlib's backend registry, add + # Positron's backends to the static table that `%matplotlib -l` lists. + register_with_legacy_ipython() + def init_user_ns(self): super().init_user_ns() @@ -388,17 +392,20 @@ def enable_matplotlib(self, gui=None): # 1. Select the matplotlib backend and gui event loop. requested = gui.lower() if isinstance(gui, str) else gui + requested_backend = Backend.from_name(requested) if isinstance(requested, str) else None if requested in (None, "auto", "inline"): # Positron's backend is both the session's default and its inline backend. # This is a reroute through the full switch path rather than a no-op when # already active, so that `%matplotlib inline` after `%matplotlib qt` # switches back. See https://github.com/posit-dev/positron/issues/15116. - # - # An explicit `%matplotlib positron-console`/`positron-notebook` intentionally - # falls through to the `else` branch instead: `pt.find_gui_and_backend` below - # resolves those names through matplotlib's backend registry on matplotlib - # >= 3.9, same as any other named backend. gui, backend = None, Backend.for_session_mode(self.session_mode).preferred_name + elif requested_backend is not None: + # An explicit `%matplotlib positron-console`/`positron-notebook` (either + # spelling) selects that flavor outright, even across session modes. + # Resolved here rather than through `pt.find_gui_and_backend`, which only + # knows entry-point backends like ours on IPython >= 8.24 with matplotlib + # >= 3.9; earlier IPython raises KeyError for them. + gui, backend = None, requested_backend.preferred_name else: gui, backend = pt.find_gui_and_backend(gui, self.pylab_gui_select) if gui is not None: diff --git a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_console.py b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_console.py index 352537145f0..2fdceb26a39 100644 --- a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_console.py +++ b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_console.py @@ -704,6 +704,26 @@ def test_mpl_shutdown(shell: PositronShell, plots_service: PlotsService) -> None assert all(comm._closed for comm in plot_comms) # noqa: SLF001 +def test_set_matplotlib_formats_is_noop_in_console(shell: PositronShell) -> None: + """ + `set_matplotlib_formats` is a silent no-op in console mode. + + The console backend routes figures to the Plots pane via `canvas.render`, which + negotiates its own format; running IPython's `select_figure_formats` here would + register stray inline `image/*` display formatters alongside it (see + `formats.install_set_matplotlib_formats_patch`). + """ + from matplotlib.figure import Figure + + shell.run_cell( + "from matplotlib_inline.backend_inline import set_matplotlib_formats\n" + "set_matplotlib_formats('png')" + ).raise_error() + + for formatter in shell.display_formatter.formatters.values(): + assert Figure not in formatter + + def test_plotnine_close_then_show(shell: PositronShell, plots_service: PlotsService) -> None: """Test that a plotnine plot renders and then closes comm correctly.""" shell.run_cell("""\ diff --git a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_formats.py b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_formats.py new file mode 100644 index 00000000000..949e4224482 --- /dev/null +++ b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_formats.py @@ -0,0 +1,205 @@ +# +# Copyright (C) 2026 Posit Software, PBC. All rights reserved. +# Licensed under the Elastic License 2.0. See LICENSE.txt for license information. +# +""" +Tests for `set_matplotlib_formats` support in Positron's notebook matplotlib backend. + +Exercises `matplotlib_backend/formats.py`: format selection and aliasing +(`retina`/`png2x` -> `png`), kwargs passthrough, the `set_matplotlib_formats` patch's +`from`-import binding, and that a format selection survives a round trip through a +non-Positron backend. +""" + +from __future__ import annotations + +import base64 +from typing import TYPE_CHECKING, Iterator + +import matplotlib +import pytest +from IPython.utils.capture import RichOutput, capture_output +from matplotlib.backends.backend_agg import FigureCanvasAgg + +from positron.matplotlib_backend import formats, notebook +from positron.session_mode import SessionMode +from positron.utils import jpeg_pixel_size, png_pixel_size + +from ..utils import run_with_metadata + +if TYPE_CHECKING: + from positron.positron_ipkernel import PositronShell + +# The `from`-import form IPython's own docs recommend, which binds whatever +# `matplotlib_inline.backend_inline.set_matplotlib_formats` is at import time. +_IMPORT_SET_FORMATS = "from matplotlib_inline.backend_inline import set_matplotlib_formats" + +_PLOT_CODE = "import matplotlib.pyplot as plt\nfig, ax = plt.subplots()\nax.plot([0, 1], [0, 1])" + + +@pytest.fixture(autouse=True) +def _setup(shell, monkeypatch): + """Configure the shell to match the notebook environment.""" + monkeypatch.setattr(shell, "session_mode", SessionMode.NOTEBOOK) + + +@pytest.fixture +def backend(shell: PositronShell) -> Iterator[PositronShell]: + """A fixture that configures matplotlib to use the Positron notebook backend.""" + prev = matplotlib.get_backend() + matplotlib.use("module://positron.matplotlib_backend.notebook") + notebook.activate() + + yield shell + + notebook.deactivate() + matplotlib.use(prev) + + # `_selected_formats` is module state that intentionally survives a backend round + # trip (see `test_preference_survives_backend_round_trip`), so it doesn't reset + # itself on `deactivate`. Reset it here to the default so a format selected in one + # test doesn't leak into the next (e.g. other test files' `backend` fixtures expect + # `png` on activation). + formats._selected_formats.clear() # noqa: SLF001 + formats._selected_formats.add("png") # noqa: SLF001 + + +def _set_formats(shell: PositronShell, call: str) -> None: + """Run `set_matplotlib_formats(...)` via the `from`-import path IPython documents.""" + shell.run_cell(f"{_IMPORT_SET_FORMATS}\n{call}").raise_error() + + +def _plot(*, meta: dict | None = None) -> RichOutput: + """Run a cell that creates a figure and return its single captured display output.""" + with capture_output() as captured: + run_with_metadata(_PLOT_CODE, meta) + assert len(captured.outputs) == 1 + return captured.outputs[0] + + +def test_svg(backend: PositronShell): + """`set_matplotlib_formats('svg')` displays only svg, decoded as utf-8 text.""" + _set_formats(backend, "set_matplotlib_formats('svg')") + + output = _plot() + + assert "image/svg+xml" in output.data + assert "image/png" not in output.data + assert isinstance(output.data["image/svg+xml"], str) + + +def test_retina_aliases_to_png(backend: PositronShell): + """ + `retina` still renders as `image/png`, at Positron's own device pixel ratio. + + Unlike IPython's `retina_figure` (a hardcoded 2x dpi-doubling on top of Positron's + own DPR handling), Positron already renders at the frontend's actual pixel ratio, + so `retina` is a no-op alias to `png`: the reported logical size matches a ratio=1 + render regardless of the requested ratio, and the actual pixel size scales with + the ratio rather than a hardcoded 2x. + + Uses ratio=3 (not 2): IPython's fixed 2x doubling would coincidentally produce + the same numbers at ratio=2, masking a regression back to the old behavior. + """ + _set_formats(backend, "set_matplotlib_formats('retina')") + + baseline = _plot(meta={"output_pixel_ratio": 1}) + baseline_w, baseline_h = png_pixel_size(base64.b64decode(baseline.data["image/png"])) + + ratio = 3 + output = _plot(meta={"output_pixel_ratio": ratio}) + + assert "image/png" in output.data + assert "image/jpeg" not in output.data + w, h = png_pixel_size(base64.b64decode(output.data["image/png"])) + metadata = output.metadata.get("image/png", {}) + # Actual pixels scale with the requested ratio ... + assert w == pytest.approx(ratio * baseline_w, abs=5) + assert h == pytest.approx(ratio * baseline_h, abs=5) + # ... but the reported logical size stays at the ratio=1 size. + assert metadata["width"] == pytest.approx(baseline_w, abs=5) + assert metadata["height"] == pytest.approx(baseline_h, abs=5) + + +def test_jpeg_with_pixel_ratio(backend: PositronShell): + """`jpeg` output is present and its metadata reflects the requested pixel ratio.""" + _set_formats(backend, "set_matplotlib_formats('jpeg')") + + output = _plot(meta={"output_pixel_ratio": 2}) + + assert "image/jpeg" in output.data + jpeg = base64.b64decode(output.data["image/jpeg"]) + w, h = jpeg_pixel_size(jpeg) + metadata = output.metadata.get("image/jpeg", {}) + assert metadata["width"] == pytest.approx(w / 2, abs=5) + assert metadata["height"] == pytest.approx(h / 2, abs=5) + + +def test_multiple_formats(backend: PositronShell): + """`set_matplotlib_formats('png', 'svg')` displays both mimes in one output.""" + _set_formats(backend, "set_matplotlib_formats('png', 'svg')") + + output = _plot() + + assert "image/png" in output.data + assert "image/svg+xml" in output.data + + +def test_unknown_format_raises(backend: PositronShell): + """An unrecognized format raises `ValueError`, matching IPython's contract.""" + with pytest.raises(ValueError): + _set_formats(backend, "set_matplotlib_formats('bmp')") + + +def test_kwargs_passthrough(backend: PositronShell, monkeypatch: pytest.MonkeyPatch): + """Explicit kwargs (e.g. `pil_kwargs`) reach `canvas.print_figure`.""" + # Asserting on the rendered jpeg bytes would be flaky (encoder-dependent), so spy on + # the call instead. + calls: list[dict] = [] + original_print_figure = FigureCanvasAgg.print_figure + + def spy(self, *args, **kwargs): + calls.append(kwargs) + return original_print_figure(self, *args, **kwargs) + + monkeypatch.setattr(FigureCanvasAgg, "print_figure", spy) + + _set_formats(backend, "set_matplotlib_formats('jpeg', pil_kwargs={'quality': 90})") + _plot() + + assert len(calls) == 1 + assert calls[0]["pil_kwargs"] == {"quality": 90} + + +def test_from_import_binding(backend: PositronShell): + """ + `from matplotlib_inline.backend_inline import set_matplotlib_formats` binds ours. + + The `from`-import binds whatever the module attribute is at import time, which is + already Positron's patch: it's installed at backend activation, before any user + code runs. + """ + _set_formats(backend, "set_matplotlib_formats('svg')") + + output = _plot() + + assert "image/svg+xml" in output.data + # If IPython's own formatters had been installed instead, `image/png` would also + # be present (upstream's `select_figure_formats` only touches formats you ask for, + # but never registers ours, so this also confirms the request routed to Positron's + # `_display_figure` and not upstream's `print_figure`). + assert "image/png" not in output.data + + +def test_preference_survives_backend_round_trip(backend: PositronShell): + """A format selection survives deactivating and reactivating the notebook backend.""" + _set_formats(backend, "set_matplotlib_formats('svg')") + + # Simulate switching to a non-Positron backend and back (e.g. `%matplotlib qt` then + # `%matplotlib inline`). + notebook.deactivate() + notebook.activate() + + output = _plot() + + assert "image/svg+xml" in output.data diff --git a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_switching.py b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_switching.py index 0e77b14bc6d..9b4f50c330c 100644 --- a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_switching.py +++ b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_switching.py @@ -17,6 +17,7 @@ import contextlib import sys +from functools import partial from typing import TYPE_CHECKING, Callable, Iterator, NamedTuple import matplotlib @@ -29,6 +30,7 @@ from positron.matplotlib_backend import ( Backend, console, + formats, notebook, selects_module, ) @@ -67,7 +69,11 @@ def _notebook_integration_installed(shell: PositronShell) -> bool: formatter = shell.display_formatter.formatters["image/png"] with contextlib.suppress(KeyError): # `lookup_by_type` raises KeyError when no formatter is registered for the type. - return formatter.lookup_by_type(Figure) is notebook._display_figure # noqa: SLF001 + # The registered callable is a `partial` binding `_display_figure` (see + # `formats.select_figure_formats`), not the bare function, since it also binds + # `format_` and any `print_figure_kwargs`. + current = formatter.lookup_by_type(Figure) + return isinstance(current, partial) and current.func is notebook._display_figure # noqa: SLF001 return False @@ -284,6 +290,47 @@ def test_positron_short_names_are_listed_backends(shell: PositronShell): assert Backend.NOTEBOOK.short_name in captured.stdout +def test_explicit_short_name_without_registry_resolution( + shell: PositronShell, positron_backend: Flavor, monkeypatch: pytest.MonkeyPatch +): + """ + Explicit short names select the backend even when IPython can't resolve them. + + IPython < 8.24 resolves `%matplotlib ` through a static table that raises + KeyError for entry-point backends like Positron's, instead of matplotlib's backend + registry. Simulate that here: the switch must not reach `find_gui_and_backend`. + """ + from IPython.core import pylabtools as pt + + def legacy_find_gui_and_backend(gui=None, gui_select=None): # noqa: ARG001 + raise KeyError(gui) + + monkeypatch.setattr(pt, "find_gui_and_backend", legacy_find_gui_and_backend) + + shell.run_cell(f"%matplotlib {positron_backend.backend.short_name}").raise_error() + + assert _state(shell, positron_backend) == _positron_active(positron_backend) + + +def test_register_with_legacy_ipython_adds_short_names(monkeypatch: pytest.MonkeyPatch): + """On IPython < 8.24, the short names are added to the static backend table for `-l`.""" + from IPython.core import pylabtools as pt + + # Simulate IPython < 8.24: no registry-based lister, `-l` reads `pt.backends`. + if hasattr(pt, "_list_matplotlib_backends_and_gui_loops"): + monkeypatch.delattr(pt, "_list_matplotlib_backends_and_gui_loops") + legacy_table = {"inline": INLINE_BACKEND_NAME} + monkeypatch.setattr(pt, "backends", legacy_table) + + matplotlib_backend.register_with_legacy_ipython() + + assert legacy_table == { + "inline": INLINE_BACKEND_NAME, + Backend.CONSOLE.short_name: Backend.CONSOLE.full_name, + Backend.NOTEBOOK.short_name: Backend.NOTEBOOK.full_name, + } + + def test_other_backend_deactivates(shell: PositronShell, positron_backend: Flavor): """Switching to another backend removes Positron's hooks instead of leaving them.""" shell.run_cell(f"%matplotlib {OTHER_BACKEND_NAME}").raise_error() @@ -352,6 +399,49 @@ def test_matplotlib_inline_escape_hatch(shell: PositronShell, positron_backend: assert _state(shell, positron_backend) == _positron_active(positron_backend) +def test_set_matplotlib_formats_patch_installed_while_active(positron_backend: Flavor): # noqa: ARG001 + """The shared `set_matplotlib_formats` patch is installed while a Positron flavor is active.""" + import matplotlib_inline.backend_inline as backend_inline + + assert backend_inline.set_matplotlib_formats is formats._installed_set_matplotlib_formats # noqa: SLF001 + + +def test_set_matplotlib_formats_patch_restored_after_switch( + shell: PositronShell, + positron_backend: Flavor, # noqa: ARG001 +): + """Switching to a non-Positron backend restores the original `set_matplotlib_formats`.""" + import matplotlib_inline.backend_inline as backend_inline + + original = formats._original_set_matplotlib_formats # noqa: SLF001 + + shell.run_cell(f"%matplotlib {OTHER_BACKEND_NAME}").raise_error() + + assert backend_inline.set_matplotlib_formats is original + + +def test_notebook_to_console_switch_keeps_patch_installed( + shell: PositronShell, + notebook_backend: Flavor, # noqa: ARG001 +): + """ + A notebook -> console switch runs activate-before-deactivate; the shared patch survives. + + Pins the call-time-dispatch / shared-uninstall design in + `formats.install_set_matplotlib_formats_patch`: a per-flavor install/uninstall would + have the console flavor's activate immediately followed by the notebook flavor's + deactivate, unpatching the patch the activate just installed. + """ + import matplotlib_inline.backend_inline as backend_inline + + installed_before = backend_inline.set_matplotlib_formats + + shell.run_cell(f"%matplotlib {Backend.CONSOLE.short_name}").raise_error() + + assert backend_inline.set_matplotlib_formats is installed_before + assert backend_inline.set_matplotlib_formats is formats._installed_set_matplotlib_formats # noqa: SLF001 + + def test_selects_module_recognizes_own_names(flavor: Flavor): """`selects_module` accepts a flavor's own short name and its `module://` name.""" assert selects_module(flavor.backend.short_name, flavor.module.__name__) diff --git a/extensions/positron-python/python_files/posit/positron/tests/test_utils.py b/extensions/positron-python/python_files/posit/positron/tests/test_utils.py index 2126240fedc..664f1c15f7a 100644 --- a/extensions/positron-python/python_files/posit/positron/tests/test_utils.py +++ b/extensions/positron-python/python_files/posit/positron/tests/test_utils.py @@ -3,9 +3,14 @@ # Licensed under the Elastic License 2.0. See LICENSE.txt for license information. # +import io + +import matplotlib +import matplotlib.pyplot as plt import pytest +from PIL import Image -from positron.utils import get_qualname +from positron.utils import get_qualname, jpeg_pixel_size class BadGetAttrImpl: @@ -22,3 +27,25 @@ def test_get_qualname_handles_bad_class(value) -> None: # qualname should be a valid string and not raise any errors assert isinstance(qualname, str), f"Expected string, got {type(qualname)}" assert qualname == "positron.tests.test_utils.BadGetAttrImpl" + + +def test_jpeg_pixel_size() -> None: + """`jpeg_pixel_size` reads a JPEG's pixel dimensions from its first SOF marker.""" + prev_backend = matplotlib.get_backend() + matplotlib.use("agg") + try: + fig, ax = plt.subplots(figsize=(4, 3), dpi=100) + ax.plot([0, 1], [0, 1]) + buffer = io.BytesIO() + fig.savefig(buffer, format="jpeg") + plt.close(fig) + finally: + matplotlib.use(prev_backend) + data = buffer.getvalue() + + # Cross-check against PIL's own JPEG header parsing rather than hardcoding the + # expected size, since the encoded size can differ slightly from the figure's + # requested inch size (dpi rounding). + expected = Image.open(io.BytesIO(data)).size + + assert jpeg_pixel_size(data) == expected diff --git a/extensions/positron-python/python_files/posit/positron/utils.py b/extensions/positron-python/python_files/posit/positron/utils.py index 6d202f6b3bc..6153c64efe5 100644 --- a/extensions/positron-python/python_files/posit/positron/utils.py +++ b/extensions/positron-python/python_files/posit/positron/utils.py @@ -390,3 +390,29 @@ def png_pixel_size(data: bytes) -> tuple[int, int]: ihdr = data.index(b"IHDR") # The next 8 bytes after "IHDR" are the width and height, big-endian. return struct.unpack(">ii", data[ihdr + 4 : ihdr + 12]) + + +# SOFn (start of frame) marker bytes that carry a JPEG's height/width. Excludes 0xC4 +# (DHT), 0xC8 (JPG, reserved), and 0xCC (DAC), which share the 0xC0-0xCF range but +# aren't SOF markers. +_JPEG_SOF_MARKERS = frozenset(range(0xC0, 0xD0)) - {0xC4, 0xC8, 0xCC} +# Markers with no length-prefixed payload to skip over: SOI/EOI and the RST markers. +_JPEG_STANDALONE_MARKERS = frozenset({0xD8, 0xD9, *range(0xD0, 0xD8)}) + + +def jpeg_pixel_size(data: bytes) -> tuple[int, int]: + """Read the (width, height) from a JPEG's first SOFn (start of frame) segment.""" + i = 2 # Skip the SOI marker (0xFFD8). + while i < len(data) - 1: + marker = data[i + 1] + i += 2 + if marker in _JPEG_STANDALONE_MARKERS: + continue + length = struct.unpack(">H", data[i : i + 2])[0] + if marker in _JPEG_SOF_MARKERS: + # SOFn payload: 1 byte precision, then height, width, both big-endian. + _, height, width = struct.unpack(">BHH", data[i + 2 : i + 7]) + return width, height + # Not a SOF marker; skip its payload and keep scanning. + i += length + raise ValueError("No SOF marker found in JPEG data") From 881f32c5c9e6062e19595dba2b4f61dc1b395ac3 Mon Sep 17 00:00:00 2001 From: seem Date: Wed, 29 Jul 2026 21:13:35 +0200 Subject: [PATCH 08/26] refactor: matplotlib backend registry + simplifications --- .../positron/matplotlib_backend/__init__.py | 90 ++++++++---- .../positron/matplotlib_backend/console.py | 58 +++----- .../positron/matplotlib_backend/formats.py | 109 ++++++++------ .../positron/matplotlib_backend/notebook.py | 133 +++--------------- .../test_matplotlib_backend/test_console.py | 16 +-- .../test_matplotlib_backend/test_formats.py | 36 +++-- .../test_matplotlib_backend/test_notebook.py | 10 +- .../test_matplotlib_backend/test_switching.py | 105 ++++++++++---- .../posit/positron/tests/test_utils.py | 29 +--- .../python_files/posit/positron/utils.py | 34 ----- 10 files changed, 276 insertions(+), 344 deletions(-) diff --git a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/__init__.py b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/__init__.py index d1345ffd13f..5e436c352d4 100644 --- a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/__init__.py +++ b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/__init__.py @@ -19,12 +19,19 @@ from __future__ import annotations import importlib -import sys +import logging from enum import Enum -from typing import Protocol, cast +from typing import TYPE_CHECKING, Protocol, cast + +from IPython.core.getipython import get_ipython from ..session_mode import SessionMode +if TYPE_CHECKING: + from IPython.core.interactiveshell import InteractiveShell + +logger = logging.getLogger(__name__) + _MODULE_PREFIX = "module://" @@ -54,7 +61,15 @@ def for_session_mode(cls, session_mode: SessionMode) -> Backend: @classmethod def from_name(cls, backend: str) -> Backend | None: - """The Positron backend that `backend` selects, or None if `backend` isn't ours.""" + """ + The Positron backend that `backend` selects, or None if `backend` isn't ours. + + Matches either spelling (short name or `module://` name). That matters at a + backend module's self-activation time (see the trailer at the bottom of + `console.py`/`notebook.py`): when matplotlib switches to a short name, + `matplotlib.get_backend()` at module-import time can still report the short + name rather than the `module://` spelling it eventually settles into. + """ # Short names are case-insensitive in matplotlib's registry; `module://` names # aren't, since everything after the prefix is an importable module path. normalized = backend if backend.startswith(_MODULE_PREFIX) else backend.lower() @@ -92,24 +107,18 @@ def import_module(self) -> BackendModule: class BackendModule(Protocol): - """The lifecycle that each Positron matplotlib backend module implements.""" - - def activate(self) -> None: ... - - def deactivate(self) -> None: ... - - -def selects_module(backend: str, module_name: str) -> bool: """ - Whether `backend` selects the Positron backend module `module_name`. + The hooks each Positron backend module implements. - True for the module's own `module://` name and its short name. Checking the short - name matters at self-activation time: when matplotlib switches to a short name, - `matplotlib.get_backend()` at module-import time can still report the short name - rather than the `module://` spelling it eventually settles into. + Called only by `configure_positron_support`, which owns activation state and + guarantees pairing and ordering: the previously active backend is uninstalled + before the next one is installed. `install` and `uninstall` are therefore not + idempotent and hold no activation state of their own. """ - candidate = Backend.from_name(backend) - return candidate is not None and candidate.module_name == module_name + + def install(self, shell: InteractiveShell) -> None: ... + + def uninstall(self, shell: InteractiveShell) -> None: ... def register_with_legacy_ipython() -> None: @@ -144,21 +153,42 @@ def _get_backend_registry(): return None -def configure_positron_support(backend: str) -> None: +# The Positron backend whose hooks are currently installed. Owned exclusively by +# `configure_positron_support`; backend modules hold no activation state of their own. +_active_backend: Backend | None = None + + +def configure_positron_support(backend: str | Backend) -> None: """ Activate or deactivate Positron's matplotlib backend for a switch to `backend`. Mirrors `matplotlib_inline.backend_inline.configure_inline_support`, the de facto IPython interface for switch-time lifecycle: called on every backend switch, and - self-dispatching on whether the new backend is its own. Activating installs the - backend's shell hooks; deactivating removes them, so switching to another backend - (`%matplotlib qt`) no longer leaves them behind. + self-dispatching on whether the new backend is its own. + + Sole owner of activation state: the previously active backend's hooks are + uninstalled before the new backend's are installed, and a switch to the already + active backend is a no-op. """ - target = Backend.from_name(backend) - for candidate in Backend: - if candidate is target: - candidate.import_module().activate() - elif candidate.module_name in sys.modules: - # Only deactivate a module that's already imported. Importing one just to - # deactivate it would pull in matplotlib and self-activate for nothing. - candidate.import_module().deactivate() + global _active_backend + + target = backend if isinstance(backend, Backend) else Backend.from_name(backend) + if target is _active_backend: + return + + shell = get_ipython() + if shell is None: + logger.warning("No IPython shell found; Positron matplotlib support not configured") + return + + if _active_backend is not None: + _active_backend.import_module().uninstall(shell) + _active_backend = None + + if target is not None: + # Enable interactive mode (i.e. redraw after every plotting command). + import matplotlib + + matplotlib.interactive(True) # noqa: FBT003 + target.import_module().install(shell) + _active_backend = target diff --git a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/console.py b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/console.py index 8a66a5a1de8..b5a45959c6c 100644 --- a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/console.py +++ b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/console.py @@ -25,14 +25,14 @@ import matplotlib import matplotlib.pyplot as plt -from IPython.core.getipython import get_ipython from matplotlib.backend_bases import FigureManagerBase from matplotlib.backends.backend_agg import FigureCanvasAgg from ..execute_request import PositronExecuteRequest -from . import formats, selects_module +from . import Backend, configure_positron_support, formats if TYPE_CHECKING: + from IPython.core.interactiveshell import InteractiveShell from matplotlib.figure import Figure from ..plot_comm import PlotSize @@ -336,7 +336,7 @@ def _hash_buffer_rgba(self) -> str: return hashlib.sha1(self.buffer_rgba()).hexdigest() -# The original and installed `plt.gca`, so `deactivate` can undo our changes. +# The original and installed `plt.gca`, so `uninstall` can undo our changes. _original_gca = None _installed_gca = None @@ -405,25 +405,8 @@ def _uninstall_library_gca_redirect() -> None: FigureManager = FigureManagerPositron -# True while this backend's shell hooks are installed, so that repeated activation -# (e.g. `%matplotlib inline` twice) doesn't register duplicate hooks. -_active = False - - -def activate() -> None: - """Activate the Positron matplotlib console backend. Safe to call repeatedly.""" - global _active - if _active: - return - - shell = get_ipython() - if shell is None: - logger.warning("No IPython shell found; matplotlib console backend not activated") - return - - # Enable interactive mode (i.e. redraw after every plotting command). - matplotlib.interactive(True) # noqa: FBT003 - +def install(shell: InteractiveShell) -> None: + """Install the console backend's shell hooks. See `BackendModule`.""" # Detach high-level plotting library figures (e.g. seaborn) so that re-running # plotting code starts a fresh figure instead of drawing onto the previous one. # See https://github.com/posit-dev/positron/issues/8898. @@ -435,31 +418,24 @@ def activate() -> None: # negotiates its own format via `canvas.render(format_=...)`. formats.install_set_matplotlib_formats_patch() - _active = True - - -def deactivate() -> None: - """Deactivate the Positron matplotlib console backend. Safe to call repeatedly.""" - global _active - _active = False - - shell = get_ipython() - if shell is None: - logger.warning("No IPython shell found; matplotlib console backend not deactivated") - return - # Suppress ValueError so that deactivating a backend that was never activated is a - # no-op. +def uninstall(shell: InteractiveShell) -> None: + """Remove the console backend's shell hooks. See `BackendModule`.""" + # Suppress ValueError in case user code unregistered the hook itself; a backend + # switch shouldn't crash over it. with contextlib.suppress(ValueError): shell.events.unregister("post_execute", _detach_library_figures) _uninstall_library_gca_redirect() - # Undo the `set_matplotlib_formats` patch, unless the notebook flavor is still active. formats.uninstall_set_matplotlib_formats_patch() -# If we are the selected backend, activate. -# This is expected to run when the backend is selected. See the note at the top of the file. -if selects_module(matplotlib.get_backend(), __name__): - activate() +# If we are the selected backend, activate through the registry, which also tears +# down the other flavor if it was active. This runs when matplotlib imports the +# module to switch to it. See the note at the top of the file. `Backend.from_name` +# matches both spellings, which matters here: matplotlib can still report our short +# name via `get_backend()` at this point, before it settles into the `module://` +# spelling. +if Backend.from_name(matplotlib.get_backend()) is Backend.CONSOLE: + configure_positron_support(Backend.CONSOLE) diff --git a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/formats.py b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/formats.py index 56021962f96..35b2f54d9ea 100644 --- a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/formats.py +++ b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/formats.py @@ -13,19 +13,25 @@ backend -- see `install_set_matplotlib_formats_patch` for why a shared, call-time dispatch is needed instead of one patch per flavor. -NOTE: only ever imported from a flavor's `activate`/`deactivate`, by which point +NOTE: only ever imported from a flavor's `install`/`uninstall`, by which point matplotlib is guaranteed to be importable. """ from __future__ import annotations +import contextlib import logging -import sys +from binascii import b2a_base64 from functools import partial -from typing import TYPE_CHECKING, Callable +from typing import TYPE_CHECKING, Callable, cast import matplotlib + +# Same private helpers IPython's own retina path uses for identical metadata; +# stable for 10+ years. +from IPython.core.display import _jpegxy, _pngxy from IPython.core.getipython import get_ipython +from IPython.core.pylabtools import print_figure from matplotlib.figure import Figure from . import Backend @@ -50,15 +56,15 @@ # The formats IPython's `select_figure_formats` accepts; anything else is a `ValueError`. _SUPPORTED_FORMATS = frozenset({"png", "retina", "png2x", "jpg", "jpeg", "svg", "pdf"}) -# The formats currently selected; applied on the notebook backend's `activate()` so the +# The pixel-size reader for each raster format's metadata. svg scales natively and +# pdf isn't rendered inline, so neither needs a pixel-size-derived `width`/`height`. +_PIXEL_SIZE_BY_FORMAT = {"png": _pngxy, "jpeg": _jpegxy} + +# The formats currently selected; applied on the notebook backend's `install()` so the # user's choice survives a round trip through a non-Positron backend (e.g. # `%matplotlib qt` then back to `%matplotlib inline`). _selected_formats: set[str] = {"png"} -# The (mime type, formatter callable) pairs registered by the last `select_figure_formats` -# call, so `pop_registered_formatters` pops exactly ours and nothing else. -_registered: list[tuple[str, Callable]] = [] - def _formatters(shell: InteractiveShell): """The shell's mime-to-formatter mapping, asserting the display formatter exists.""" @@ -66,6 +72,43 @@ def _formatters(shell: InteractiveShell): return shell.display_formatter.formatters +def _display_figure( + fig: Figure, *, format_="png", **print_figure_kwargs +) -> tuple[str, dict] | None: + """ + Render a figure to its Jupyter display wire format. + + Rendering is delegated to `IPython.core.pylabtools.print_figure` -- the function + IPython's own inline formatters wrap -- so output matches other Jupyter frontends + (empty-figure suppression, kwarg precedence, facecolor handling, dpi). On top of + that, Positron attaches the intended logical size as metadata when rendering at a + device pixel ratio other than 1. + """ + # `bbox_inches="tight"` is `print_figure`'s own default; deliberately not passed + # here so a user-supplied `bbox_inches` in `print_figure_kwargs` can't collide + # with a duplicate keyword. + data = print_figure(fig, fmt=format_, **print_figure_kwargs) + if data is None: + # Empty figure: display nothing, mirroring IPython. + return None + + # svg comes back as an already-decoded string and scales natively, so it carries + # no size metadata; everything else is raw bytes to base64-encode. + if format_ == "svg": + # `print_figure` returns `str` only for svg; narrowed by the `format_` check above. + return cast("str", data), {} + decoded = b2a_base64(cast("bytes", data), newline=False).decode("ascii") + + metadata = {} + pixel_size = _PIXEL_SIZE_BY_FORMAT.get(format_) + if pixel_size is not None and (ratio := fig.canvas.device_pixel_ratio) != 1: + w, h = pixel_size(cast("bytes", data)) + metadata["width"] = int(w) / ratio + metadata["height"] = int(h) / ratio + + return decoded, metadata + + def select_figure_formats(shell: InteractiveShell, formats, **print_figure_kwargs) -> None: """ Positron's mirror of `IPython.core.pylabtools.select_figure_formats`. @@ -79,11 +122,6 @@ def select_figure_formats(shell: InteractiveShell, formats, **print_figure_kwarg touched, so a typo (e.g. `'bmp'`) leaves a previously working selection in place instead of clobbering it. """ - # Imported lazily: `notebook` imports this module at top level, so importing it - # back here at module scope would be circular. By call time both modules are - # fully loaded. - from .notebook import _display_figure - if isinstance(formats, str): formats = {formats} formats = set(formats) @@ -99,36 +137,31 @@ def select_figure_formats(shell: InteractiveShell, formats, **print_figure_kwarg # before this patch was in place). for formatter in _formatters(shell).values(): formatter.pop(Figure, None) - _registered.clear() for requested in formats: normalized = _FORMAT_ALIASES.get(requested, requested) mime = _MIME_BY_FORMAT[normalized] callable_ = partial(_display_figure, format_=normalized, **print_figure_kwargs) _formatters(shell)[mime].for_type(Figure, callable_) - _registered.append((mime, callable_)) _selected_formats.clear() _selected_formats.update(formats) def apply_selected_formats(shell: InteractiveShell) -> None: - """Register the currently selected formats. Called by the notebook backend's `activate`.""" + """Register the currently selected formats. Called by the notebook backend's `install`.""" select_figure_formats(shell, _selected_formats) def pop_registered_formatters(shell: InteractiveShell) -> None: - """Unregister the formatters `select_figure_formats` registered. Called by `deactivate`.""" - for mime, callable_ in _registered: - formatter = _formatters(shell)[mime] - try: - if formatter.lookup_by_type(Figure) is callable_: - # It's still ours, remove it. + """Unregister the formatters `select_figure_formats` registered. Called by `uninstall`.""" + for formatter in _formatters(shell).values(): + # `lookup_by_type` raises KeyError if there's no formatter for the type. + with contextlib.suppress(KeyError): + current = formatter.lookup_by_type(Figure) + if isinstance(current, partial) and current.func is _display_figure: + # It's still ours (a `partial` binding `_display_figure`), remove it. formatter.pop(Figure) - except KeyError: - # `lookup_by_type` raises if there's no formatter for the type, nothing to do. - pass - _registered.clear() # The original and installed `set_matplotlib_formats`, so `uninstall_...` can undo @@ -142,11 +175,10 @@ def install_set_matplotlib_formats_patch() -> None: Patch `matplotlib_inline.backend_inline.set_matplotlib_formats`. Safe to call repeatedly. The patch is shared by both flavors and dispatches at call time on - `matplotlib.get_backend()`, rather than being installed/uninstalled per flavor. - `configure_positron_support` iterates flavors in a fixed order, so on e.g. a - notebook -> console switch the new flavor's activate runs before the old flavor's - deactivate; a per-flavor install/uninstall would have that deactivate unpatch the - patch the activate just installed. + `matplotlib.get_backend()`, so it stays correct however the active flavor changes + between install and call time. Dispatching on the live backend string rather than + the registry's state is deliberate: a user can switch backends with a bare + `matplotlib.use(...)` that never goes through the registry. """ global _original_set_matplotlib_formats, _installed_set_matplotlib_formats if _installed_set_matplotlib_formats is not None: @@ -184,10 +216,6 @@ def uninstall_set_matplotlib_formats_patch() -> None: if _installed_set_matplotlib_formats is None: return - if _any_flavor_active(): - # Another flavor is still active; leave the shared patch installed for it. - return - import matplotlib_inline.backend_inline as backend_inline # Only restore if our patch is still installed; something else may have patched @@ -197,14 +225,3 @@ def uninstall_set_matplotlib_formats_patch() -> None: _original_set_matplotlib_formats = None _installed_set_matplotlib_formats = None - - -def _any_flavor_active() -> bool: - """Whether either of Positron's matplotlib backend flavors is currently active.""" - for candidate in Backend: - # Only check a module that's already imported: one that was never imported - # was never active, and importing it now would self-activate it for nothing. - module = sys.modules.get(candidate.module_name) - if module is not None and getattr(module, "_active", False): - return True - return False diff --git a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/notebook.py b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/notebook.py index d50f147c08b..5a0aeb4c687 100644 --- a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/notebook.py +++ b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/notebook.py @@ -20,29 +20,24 @@ from __future__ import annotations import contextlib -import io import logging -from binascii import b2a_base64 -from typing import Literal, cast +from typing import TYPE_CHECKING, Literal, cast import matplotlib import matplotlib.pyplot as plt -from IPython.core.getipython import get_ipython from IPython.display import display from matplotlib._pylab_helpers import Gcf from matplotlib.backend_bases import FigureManagerBase from matplotlib.backends.backend_agg import FigureCanvasAgg from matplotlib.figure import Figure -from ..execute_request import PositronExecuteRequest, current_execute_request -from ..utils import jpeg_pixel_size, png_pixel_size -from . import formats, selects_module +from ..execute_request import current_execute_request +from . import Backend, configure_positron_support, formats -logger = logging.getLogger(__name__) +if TYPE_CHECKING: + from IPython.core.interactiveshell import InteractiveShell -# The pixel-size reader for each raster format's metadata. svg scales natively and -# pdf isn't rendered inline, so neither needs a pixel-size-derived `width`/`height`. -_PIXEL_SIZE_BY_FORMAT = {"png": png_pixel_size, "jpeg": jpeg_pixel_size} +logger = logging.getLogger(__name__) def new_figure_manager( @@ -69,7 +64,9 @@ def new_figure_manager( manager: FigureManagerPositronNotebook = cast( "FigureManagerPositronNotebook", FigureCanvasPositronNotebook.new_manager(figure, num) ) - manager.set_execute_request(execute_request) + # Set the device pixel ratio to the execute request's value, if provided. + if (pixel_ratio := execute_request.output_pixel_ratio) is not None: + manager.canvas._set_device_pixel_ratio(pixel_ratio) # type: ignore # noqa: SLF001 return manager @@ -91,69 +88,10 @@ def pyplot_show(cls, *, block: bool | None = None) -> None: if Gcf.get_all_fig_managers(): plt.close("all") - def set_execute_request(self, execute_request: PositronExecuteRequest): - """Set the current execute request.""" - self.execute_request = execute_request - self.canvas.set_execute_request(execute_request) - class FigureCanvasPositronNotebook(FigureCanvasAgg): manager_class = FigureManagerPositronNotebook # type: ignore - def set_execute_request(self, execute_request: PositronExecuteRequest) -> None: - """Set the current execute request.""" - # Set the device pixel ratio to the execute request's value, if provided. - if (pixel_ratio := execute_request.output_pixel_ratio) is not None: - self._set_device_pixel_ratio(pixel_ratio) # type: ignore - - -def _display_figure( - fig: Figure, *, format_="png", **print_figure_kwargs -) -> tuple[str, dict] | None: - """Render a figure to its Jupyter display wire format.""" - # NOTE: This implementation must match `IPython.core.pylabtools.print_figure` otherwise - # users will get different results in Positron vs other notebook editors. - - # Don't display empty figures; mirrors IPython. - if not fig.axes and not fig.lines: - return None - - # Render the figure to bytes. - canvas = fig.canvas - kw = { - "format": format_, - # Must pass `fig.dpi` otherwise `print_figure` renders at a pixel ratio of 1. - "dpi": fig.dpi, - # Tight bbox mirrors IPython. - "bbox_inches": "tight", - } - # Explicit kwargs (e.g. `set_matplotlib_formats(..., pil_kwargs=...)`) win, matching - # upstream's `print_figure`. - kw.update(print_figure_kwargs) - with io.BytesIO() as figure_buffer: - canvas.print_figure(figure_buffer, **kw) - data = figure_buffer.getvalue() - - # Decode bytes to string. - if format_ == "svg": - decoded = data.decode("utf-8") - else: - decoded = b2a_base64(data, newline=False).decode("ascii") - - # Prepare figure metadata. - metadata = {} - - # If the canvas has a custom device pixel ratio, include the intended pixel size in - # the metadata. Only raster formats (png, jpeg) support this: svg scales natively, - # and pdf isn't rendered inline. - pixel_size = _PIXEL_SIZE_BY_FORMAT.get(format_) - if pixel_size is not None and (ratio := canvas.device_pixel_ratio) != 1: - w, h = pixel_size(data) - metadata["width"] = int(w) / ratio - metadata["height"] = int(h) / ratio - - return decoded, metadata - # Fulfil the matplotlib backend API. FigureCanvas = FigureCanvasPositronNotebook @@ -168,26 +106,8 @@ def _show_figures(): logger.exception("Error showing figures in post execute hook") -# True while this backend's shell hooks are installed, so that repeated activation -# (e.g. `%matplotlib inline` twice) doesn't register duplicate hooks. -_active = False - - -def activate() -> None: - """Activate the Positron matplotlib notebook backend. Safe to call repeatedly.""" - global _active - if _active: - return - - shell = get_ipython() - if shell is None: - logger.warning("No IPython shell found; matplotlib notebook backend not activated") - return - - # I think the only part of enable_matplotlib_integration that we really need - # is to set interactive mode and to register the flush hook (below). - matplotlib.interactive(True) # noqa: FBT003 - +def install(shell: InteractiveShell) -> None: + """Install the notebook backend's shell hooks. See `BackendModule`.""" # Register a hook to show all figures after cell execution. shell.events.register("post_execute", _show_figures) @@ -200,32 +120,25 @@ def activate() -> None: # instead of IPython's. formats.install_set_matplotlib_formats_patch() - _active = True - - -def deactivate() -> None: - """Deactivate the Positron matplotlib notebook backend. Safe to call repeatedly.""" - global _active - _active = False - - shell = get_ipython() - if shell is None: - logger.warning("No IPython shell found; matplotlib notebook backend not deactivated") - return - # Unregister the post execute hook. Suppress ValueError so that deactivating a - # backend that was never activated is a no-op. +def uninstall(shell: InteractiveShell) -> None: + """Remove the notebook backend's shell hooks. See `BackendModule`.""" + # Suppress ValueError in case user code unregistered the hook itself; a backend + # switch shouldn't crash over it. with contextlib.suppress(ValueError): shell.events.unregister("post_execute", _show_figures) # Unregister our formatter(s) for matplotlib Figure objects. formats.pop_registered_formatters(shell) - # Undo the `set_matplotlib_formats` patch, unless the console flavor is still active. formats.uninstall_set_matplotlib_formats_patch() -# If we are the selected backend, activate. -# This is expected to run when the backend is selected. See the note at the top of the file. -if selects_module(matplotlib.get_backend(), __name__): - activate() +# If we are the selected backend, activate through the registry, which also tears +# down the other flavor if it was active. This runs when matplotlib imports the +# module to switch to it. See the note at the top of the file. `Backend.from_name` +# matches both spellings, which matters here: matplotlib can still report our short +# name via `get_backend()` at this point, before it settles into the `module://` +# spelling. +if Backend.from_name(matplotlib.get_backend()) is Backend.NOTEBOOK: + configure_positron_support(Backend.NOTEBOOK) diff --git a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_console.py b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_console.py index 2fdceb26a39..f66dd30433e 100644 --- a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_console.py +++ b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_console.py @@ -13,7 +13,7 @@ import pytest from PIL import Image -from positron.matplotlib_backend.console import activate, deactivate +from positron.matplotlib_backend import Backend, configure_positron_support from positron.plot_comm import PlotRenderFormat, PlotSize, PlotUnit from positron.plots import PlotsService from positron.positron_ipkernel import PositronIPyKernel, _CommTarget @@ -39,9 +39,9 @@ def setup_positron_matplotlib_backend() -> None: # The backend is set in the kernel app, which isn't currently available in our tests, # so set it here too. matplotlib.use("module://positron.matplotlib_backend.console") - activate() + configure_positron_support(Backend.CONSOLE) yield - deactivate() + configure_positron_support(prev) matplotlib.use(prev) @@ -667,26 +667,22 @@ def test_mpl_detect_library_walks_call_stack() -> None: def test_mpl_deactivate_restores_gca() -> None: - """`deactivate` restores the original `plt.gca` that `activate` patched over.""" + """Switching away restores the original `plt.gca` that installation patched over.""" import matplotlib.pyplot as plt from positron.matplotlib_backend import console - # The autouse fixture already activated, installing our redirect. + # The autouse fixture activated the console backend, installing our redirect. assert console._installed_gca is not None # noqa: SLF001 installed_gca = plt.gca original_gca = console._original_gca # noqa: SLF001 - console.deactivate() + configure_positron_support("agg") assert console._installed_gca is None # noqa: SLF001 assert plt.gca is original_gca assert plt.gca is not installed_gca - # Re-activate so later tests keep the redirect installed and the fixture's teardown - # `deactivate` (a no-op once uninstalled) stays balanced. - console.activate() - def test_mpl_shutdown(shell: PositronShell, plots_service: PlotsService) -> None: plot_comms = [_create_mpl_plot(shell, plots_service) for _ in range(2)] diff --git a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_formats.py b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_formats.py index 949e4224482..36f62328776 100644 --- a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_formats.py +++ b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_formats.py @@ -14,16 +14,18 @@ from __future__ import annotations import base64 +import io from typing import TYPE_CHECKING, Iterator import matplotlib import pytest +from IPython.core.display import _jpegxy, _pngxy from IPython.utils.capture import RichOutput, capture_output from matplotlib.backends.backend_agg import FigureCanvasAgg +from PIL import Image -from positron.matplotlib_backend import formats, notebook +from positron.matplotlib_backend import Backend, configure_positron_support, formats from positron.session_mode import SessionMode -from positron.utils import jpeg_pixel_size, png_pixel_size from ..utils import run_with_metadata @@ -48,11 +50,11 @@ def backend(shell: PositronShell) -> Iterator[PositronShell]: """A fixture that configures matplotlib to use the Positron notebook backend.""" prev = matplotlib.get_backend() matplotlib.use("module://positron.matplotlib_backend.notebook") - notebook.activate() + configure_positron_support(Backend.NOTEBOOK) yield shell - notebook.deactivate() + configure_positron_support(prev) matplotlib.use(prev) # `_selected_formats` is module state that intentionally survives a backend round @@ -104,14 +106,14 @@ def test_retina_aliases_to_png(backend: PositronShell): _set_formats(backend, "set_matplotlib_formats('retina')") baseline = _plot(meta={"output_pixel_ratio": 1}) - baseline_w, baseline_h = png_pixel_size(base64.b64decode(baseline.data["image/png"])) + baseline_w, baseline_h = _pngxy(base64.b64decode(baseline.data["image/png"])) ratio = 3 output = _plot(meta={"output_pixel_ratio": ratio}) assert "image/png" in output.data assert "image/jpeg" not in output.data - w, h = png_pixel_size(base64.b64decode(output.data["image/png"])) + w, h = _pngxy(base64.b64decode(output.data["image/png"])) metadata = output.metadata.get("image/png", {}) # Actual pixels scale with the requested ratio ... assert w == pytest.approx(ratio * baseline_w, abs=5) @@ -129,7 +131,7 @@ def test_jpeg_with_pixel_ratio(backend: PositronShell): assert "image/jpeg" in output.data jpeg = base64.b64decode(output.data["image/jpeg"]) - w, h = jpeg_pixel_size(jpeg) + w, h = _jpegxy(jpeg) metadata = output.metadata.get("image/jpeg", {}) assert metadata["width"] == pytest.approx(w / 2, abs=5) assert metadata["height"] == pytest.approx(h / 2, abs=5) @@ -191,14 +193,30 @@ def test_from_import_binding(backend: PositronShell): assert "image/png" not in output.data +def test_savefig_facecolor_rcparam_is_ignored(backend: PositronShell): + """Inline output uses the figure's own facecolor, not `savefig.facecolor`, matching Jupyter.""" + try: + backend.run_cell( + "import matplotlib.pyplot as plt\nplt.rcParams['savefig.facecolor'] = 'red'" + ).raise_error() + output = _plot() + png = base64.b64decode(output.data["image/png"]) + image = Image.open(io.BytesIO(png)).convert("RGB") + # The corner comes from the figure's default facecolor, not the red rcParam. + assert image.getpixel((0, 0)) != (255, 0, 0) + finally: + # rcParams are process-global and shared with the test shell; restore. + matplotlib.rcParams["savefig.facecolor"] = matplotlib.rcParamsDefault["savefig.facecolor"] + + def test_preference_survives_backend_round_trip(backend: PositronShell): """A format selection survives deactivating and reactivating the notebook backend.""" _set_formats(backend, "set_matplotlib_formats('svg')") # Simulate switching to a non-Positron backend and back (e.g. `%matplotlib qt` then # `%matplotlib inline`). - notebook.deactivate() - notebook.activate() + configure_positron_support("agg") + configure_positron_support(Backend.NOTEBOOK) output = _plot() diff --git a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_notebook.py b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_notebook.py index 1b0f8f9e4c3..5b1a4324113 100644 --- a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_notebook.py +++ b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_notebook.py @@ -10,11 +10,11 @@ import matplotlib import pytest +from IPython.core.display import _pngxy from IPython.utils.capture import RichOutput, capture_output -from positron.matplotlib_backend.notebook import activate, deactivate +from positron.matplotlib_backend import Backend, configure_positron_support from positron.session_mode import SessionMode -from positron.utils import png_pixel_size from ..utils import run_with_metadata @@ -38,11 +38,11 @@ def backend(shell): """A fixture that configures matplotlib to use the Positron notebook backend.""" prev = matplotlib.get_backend() matplotlib.use("module://positron.matplotlib_backend.notebook") - activate() + configure_positron_support(Backend.NOTEBOOK) yield NotebookBackendFixture(shell) - deactivate() + configure_positron_support(prev) matplotlib.use(prev) @@ -66,7 +66,7 @@ def figure_size(self): @property def png_pixel_size(self): - return png_pixel_size(self.png) + return _pngxy(self.png) class NotebookBackendFixture: diff --git a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_switching.py b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_switching.py index 9b4f50c330c..40c516c494e 100644 --- a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_switching.py +++ b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_switching.py @@ -9,8 +9,8 @@ keep the session's own Positron backend active, an explicit `positron-console` or `positron-notebook` activates that flavor outright (even across session modes), another backend tears Positron's hooks down, and switching back restores them. `agg` is the -"other" backend throughout so the tests run headless. Also covers `selects_module` and -`Backend`'s name-resolution helpers behind all of this. +"other" backend throughout so the tests run headless. Also covers `Backend`'s +name-resolution helper (`from_name`) behind all of this. """ from __future__ import annotations @@ -32,7 +32,6 @@ console, formats, notebook, - selects_module, ) from positron.session_mode import SessionMode @@ -73,7 +72,7 @@ def _notebook_integration_installed(shell: PositronShell) -> bool: # `formats.select_figure_formats`), not the bare function, since it also binds # `format_` and any `print_figure_kwargs`. current = formatter.lookup_by_type(Figure) - return isinstance(current, partial) and current.func is notebook._display_figure # noqa: SLF001 + return isinstance(current, partial) and current.func is formats._display_figure # noqa: SLF001 return False @@ -116,6 +115,12 @@ def notebook_backend(shell: PositronShell, monkeypatch: pytest.MonkeyPatch) -> I yield from _session_with_backend(NOTEBOOK, shell, monkeypatch) +@pytest.fixture +def console_backend(shell: PositronShell, monkeypatch: pytest.MonkeyPatch) -> Iterator[Flavor]: + """A console session with Positron's console backend active.""" + yield from _session_with_backend(CONSOLE, shell, monkeypatch) + + def _session_with_backend( flavor: Flavor, shell: PositronShell, monkeypatch: pytest.MonkeyPatch ) -> Iterator[Flavor]: @@ -132,12 +137,11 @@ def _session_with_backend( previous_backend = matplotlib.get_backend() matplotlib.use(flavor.backend.full_name) - flavor.module.activate() + matplotlib_backend.configure_positron_support(flavor.backend) yield flavor - console.deactivate() - notebook.deactivate() + matplotlib_backend.configure_positron_support(previous_backend) _reset_matplotlib_inline(shell) shell.kernel.plots_service.shutdown() plt.close("all") @@ -281,6 +285,44 @@ def test_explicit_short_name_selects_other_flavor_across_modes( } +def test_console_flavor_in_notebook_session_routes_to_plots_pane( + shell: PositronShell, + notebook_backend: Flavor, # noqa: ARG001 +): + """ + In a notebook session, `%matplotlib positron-console` routes figures to the Plots pane. + + The cross-mode switch is a feature: a notebook (or Quarto inline output) session can + opt its figures out of inline display and into the Plots pane's comm-based rendering. + """ + shell.run_cell(f"%matplotlib {Backend.CONSOLE.short_name}").raise_error() + + with capture_output() as captured: + shell.run_cell("import matplotlib.pyplot as plt\nfig, ax = plt.subplots()").raise_error() + + # The figure opened a plot comm (the Plots pane) instead of displaying inline. + assert len(shell.kernel.plots_service._plots) == 1 # noqa: SLF001 + assert not captured.outputs + + +def test_notebook_flavor_in_console_session_displays_inline( + shell: PositronShell, + console_backend: Flavor, # noqa: ARG001 +): + """In a console session, `%matplotlib positron-notebook` displays figures inline.""" + shell.run_cell(f"%matplotlib {Backend.NOTEBOOK.short_name}").raise_error() + + with capture_output() as captured: + shell.run_cell( + "import matplotlib.pyplot as plt\nfig, ax = plt.subplots()\nax.plot([0, 1], [0, 1])" + ).raise_error() + + # The figure displayed inline (as a PNG) instead of opening a plot comm. + assert not shell.kernel.plots_service._plots # noqa: SLF001 + assert len(captured.outputs) == 1 + assert captured.outputs[0]._repr_png_() is not None + + def test_positron_short_names_are_listed_backends(shell: PositronShell): """Both of Positron's backends are first-class names, so they show up in `%matplotlib -l`.""" with capture_output() as captured: @@ -351,10 +393,10 @@ def test_switching_back_reactivates(shell: PositronShell, positron_backend: Flav assert _state(shell, positron_backend) == _positron_active(positron_backend) -def test_deactivate_before_activate(shell: PositronShell, positron_backend: Flavor): - """Deactivating a backend that was never activated is a no-op, not an error.""" - positron_backend.module.deactivate() - positron_backend.module.deactivate() +def test_deactivate_twice_is_noop(shell: PositronShell, positron_backend: Flavor): + """A second teardown for a non-Positron backend is a no-op, not an error.""" + matplotlib_backend.configure_positron_support(OTHER_BACKEND_NAME) + matplotlib_backend.configure_positron_support(OTHER_BACKEND_NAME) assert _state(shell, positron_backend) == { "backend": positron_backend.backend.full_name, @@ -420,17 +462,18 @@ def test_set_matplotlib_formats_patch_restored_after_switch( assert backend_inline.set_matplotlib_formats is original -def test_notebook_to_console_switch_keeps_patch_installed( +def test_notebook_to_console_switch_reinstalls_patch( shell: PositronShell, notebook_backend: Flavor, # noqa: ARG001 ): """ - A notebook -> console switch runs activate-before-deactivate; the shared patch survives. + A notebook -> console switch now uninstalls before installing. - Pins the call-time-dispatch / shared-uninstall design in - `formats.install_set_matplotlib_formats_patch`: a per-flavor install/uninstall would - have the console flavor's activate immediately followed by the notebook flavor's - deactivate, unpatching the patch the activate just installed. + Pins the registry's ordering guarantee: the outgoing flavor's `uninstall` runs + before the incoming flavor's `install`, so the shared `set_matplotlib_formats` + patch is torn down and reinstalled (a fresh closure) across the switch instead of + surviving unchanged as it did under the old activate-before-deactivate ordering. + It stays correctly installed and dispatching afterwards. """ import matplotlib_inline.backend_inline as backend_inline @@ -438,33 +481,33 @@ def test_notebook_to_console_switch_keeps_patch_installed( shell.run_cell(f"%matplotlib {Backend.CONSOLE.short_name}").raise_error() - assert backend_inline.set_matplotlib_formats is installed_before + assert backend_inline.set_matplotlib_formats is not installed_before assert backend_inline.set_matplotlib_formats is formats._installed_set_matplotlib_formats # noqa: SLF001 -def test_selects_module_recognizes_own_names(flavor: Flavor): - """`selects_module` accepts a flavor's own short name and its `module://` name.""" - assert selects_module(flavor.backend.short_name, flavor.module.__name__) - assert selects_module(flavor.backend.full_name, flavor.module.__name__) +def test_from_name_recognizes_own_names(flavor: Flavor): + """`Backend.from_name` accepts a flavor's own short name and its `module://` name.""" + assert Backend.from_name(flavor.backend.short_name) is flavor.backend + assert Backend.from_name(flavor.backend.full_name) is flavor.backend -def test_selects_module_rejects_other_names(flavor: Flavor): - """`selects_module` rejects the other flavor's names and a foreign backend.""" +def test_from_name_rejects_other_names(flavor: Flavor): + """`Backend.from_name` doesn't resolve the other flavor's names to this flavor, or a foreign backend at all.""" other = _other_flavor(flavor) - assert not selects_module(other.backend.short_name, flavor.module.__name__) - assert not selects_module(other.backend.full_name, flavor.module.__name__) - assert not selects_module(OTHER_BACKEND_NAME, flavor.module.__name__) + assert Backend.from_name(other.backend.short_name) is not flavor.backend + assert Backend.from_name(other.backend.full_name) is not flavor.backend + assert Backend.from_name(OTHER_BACKEND_NAME) is None -def test_selects_module_short_name_case_insensitive(flavor: Flavor): +def test_from_name_short_name_case_insensitive(flavor: Flavor): """Short names match case-insensitively, like matplotlib's backend registry.""" - assert selects_module(flavor.backend.short_name.upper(), flavor.module.__name__) + assert Backend.from_name(flavor.backend.short_name.upper()) is flavor.backend -def test_selects_module_full_name_case_sensitive(flavor: Flavor): +def test_from_name_full_name_case_sensitive(flavor: Flavor): """The path after `module://` is an importable module path, so case matters.""" - assert not selects_module(f"module://{flavor.module.__name__.upper()}", flavor.module.__name__) + assert Backend.from_name(f"module://{flavor.module.__name__.upper()}") is None @pytest.mark.parametrize( diff --git a/extensions/positron-python/python_files/posit/positron/tests/test_utils.py b/extensions/positron-python/python_files/posit/positron/tests/test_utils.py index 664f1c15f7a..2126240fedc 100644 --- a/extensions/positron-python/python_files/posit/positron/tests/test_utils.py +++ b/extensions/positron-python/python_files/posit/positron/tests/test_utils.py @@ -3,14 +3,9 @@ # Licensed under the Elastic License 2.0. See LICENSE.txt for license information. # -import io - -import matplotlib -import matplotlib.pyplot as plt import pytest -from PIL import Image -from positron.utils import get_qualname, jpeg_pixel_size +from positron.utils import get_qualname class BadGetAttrImpl: @@ -27,25 +22,3 @@ def test_get_qualname_handles_bad_class(value) -> None: # qualname should be a valid string and not raise any errors assert isinstance(qualname, str), f"Expected string, got {type(qualname)}" assert qualname == "positron.tests.test_utils.BadGetAttrImpl" - - -def test_jpeg_pixel_size() -> None: - """`jpeg_pixel_size` reads a JPEG's pixel dimensions from its first SOF marker.""" - prev_backend = matplotlib.get_backend() - matplotlib.use("agg") - try: - fig, ax = plt.subplots(figsize=(4, 3), dpi=100) - ax.plot([0, 1], [0, 1]) - buffer = io.BytesIO() - fig.savefig(buffer, format="jpeg") - plt.close(fig) - finally: - matplotlib.use(prev_backend) - data = buffer.getvalue() - - # Cross-check against PIL's own JPEG header parsing rather than hardcoding the - # expected size, since the encoded size can differ slightly from the figure's - # requested inch size (dpi rounding). - expected = Image.open(io.BytesIO(data)).size - - assert jpeg_pixel_size(data) == expected diff --git a/extensions/positron-python/python_files/posit/positron/utils.py b/extensions/positron-python/python_files/posit/positron/utils.py index 6153c64efe5..2d119dd9e04 100644 --- a/extensions/positron-python/python_files/posit/positron/utils.py +++ b/extensions/positron-python/python_files/posit/positron/utils.py @@ -11,7 +11,6 @@ import logging import os import re -import struct import sys import threading import urllib.parse @@ -383,36 +382,3 @@ def get_command_uri(command: str, *args: str) -> str: def strip_ansi(text): """Strip ANSI escape sequences from text.""" return ansi_escape_re.sub("", text) - - -def png_pixel_size(data: bytes) -> tuple[int, int]: - """Read the (width, height) from a PNG's IHDR chunk.""" - ihdr = data.index(b"IHDR") - # The next 8 bytes after "IHDR" are the width and height, big-endian. - return struct.unpack(">ii", data[ihdr + 4 : ihdr + 12]) - - -# SOFn (start of frame) marker bytes that carry a JPEG's height/width. Excludes 0xC4 -# (DHT), 0xC8 (JPG, reserved), and 0xCC (DAC), which share the 0xC0-0xCF range but -# aren't SOF markers. -_JPEG_SOF_MARKERS = frozenset(range(0xC0, 0xD0)) - {0xC4, 0xC8, 0xCC} -# Markers with no length-prefixed payload to skip over: SOI/EOI and the RST markers. -_JPEG_STANDALONE_MARKERS = frozenset({0xD8, 0xD9, *range(0xD0, 0xD8)}) - - -def jpeg_pixel_size(data: bytes) -> tuple[int, int]: - """Read the (width, height) from a JPEG's first SOFn (start of frame) segment.""" - i = 2 # Skip the SOI marker (0xFFD8). - while i < len(data) - 1: - marker = data[i + 1] - i += 2 - if marker in _JPEG_STANDALONE_MARKERS: - continue - length = struct.unpack(">H", data[i : i + 2])[0] - if marker in _JPEG_SOF_MARKERS: - # SOFn payload: 1 byte precision, then height, width, both big-endian. - _, height, width = struct.unpack(">BHH", data[i + 2 : i + 7]) - return width, height - # Not a SOF marker; skip its payload and keep scanning. - i += length - raise ValueError("No SOF marker found in JPEG data") From 7a2079c0d96131b4059761db4d60a038ef6bebc2 Mon Sep 17 00:00:00 2001 From: seem Date: Wed, 29 Jul 2026 21:28:10 +0200 Subject: [PATCH 09/26] registry owns set_matplotlib_formats patch --- .../positron/matplotlib_backend/__init__.py | 10 +++++++ .../positron/matplotlib_backend/console.py | 8 +----- .../positron/matplotlib_backend/formats.py | 26 +++++++++---------- .../positron/matplotlib_backend/notebook.py | 6 ----- .../test_matplotlib_backend/test_switching.py | 15 +++++------ 5 files changed, 31 insertions(+), 34 deletions(-) diff --git a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/__init__.py b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/__init__.py index 5e436c352d4..159561f30c9 100644 --- a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/__init__.py +++ b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/__init__.py @@ -192,3 +192,13 @@ def configure_positron_support(backend: str | Backend) -> None: matplotlib.interactive(True) # noqa: FBT003 target.import_module().install(shell) _active_backend = target + + # The `set_matplotlib_formats` patch is flavor-agnostic (it dispatches at call + # time on the live backend), so it's owned here rather than by the flavors: + # installed while any Positron backend is active, removed when none is. + from . import formats + + if target is not None: + formats.install_set_matplotlib_formats_patch() + else: + formats.uninstall_set_matplotlib_formats_patch() diff --git a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/console.py b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/console.py index b5a45959c6c..25ece5ace63 100644 --- a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/console.py +++ b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/console.py @@ -29,7 +29,7 @@ from matplotlib.backends.backend_agg import FigureCanvasAgg from ..execute_request import PositronExecuteRequest -from . import Backend, configure_positron_support, formats +from . import Backend, configure_positron_support if TYPE_CHECKING: from IPython.core.interactiveshell import InteractiveShell @@ -414,10 +414,6 @@ def install(shell: InteractiveShell) -> None: _install_library_gca_redirect() - # Intercept `set_matplotlib_formats`: it's a no-op here, since the Plots pane - # negotiates its own format via `canvas.render(format_=...)`. - formats.install_set_matplotlib_formats_patch() - def uninstall(shell: InteractiveShell) -> None: """Remove the console backend's shell hooks. See `BackendModule`.""" @@ -428,8 +424,6 @@ def uninstall(shell: InteractiveShell) -> None: _uninstall_library_gca_redirect() - formats.uninstall_set_matplotlib_formats_patch() - # If we are the selected backend, activate through the registry, which also tears # down the other flavor if it was active. This runs when matplotlib imports the diff --git a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/formats.py b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/formats.py index 35b2f54d9ea..8be27062774 100644 --- a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/formats.py +++ b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/formats.py @@ -7,14 +7,13 @@ That function (and its sibling `select_figure_formats`) operate entirely inside matplotlib-inline/IPython, with no Positron code on the stack, so the only way to -intercept them is to patch `set_matplotlib_formats` itself. Both of Positron's -matplotlib backend flavors (console, notebook) install the *same* patch and leave it -dispatching at call time on whichever flavor is currently the active matplotlib -backend -- see `install_set_matplotlib_formats_patch` for why a shared, call-time -dispatch is needed instead of one patch per flavor. - -NOTE: only ever imported from a flavor's `install`/`uninstall`, by which point -matplotlib is guaranteed to be importable. +intercept them is to patch `set_matplotlib_formats` itself. The patch is installed +by `configure_positron_support` whenever a Positron backend (console or notebook) is +active, and removed once none is -- see `install_set_matplotlib_formats_patch` for why +a shared, call-time dispatch is needed instead of one patch per flavor. + +NOTE: only ever imported from `configure_positron_support`, by which point matplotlib +is guaranteed to be importable. """ from __future__ import annotations @@ -174,11 +173,12 @@ def install_set_matplotlib_formats_patch() -> None: """ Patch `matplotlib_inline.backend_inline.set_matplotlib_formats`. Safe to call repeatedly. - The patch is shared by both flavors and dispatches at call time on - `matplotlib.get_backend()`, so it stays correct however the active flavor changes - between install and call time. Dispatching on the live backend string rather than - the registry's state is deliberate: a user can switch backends with a bare - `matplotlib.use(...)` that never goes through the registry. + Installed by `configure_positron_support` whenever a Positron backend is active, + and dispatches at call time on `matplotlib.get_backend()`, so it stays correct + however the active flavor changes between install and call time. Dispatching on + the live backend string rather than the registry's state is deliberate: a user can + switch backends with a bare `matplotlib.use(...)` that never goes through the + registry. """ global _original_set_matplotlib_formats, _installed_set_matplotlib_formats if _installed_set_matplotlib_formats is not None: diff --git a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/notebook.py b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/notebook.py index 5a0aeb4c687..30d17a75d0d 100644 --- a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/notebook.py +++ b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/notebook.py @@ -116,10 +116,6 @@ def install(shell: InteractiveShell) -> None: # `set_matplotlib_formats`). formats.apply_selected_formats(shell) - # Intercept `set_matplotlib_formats` so a user's call re-registers our formatters - # instead of IPython's. - formats.install_set_matplotlib_formats_patch() - def uninstall(shell: InteractiveShell) -> None: """Remove the notebook backend's shell hooks. See `BackendModule`.""" @@ -131,8 +127,6 @@ def uninstall(shell: InteractiveShell) -> None: # Unregister our formatter(s) for matplotlib Figure objects. formats.pop_registered_formatters(shell) - formats.uninstall_set_matplotlib_formats_patch() - # If we are the selected backend, activate through the registry, which also tears # down the other flavor if it was active. This runs when matplotlib imports the diff --git a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_switching.py b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_switching.py index 40c516c494e..7094ed5ea66 100644 --- a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_switching.py +++ b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_switching.py @@ -462,18 +462,17 @@ def test_set_matplotlib_formats_patch_restored_after_switch( assert backend_inline.set_matplotlib_formats is original -def test_notebook_to_console_switch_reinstalls_patch( +def test_notebook_to_console_switch_keeps_patch_installed( shell: PositronShell, notebook_backend: Flavor, # noqa: ARG001 ): """ - A notebook -> console switch now uninstalls before installing. + A notebook -> console switch keeps the same `set_matplotlib_formats` patch installed. - Pins the registry's ordering guarantee: the outgoing flavor's `uninstall` runs - before the incoming flavor's `install`, so the shared `set_matplotlib_formats` - patch is torn down and reinstalled (a fresh closure) across the switch instead of - surviving unchanged as it did under the old activate-before-deactivate ordering. - It stays correctly installed and dispatching afterwards. + The patch's lifecycle is owned by `configure_positron_support` ("a Positron backend + is active"), not by the individual flavors, so a cross-flavor switch -- which never + lets `_active_backend` become `None` in between -- leaves the same patch object in + place instead of tearing it down and reinstalling a fresh one. """ import matplotlib_inline.backend_inline as backend_inline @@ -481,7 +480,7 @@ def test_notebook_to_console_switch_reinstalls_patch( shell.run_cell(f"%matplotlib {Backend.CONSOLE.short_name}").raise_error() - assert backend_inline.set_matplotlib_formats is not installed_before + assert backend_inline.set_matplotlib_formats is installed_before assert backend_inline.set_matplotlib_formats is formats._installed_set_matplotlib_formats # noqa: SLF001 From 3fd9fd4fd0767c7a3bacdcc82494b63725e41135 Mon Sep 17 00:00:00 2001 From: seem Date: Wed, 29 Jul 2026 21:35:27 +0200 Subject: [PATCH 10/26] narrow enable_matplotlib override to a seam --- .../positron/matplotlib_backend/__init__.py | 71 ++++++++++++ .../posit/positron/positron_ipkernel.py | 108 +++++------------- .../test_matplotlib_backend/test_switching.py | 16 +++ 3 files changed, 117 insertions(+), 78 deletions(-) diff --git a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/__init__.py b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/__init__.py index 159561f30c9..70e553c445a 100644 --- a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/__init__.py +++ b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/__init__.py @@ -20,6 +20,7 @@ import importlib import logging +import sys from enum import Enum from typing import TYPE_CHECKING, Protocol, cast @@ -202,3 +203,73 @@ def configure_positron_support(backend: str | Backend) -> None: formats.install_set_matplotlib_formats_patch() else: formats.uninstall_set_matplotlib_formats_patch() + + +_MATPLOTLIB_INLINE_MODULE = "matplotlib_inline.backend_inline" +_MATPLOTLIB_INLINE_BACKENDS = ("inline", f"module://{_MATPLOTLIB_INLINE_MODULE}") + + +def _needs_inline_support(shell: InteractiveShell, backend: str) -> bool: + """ + Whether matplotlib-inline's `configure_inline_support` should be called for `backend`. + + Only when matplotlib-inline is the backend being switched to, or when it's currently + active and needs tearing down. Calling it in any other case would instantiate + `InlineBackend` into `shell.configurables`, arming a traitlets observer that re-runs + `select_figure_formats` -- popping Positron's figure formatter -- on any later + `%config InlineBackend.*` assignment. + """ + if backend in _MATPLOTLIB_INLINE_BACKENDS: + return True + + # matplotlib-inline is active if its post execute hook is registered, whether by an + # earlier switch through here or by its backend module self-activating on import. + module = sys.modules.get(_MATPLOTLIB_INLINE_MODULE) + flush_figures = getattr(module, "flush_figures", None) + return flush_figures is not None and flush_figures in shell.events.callbacks.get( + "post_execute", [] + ) + + +# The real `configure_inline_support`, captured when the switch hook is installed. +_original_configure_inline_support = None + + +def install_backend_switch_hook() -> None: + """ + Wrap `matplotlib_inline.backend_inline.configure_inline_support` with Positron's seam. + + IPython's `enable_matplotlib` imports that function from the module at call time on + every backend switch, so wrapping the module attribute makes every switch -- including + ones to foreign backends via `super()` -- run `configure_matplotlib_support`. + Idempotent. Installed lazily (not at kernel init) because importing matplotlib_inline + pulls in matplotlib, which may not be installed until the user actually plots. + Never uninstalled, matching the kernel's other third-party patches; the wrapper + preserves upstream behavior whenever Positron isn't involved. + """ + global _original_configure_inline_support + if _original_configure_inline_support is not None: + return + + import matplotlib_inline.backend_inline as backend_inline + + _original_configure_inline_support = backend_inline.configure_inline_support + backend_inline.configure_inline_support = configure_matplotlib_support + + +def configure_matplotlib_support(shell: InteractiveShell, backend: str) -> None: + """ + Configure inline figure display after a matplotlib backend switch. + + matplotlib-inline is configured only when it's involved (see + `_needs_inline_support`): calling it for any other switch would instantiate + `InlineBackend` into `shell.configurables`, arming a traitlets observer that pops + Positron's figure formatter on any later `%config InlineBackend.*` assignment. + Positron's support is configured on every switch, and goes last: tearing down + matplotlib-inline re-runs `select_figure_formats`, which pops the figure formatter + that activating Positron's backend then registers. + """ + install_backend_switch_hook() + if _needs_inline_support(shell, backend): + _original_configure_inline_support(shell, backend) + configure_positron_support(backend) diff --git a/extensions/positron-python/python_files/posit/positron/positron_ipkernel.py b/extensions/positron-python/python_files/posit/positron/positron_ipkernel.py index 3e68610a352..2f91355e200 100644 --- a/extensions/positron-python/python_files/posit/positron/positron_ipkernel.py +++ b/extensions/positron-python/python_files/posit/positron/positron_ipkernel.py @@ -251,31 +251,6 @@ def _positron_exec_metadata(self, line: str) -> None: # noqa: ARG002 # keep reference to original showwarning original_showwarning = warnings.showwarning -_MATPLOTLIB_INLINE_MODULE = "matplotlib_inline.backend_inline" -_MATPLOTLIB_INLINE_BACKENDS = ("inline", f"module://{_MATPLOTLIB_INLINE_MODULE}") - - -def _needs_inline_support(shell: InteractiveShell, backend: str) -> bool: - """ - Whether matplotlib-inline's `configure_inline_support` should be called for `backend`. - - Only when matplotlib-inline is the backend being switched to, or when it's currently - active and needs tearing down. Calling it in any other case would instantiate - `InlineBackend` into `shell.configurables`, arming a traitlets observer that re-runs - `select_figure_formats` -- popping Positron's figure formatter -- on any later - `%config InlineBackend.*` assignment. - """ - if backend in _MATPLOTLIB_INLINE_BACKENDS: - return True - - # matplotlib-inline is active if its post execute hook is registered, whether by an - # earlier switch through here or by its backend module self-activating on import. - module = sys.modules.get(_MATPLOTLIB_INLINE_MODULE) - flush_figures = getattr(module, "flush_figures", None) - return flush_figures is not None and flush_figures in shell.events.callbacks.get( - "post_execute", [] - ) - class PositronShell(ZMQInteractiveShell): kernel: PositronIPyKernel @@ -374,73 +349,50 @@ def enable_matplotlib(self, gui=None): """ Enable interactive matplotlib and inline figure support. - Overrides IPython so that bare `%matplotlib` and `%matplotlib inline` both - activate the Positron matplotlib backend that suits the session mode, and so - that switching to and from another backend (`%matplotlib qt`) installs and - removes its shell hooks. - - Mirrors `InteractiveShell.enable_matplotlib` as of IPython 9.14. The body is - inlined rather than delegated to `super()` because upstream unconditionally calls - `matplotlib_inline`'s `configure_inline_support`, which instantiates - `InlineBackend` into `shell.configurables` -- arming a traitlets observer that - pops Positron's figure formatter on any later `%config InlineBackend.*` - assignment. Drift on IPython upgrades is caught by the backend switch tests. + Overrides IPython so that bare `%matplotlib`, `%matplotlib inline`, and + Positron's own backend names activate the Positron backend that suits the + session mode (or the explicitly named flavor). Every other backend is + delegated to IPython, whose switch path runs through the hook installed by + `install_backend_switch_hook`, so switching away tears Positron's hooks down. """ from IPython.core import pylabtools as pt - from .matplotlib_backend import configure_positron_support + from .matplotlib_backend import ( + configure_matplotlib_support, + install_backend_switch_hook, + ) + + # Install the seam before any switch below can need it. Idempotent; lazy + # because it imports matplotlib (see its docstring). + install_backend_switch_hook() - # 1. Select the matplotlib backend and gui event loop. requested = gui.lower() if isinstance(gui, str) else gui - requested_backend = Backend.from_name(requested) if isinstance(requested, str) else None + target = Backend.from_name(requested) if isinstance(requested, str) else None if requested in (None, "auto", "inline"): # Positron's backend is both the session's default and its inline backend. # This is a reroute through the full switch path rather than a no-op when # already active, so that `%matplotlib inline` after `%matplotlib qt` # switches back. See https://github.com/posit-dev/positron/issues/15116. - gui, backend = None, Backend.for_session_mode(self.session_mode).preferred_name - elif requested_backend is not None: - # An explicit `%matplotlib positron-console`/`positron-notebook` (either - # spelling) selects that flavor outright, even across session modes. - # Resolved here rather than through `pt.find_gui_and_backend`, which only - # knows entry-point backends like ours on IPython >= 8.24 with matplotlib - # >= 3.9; earlier IPython raises KeyError for them. - gui, backend = None, requested_backend.preferred_name - else: - gui, backend = pt.find_gui_and_backend(gui, self.pylab_gui_select) - if gui is not None: - # If we have our first gui selection, store it. - if self.pylab_gui_select is None: - self.pylab_gui_select = gui - # Otherwise if they are different, stay with the first one. - elif gui != self.pylab_gui_select: - print( - f"Warning: Cannot change to a different GUI toolkit: {gui}." - f" Using {self.pylab_gui_select} instead." - ) - gui, backend = pt.find_gui_and_backend(self.pylab_gui_select) - - # 2. Set up matplotlib for interactive use with that backend. + target = Backend.for_session_mode(self.session_mode) + + if target is None: + # A non-Positron backend (qt, agg, matplotlib-inline by module name, ...). + return super().enable_matplotlib(gui) + + # A Positron backend: switch directly instead of via super(). + # `pt.find_gui_and_backend` only resolves entry-point names like ours on + # IPython >= 8.24 with matplotlib >= 3.9, and Positron backends never have a + # gui event loop, so IPython's gui-selection logic doesn't apply. The recipe + # below matches `InteractiveShell.enable_matplotlib`'s: activate, configure + # inline support, enable the gui event loop, fix `%run`. + backend = target.preferred_name pt.activate_matplotlib(backend) - - # 3. Configure inline figure display: matplotlib-inline only when it's involved, - # and Positron's backend on every switch. Positron's goes last: tearing down - # matplotlib-inline re-runs `select_figure_formats`, which pops the figure - # formatter that activating Positron's backend then registers. - if _needs_inline_support(self, backend): - from matplotlib_inline.backend_inline import configure_inline_support - - configure_inline_support(self, backend) - configure_positron_support(backend) - - # 4. Enable the selected gui event loop, and fix %run to take plot updates into - # account. - self.enable_gui(gui) + configure_matplotlib_support(self, backend) + self.enable_gui(None) self.magics_manager.registry["ExecutionMagics"].default_runner = pt.mpl_runner( self.safe_execfile ) - - return gui, backend + return None, backend def _inspect(self, meth, oname, namespaces=None, **kw): # For `?name`, if the name isn't a live object but is an installed diff --git a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_switching.py b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_switching.py index 7094ed5ea66..051ba6b61eb 100644 --- a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_switching.py +++ b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_switching.py @@ -534,3 +534,19 @@ def test_preferred_name_falls_back_to_module_name(flavor: Flavor, monkeypatch: p monkeypatch.setattr(matplotlib_backend, "_get_backend_registry", lambda: None) assert flavor.backend.preferred_name == flavor.backend.full_name + + +def test_install_backend_switch_hook_is_idempotent(): + """Calling `install_backend_switch_hook` twice doesn't double-wrap `configure_inline_support`.""" + import matplotlib_inline.backend_inline as backend_inline + + matplotlib_backend.install_backend_switch_hook() + matplotlib_backend.install_backend_switch_hook() + + assert ( + backend_inline.configure_inline_support is matplotlib_backend.configure_matplotlib_support + ) + assert ( + matplotlib_backend._original_configure_inline_support # noqa: SLF001 + is not matplotlib_backend.configure_matplotlib_support + ) From 8952c8e4ec47a38b191c35fbe9fd1443103ffb1f Mon Sep 17 00:00:00 2001 From: seem Date: Wed, 29 Jul 2026 21:32:16 +0200 Subject: [PATCH 11/26] shared backend activation helper for tests --- .../tests/test_matplotlib_backend/conftest.py | 29 +++++++++++++++++++ .../test_matplotlib_backend/test_console.py | 10 ++----- .../test_matplotlib_backend/test_formats.py | 11 ++----- .../test_matplotlib_backend/test_notebook.py | 13 +++------ .../test_matplotlib_backend/test_switching.py | 17 ++++------- 5 files changed, 45 insertions(+), 35 deletions(-) create mode 100644 extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/conftest.py diff --git a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/conftest.py b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/conftest.py new file mode 100644 index 00000000000..af1a198c20b --- /dev/null +++ b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/conftest.py @@ -0,0 +1,29 @@ +# +# Copyright (C) 2023-2024 Posit Software, PBC. All rights reserved. +# Licensed under the Elastic License 2.0. See LICENSE.txt for license information. +# + +from __future__ import annotations + +import contextlib +from typing import TYPE_CHECKING + +import matplotlib + +from positron.matplotlib_backend import Backend, configure_positron_support + +if TYPE_CHECKING: + from typing import Iterator + + +@contextlib.contextmanager +def active_backend(backend: Backend) -> Iterator[None]: + """Activate a Positron matplotlib backend, restoring the previous backend on exit.""" + prev = matplotlib.get_backend() + matplotlib.use(backend.full_name) + configure_positron_support(backend) + try: + yield + finally: + configure_positron_support(prev) + matplotlib.use(prev) diff --git a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_console.py b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_console.py index f66dd30433e..191eb99a6df 100644 --- a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_console.py +++ b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_console.py @@ -8,7 +8,6 @@ from pathlib import Path from typing import Any, Dict, Iterable, Optional, Tuple, cast -import matplotlib import matplotlib.pyplot as plt import pytest from PIL import Image @@ -25,6 +24,7 @@ json_rpc_response, percent_difference, ) +from .conftest import active_backend # # Matplotlib backend + shell + plots service integration tests. @@ -35,14 +35,10 @@ @pytest.fixture(autouse=True) def setup_positron_matplotlib_backend() -> None: - prev = matplotlib.get_backend() # The backend is set in the kernel app, which isn't currently available in our tests, # so set it here too. - matplotlib.use("module://positron.matplotlib_backend.console") - configure_positron_support(Backend.CONSOLE) - yield - configure_positron_support(prev) - matplotlib.use(prev) + with active_backend(Backend.CONSOLE): + yield @pytest.fixture(autouse=True) diff --git a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_formats.py b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_formats.py index 36f62328776..2ff2a064143 100644 --- a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_formats.py +++ b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_formats.py @@ -28,6 +28,7 @@ from positron.session_mode import SessionMode from ..utils import run_with_metadata +from .conftest import active_backend if TYPE_CHECKING: from positron.positron_ipkernel import PositronShell @@ -48,14 +49,8 @@ def _setup(shell, monkeypatch): @pytest.fixture def backend(shell: PositronShell) -> Iterator[PositronShell]: """A fixture that configures matplotlib to use the Positron notebook backend.""" - prev = matplotlib.get_backend() - matplotlib.use("module://positron.matplotlib_backend.notebook") - configure_positron_support(Backend.NOTEBOOK) - - yield shell - - configure_positron_support(prev) - matplotlib.use(prev) + with active_backend(Backend.NOTEBOOK): + yield shell # `_selected_formats` is module state that intentionally survives a backend round # trip (see `test_preference_survives_backend_round_trip`), so it doesn't reset diff --git a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_notebook.py b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_notebook.py index 5b1a4324113..a8c719875c8 100644 --- a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_notebook.py +++ b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_notebook.py @@ -13,10 +13,11 @@ from IPython.core.display import _pngxy from IPython.utils.capture import RichOutput, capture_output -from positron.matplotlib_backend import Backend, configure_positron_support +from positron.matplotlib_backend import Backend from positron.session_mode import SessionMode from ..utils import run_with_metadata +from .conftest import active_backend if TYPE_CHECKING: from matplotlib.figure import Figure @@ -36,14 +37,8 @@ def _setup(shell, monkeypatch): @pytest.fixture def backend(shell): """A fixture that configures matplotlib to use the Positron notebook backend.""" - prev = matplotlib.get_backend() - matplotlib.use("module://positron.matplotlib_backend.notebook") - configure_positron_support(Backend.NOTEBOOK) - - yield NotebookBackendFixture(shell) - - configure_positron_support(prev) - matplotlib.use(prev) + with active_backend(Backend.NOTEBOOK): + yield NotebookBackendFixture(shell) def _parse_png_output(output: RichOutput) -> tuple[bytes, dict]: diff --git a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_switching.py b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_switching.py index 051ba6b61eb..5953a6d0915 100644 --- a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_switching.py +++ b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_switching.py @@ -36,6 +36,7 @@ from positron.session_mode import SessionMode from ..utils import run_with_metadata +from .conftest import active_backend if TYPE_CHECKING: from types import ModuleType @@ -135,17 +136,11 @@ def _session_with_backend( # Entering a gui event loop needs a running kernel application, which tests don't have. monkeypatch.setattr(shell, "enable_gui", lambda gui=None: None) # noqa: ARG005 - previous_backend = matplotlib.get_backend() - matplotlib.use(flavor.backend.full_name) - matplotlib_backend.configure_positron_support(flavor.backend) - - yield flavor - - matplotlib_backend.configure_positron_support(previous_backend) - _reset_matplotlib_inline(shell) - shell.kernel.plots_service.shutdown() - plt.close("all") - matplotlib.use(previous_backend) + with active_backend(flavor.backend): + yield flavor + _reset_matplotlib_inline(shell) + shell.kernel.plots_service.shutdown() + plt.close("all") def _reset_matplotlib_inline(shell: PositronShell) -> None: From 3e75ba5e63118cc8f7509adc9a8c4a3828e9eac5 Mon Sep 17 00:00:00 2001 From: seem Date: Thu, 30 Jul 2026 15:44:23 +0200 Subject: [PATCH 12/26] public set_device_pixel_ratio on the notebook canvas also narrow the figsize passed to Figure for pyright on the 3.9 pins, and refresh the sizing-precedence and show() docs --- .../positron/matplotlib_backend/notebook.py | 28 +++++++++++++------ 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/notebook.py b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/notebook.py index 30d17a75d0d..94458fd2b05 100644 --- a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/notebook.py +++ b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/notebook.py @@ -52,13 +52,14 @@ def new_figure_manager( # Get the current execute request. execute_request = current_execute_request() - # Sizing precedence: - # 1. per-figure code (`plt.figure(figsize=...)` or `plt.subplots(figsize=...)`), - # which passes non-None figsize - # 2. cell execute request (`#| fig-width`` and `#| fig-height`) - # 3. matplotlib config (`plt.rcParams`) + # Sizing precedence: an explicit size from user code (`plt.figure(figsize=...)`, + # `plt.subplots(figsize=...)`) wins, else the cell's `#| fig-width`/`#| fig-height`, + # else matplotlib's `figure.figsize` rcParam, which `Figure` applies for a None figsize. figsize = figsize or execute_request.figure_size - figure = FigureClass(*args, figsize=figsize, **kwargs) + # `Figure` only accepts the 3-tuple form in matplotlib >= 3.11, but we pass whatever + # matplotlib handed us straight back to it, so the pair always matches at runtime. + # The cast keeps pyright quiet against the older pins (CI type-checks on 3.9). + figure = FigureClass(*args, figsize=cast("tuple[float, float] | None", figsize), **kwargs) # Also provide the execute request to the figure manager. manager: FigureManagerPositronNotebook = cast( @@ -66,7 +67,7 @@ def new_figure_manager( ) # Set the device pixel ratio to the execute request's value, if provided. if (pixel_ratio := execute_request.output_pixel_ratio) is not None: - manager.canvas._set_device_pixel_ratio(pixel_ratio) # type: ignore # noqa: SLF001 + manager.canvas.set_device_pixel_ratio(pixel_ratio) return manager @@ -75,7 +76,10 @@ class FigureManagerPositronNotebook(FigureManagerBase): canvas: FigureCanvasPositronNotebook # type: ignore def show(self): - """Called by matplotlib when a figure is shown via `plt.show()` or `figure.show()`.""" + """Called by matplotlib when a figure is shown via `plt.show()` or `figure.show()`. + + Displays the figure inline, as an output of the currently executing cell. + """ display(self.canvas.figure) @classmethod @@ -92,6 +96,14 @@ def pyplot_show(cls, *, block: bool | None = None) -> None: class FigureCanvasPositronNotebook(FigureCanvasAgg): manager_class = FigureManagerPositronNotebook # type: ignore + def set_device_pixel_ratio(self, ratio: float) -> None: + """Scale rendered pixels by `ratio`, leaving the figure's size in inches unchanged. + + A public seam over matplotlib's protected setter, since the ratio is applied by + `new_figure_manager` from outside the canvas. + """ + self._set_device_pixel_ratio(ratio) # type: ignore + # Fulfil the matplotlib backend API. FigureCanvas = FigureCanvasPositronNotebook From ac7f616faa75b210818a02ce3a027ed9c7d66c17 Mon Sep 17 00:00:00 2001 From: seem Date: Thu, 30 Jul 2026 15:44:23 +0200 Subject: [PATCH 13/26] move matplotlib bootstrap out of init_magics; drop double normalization --- .../posit/positron/positron_ipkernel.py | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/extensions/positron-python/python_files/posit/positron/positron_ipkernel.py b/extensions/positron-python/python_files/posit/positron/positron_ipkernel.py index 2f91355e200..9c34085554e 100644 --- a/extensions/positron-python/python_files/posit/positron/positron_ipkernel.py +++ b/extensions/positron-python/python_files/posit/positron/positron_ipkernel.py @@ -320,10 +320,6 @@ def init_magics(self): # Register Positron's custom magics. self.register_magics(PositronMagics) - # On IPython versions that don't read matplotlib's backend registry, add - # Positron's backends to the static table that `%matplotlib -l` lists. - register_with_legacy_ipython() - def init_user_ns(self): super().init_user_ns() @@ -351,9 +347,14 @@ def enable_matplotlib(self, gui=None): Overrides IPython so that bare `%matplotlib`, `%matplotlib inline`, and Positron's own backend names activate the Positron backend that suits the - session mode (or the explicitly named flavor). Every other backend is + session mode (or the explicitly named backend). Every other backend is delegated to IPython, whose switch path runs through the hook installed by `install_backend_switch_hook`, so switching away tears Positron's hooks down. + + Overriding here rather than the `%matplotlib` magic also covers + `--matplotlib`/`--pylab` and their config-file equivalents: + `InteractiveShellApp.init_gui_pylab` calls `shell.enable_matplotlib` directly at + startup, never going through the magic. """ from IPython.core import pylabtools as pt @@ -362,12 +363,19 @@ def enable_matplotlib(self, gui=None): install_backend_switch_hook, ) - # Install the seam before any switch below can need it. Idempotent; lazy - # because it imports matplotlib (see its docstring). + # Both calls are idempotent bootstrap steps that only have to be done before the + # first switch, so they're done lazily here rather than at shell init. + # Install the seam before any switch below can need it; lazy because it imports + # matplotlib (see its docstring). install_backend_switch_hook() + # On IPython versions that don't read matplotlib's backend registry, add + # Positron's backends to the static table that `%matplotlib -l` lists. + register_with_legacy_ipython() + # Only the sentinel check case-folds: `Backend.from_name` normalizes short names + # itself, and keeps `module://` paths case-sensitive since they're module paths. requested = gui.lower() if isinstance(gui, str) else gui - target = Backend.from_name(requested) if isinstance(requested, str) else None + target = Backend.from_name(gui) if isinstance(gui, str) else None if requested in (None, "auto", "inline"): # Positron's backend is both the session's default and its inline backend. # This is a reroute through the full switch path rather than a no-op when From c9dd98b98b58a2dde313f98a4728a052f75b79a9 Mon Sep 17 00:00:00 2001 From: seem Date: Thu, 30 Jul 2026 15:44:31 +0200 Subject: [PATCH 14/26] test explicit show and cross-cell figure cleanup in the notebook backend --- .../test_matplotlib_backend/test_notebook.py | 47 ++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_notebook.py b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_notebook.py index a8c719875c8..d9bbef0e9eb 100644 --- a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_notebook.py +++ b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_notebook.py @@ -9,6 +9,7 @@ from typing import TYPE_CHECKING import matplotlib +import matplotlib.pyplot as plt import pytest from IPython.core.display import _pngxy from IPython.utils.capture import RichOutput, capture_output @@ -71,6 +72,12 @@ def __init__(self, shell: PositronShell) -> None: def import_matplotlib(self) -> None: self.shell.run_cell("import matplotlib.pyplot as plt").raise_error() + def run(self, *lines: str) -> list[RichOutput]: + """Run a cell and return its display outputs.""" + with capture_output() as captured: + self.shell.run_cell("\n".join(lines)).raise_error() + return captured.outputs + def plot( self, *, @@ -183,4 +190,42 @@ def test_pixel_ratio_zero_raises(backend, ratio): backend.plot(meta={"output_pixel_ratio": ratio}) -# TODO: Test plt.show and other apis, display(fig)? +def test_plt_show_displays_figure(backend): + """An explicit `plt.show()` displays the figure inline.""" + outputs = backend.run( + "import matplotlib.pyplot as plt", + "fig, ax = plt.subplots()", + "ax.plot([0, 1], [0, 1])", + "plt.show()", + ) + + # The lone output comes from the explicit `plt.show()`: it closes every figure, so the + # post-execute hook has nothing left to display. + assert len(outputs) == 1 + assert plt.get_fignums() == [] + png, _metadata = _parse_png_output(outputs[0]) + assert png.startswith(b"\x89PNG") + + +def test_figure_show_displays_figure(backend): + """An explicit `fig.show()` displays the figure inline.""" + outputs = backend.run( + "import matplotlib.pyplot as plt", + "fig, ax = plt.subplots()", + "ax.plot([0, 1], [0, 1])", + "fig.show()", + ) + + # Two outputs: the explicit `fig.show()`, then the post-execute hook, which still finds + # the figure since `Figure.show` doesn't close it. + assert len(outputs) == 2 + + +def test_figures_do_not_accumulate_across_cells(backend): + """Figures are closed once shown, so a later cell doesn't re-display earlier ones.""" + backend.plot() + assert plt.get_fignums() == [] + + # `plot` asserts a single output, which would also fail if the first figure lingered. + backend.plot() + assert plt.get_fignums() == [] From a17407893b71199f16370600743c1a7dcef28302 Mon Sep 17 00:00:00 2001 From: seem Date: Thu, 30 Jul 2026 15:44:41 +0200 Subject: [PATCH 15/26] keep the old test images gitignore path --- extensions/positron-python/.gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/extensions/positron-python/.gitignore b/extensions/positron-python/.gitignore index 78c269b907a..bd4f92b7703 100644 --- a/extensions/positron-python/.gitignore +++ b/extensions/positron-python/.gitignore @@ -51,6 +51,8 @@ dist/** l10n/ tags # --- Start Positron --- +# The old location, kept so clones that ran the pre-split suite stay clean. +python_files/posit/positron/tests/images python_files/posit/positron/tests/test_matplotlib_backend/images python_files/posit/positron/_vendor/** scripts/*.js From 5924597bf2e21984da253a5fdea8edbb9964bb68 Mon Sep 17 00:00:00 2001 From: seem Date: Thu, 30 Jul 2026 15:44:41 +0200 Subject: [PATCH 16/26] move console set_matplotlib_formats test to test_formats --- .../test_matplotlib_backend/test_console.py | 24 +---- .../test_matplotlib_backend/test_formats.py | 92 ++++++++++++------- 2 files changed, 61 insertions(+), 55 deletions(-) diff --git a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_console.py b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_console.py index 191eb99a6df..ed76e57cec7 100644 --- a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_console.py +++ b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_console.py @@ -6,7 +6,7 @@ import base64 import io from pathlib import Path -from typing import Any, Dict, Iterable, Optional, Tuple, cast +from typing import Any, Dict, Iterable, Iterator, Optional, Tuple, cast import matplotlib.pyplot as plt import pytest @@ -34,7 +34,7 @@ @pytest.fixture(autouse=True) -def setup_positron_matplotlib_backend() -> None: +def setup_positron_matplotlib_backend() -> Iterator[None]: # The backend is set in the kernel app, which isn't currently available in our tests, # so set it here too. with active_backend(Backend.CONSOLE): @@ -696,26 +696,6 @@ def test_mpl_shutdown(shell: PositronShell, plots_service: PlotsService) -> None assert all(comm._closed for comm in plot_comms) # noqa: SLF001 -def test_set_matplotlib_formats_is_noop_in_console(shell: PositronShell) -> None: - """ - `set_matplotlib_formats` is a silent no-op in console mode. - - The console backend routes figures to the Plots pane via `canvas.render`, which - negotiates its own format; running IPython's `select_figure_formats` here would - register stray inline `image/*` display formatters alongside it (see - `formats.install_set_matplotlib_formats_patch`). - """ - from matplotlib.figure import Figure - - shell.run_cell( - "from matplotlib_inline.backend_inline import set_matplotlib_formats\n" - "set_matplotlib_formats('png')" - ).raise_error() - - for formatter in shell.display_formatter.formatters.values(): - assert Figure not in formatter - - def test_plotnine_close_then_show(shell: PositronShell, plots_service: PlotsService) -> None: """Test that a plotnine plot renders and then closes comm correctly.""" shell.run_cell("""\ diff --git a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_formats.py b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_formats.py index 2ff2a064143..28295ad27c8 100644 --- a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_formats.py +++ b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_formats.py @@ -3,12 +3,14 @@ # Licensed under the Elastic License 2.0. See LICENSE.txt for license information. # """ -Tests for `set_matplotlib_formats` support in Positron's notebook matplotlib backend. - -Exercises `matplotlib_backend/formats.py`: format selection and aliasing -(`retina`/`png2x` -> `png`), kwargs passthrough, the `set_matplotlib_formats` patch's -`from`-import binding, and that a format selection survives a round trip through a -non-Positron backend. +Tests for `set_matplotlib_formats` support in Positron's matplotlib backends. + +Exercises `matplotlib_backend/formats.py` against the notebook backend, which honours +the call: format selection and aliasing (`retina`/`png2x` -> `png`), kwargs passthrough, +the `set_matplotlib_formats` patch's `from`-import binding, and that a format selection +survives a round trip through a non-Positron backend. Also covers the console backend, +where the call is a silent no-op because figures go to the Plots pane, which negotiates +its own format. """ from __future__ import annotations @@ -40,27 +42,34 @@ _PLOT_CODE = "import matplotlib.pyplot as plt\nfig, ax = plt.subplots()\nax.plot([0, 1], [0, 1])" -@pytest.fixture(autouse=True) -def _setup(shell, monkeypatch): - """Configure the shell to match the notebook environment.""" - monkeypatch.setattr(shell, "session_mode", SessionMode.NOTEBOOK) - - @pytest.fixture -def backend(shell: PositronShell) -> Iterator[PositronShell]: - """A fixture that configures matplotlib to use the Positron notebook backend.""" +def notebook_backend( + shell: PositronShell, monkeypatch: pytest.MonkeyPatch +) -> Iterator[PositronShell]: + """A notebook session with Positron's notebook backend active.""" + monkeypatch.setattr(shell, "session_mode", SessionMode.NOTEBOOK) with active_backend(Backend.NOTEBOOK): yield shell # `_selected_formats` is module state that intentionally survives a backend round # trip (see `test_preference_survives_backend_round_trip`), so it doesn't reset # itself on `deactivate`. Reset it here to the default so a format selected in one - # test doesn't leak into the next (e.g. other test files' `backend` fixtures expect + # test doesn't leak into the next (e.g. other test files' backend fixtures expect # `png` on activation). formats._selected_formats.clear() # noqa: SLF001 formats._selected_formats.add("png") # noqa: SLF001 +@pytest.fixture +def console_backend( + shell: PositronShell, monkeypatch: pytest.MonkeyPatch +) -> Iterator[PositronShell]: + """A console session with Positron's console backend active.""" + monkeypatch.setattr(shell, "session_mode", SessionMode.CONSOLE) + with active_backend(Backend.CONSOLE): + yield shell + + def _set_formats(shell: PositronShell, call: str) -> None: """Run `set_matplotlib_formats(...)` via the `from`-import path IPython documents.""" shell.run_cell(f"{_IMPORT_SET_FORMATS}\n{call}").raise_error() @@ -74,9 +83,9 @@ def _plot(*, meta: dict | None = None) -> RichOutput: return captured.outputs[0] -def test_svg(backend: PositronShell): +def test_svg(notebook_backend: PositronShell): """`set_matplotlib_formats('svg')` displays only svg, decoded as utf-8 text.""" - _set_formats(backend, "set_matplotlib_formats('svg')") + _set_formats(notebook_backend, "set_matplotlib_formats('svg')") output = _plot() @@ -85,7 +94,7 @@ def test_svg(backend: PositronShell): assert isinstance(output.data["image/svg+xml"], str) -def test_retina_aliases_to_png(backend: PositronShell): +def test_retina_aliases_to_png(notebook_backend: PositronShell): """ `retina` still renders as `image/png`, at Positron's own device pixel ratio. @@ -98,7 +107,7 @@ def test_retina_aliases_to_png(backend: PositronShell): Uses ratio=3 (not 2): IPython's fixed 2x doubling would coincidentally produce the same numbers at ratio=2, masking a regression back to the old behavior. """ - _set_formats(backend, "set_matplotlib_formats('retina')") + _set_formats(notebook_backend, "set_matplotlib_formats('retina')") baseline = _plot(meta={"output_pixel_ratio": 1}) baseline_w, baseline_h = _pngxy(base64.b64decode(baseline.data["image/png"])) @@ -118,9 +127,9 @@ def test_retina_aliases_to_png(backend: PositronShell): assert metadata["height"] == pytest.approx(baseline_h, abs=5) -def test_jpeg_with_pixel_ratio(backend: PositronShell): +def test_jpeg_with_pixel_ratio(notebook_backend: PositronShell): """`jpeg` output is present and its metadata reflects the requested pixel ratio.""" - _set_formats(backend, "set_matplotlib_formats('jpeg')") + _set_formats(notebook_backend, "set_matplotlib_formats('jpeg')") output = _plot(meta={"output_pixel_ratio": 2}) @@ -132,9 +141,9 @@ def test_jpeg_with_pixel_ratio(backend: PositronShell): assert metadata["height"] == pytest.approx(h / 2, abs=5) -def test_multiple_formats(backend: PositronShell): +def test_multiple_formats(notebook_backend: PositronShell): """`set_matplotlib_formats('png', 'svg')` displays both mimes in one output.""" - _set_formats(backend, "set_matplotlib_formats('png', 'svg')") + _set_formats(notebook_backend, "set_matplotlib_formats('png', 'svg')") output = _plot() @@ -142,13 +151,13 @@ def test_multiple_formats(backend: PositronShell): assert "image/svg+xml" in output.data -def test_unknown_format_raises(backend: PositronShell): +def test_unknown_format_raises(notebook_backend: PositronShell): """An unrecognized format raises `ValueError`, matching IPython's contract.""" with pytest.raises(ValueError): - _set_formats(backend, "set_matplotlib_formats('bmp')") + _set_formats(notebook_backend, "set_matplotlib_formats('bmp')") -def test_kwargs_passthrough(backend: PositronShell, monkeypatch: pytest.MonkeyPatch): +def test_kwargs_passthrough(notebook_backend: PositronShell, monkeypatch: pytest.MonkeyPatch): """Explicit kwargs (e.g. `pil_kwargs`) reach `canvas.print_figure`.""" # Asserting on the rendered jpeg bytes would be flaky (encoder-dependent), so spy on # the call instead. @@ -161,14 +170,14 @@ def spy(self, *args, **kwargs): monkeypatch.setattr(FigureCanvasAgg, "print_figure", spy) - _set_formats(backend, "set_matplotlib_formats('jpeg', pil_kwargs={'quality': 90})") + _set_formats(notebook_backend, "set_matplotlib_formats('jpeg', pil_kwargs={'quality': 90})") _plot() assert len(calls) == 1 assert calls[0]["pil_kwargs"] == {"quality": 90} -def test_from_import_binding(backend: PositronShell): +def test_from_import_binding(notebook_backend: PositronShell): """ `from matplotlib_inline.backend_inline import set_matplotlib_formats` binds ours. @@ -176,7 +185,7 @@ def test_from_import_binding(backend: PositronShell): already Positron's patch: it's installed at backend activation, before any user code runs. """ - _set_formats(backend, "set_matplotlib_formats('svg')") + _set_formats(notebook_backend, "set_matplotlib_formats('svg')") output = _plot() @@ -188,10 +197,10 @@ def test_from_import_binding(backend: PositronShell): assert "image/png" not in output.data -def test_savefig_facecolor_rcparam_is_ignored(backend: PositronShell): +def test_savefig_facecolor_rcparam_is_ignored(notebook_backend: PositronShell): """Inline output uses the figure's own facecolor, not `savefig.facecolor`, matching Jupyter.""" try: - backend.run_cell( + notebook_backend.run_cell( "import matplotlib.pyplot as plt\nplt.rcParams['savefig.facecolor'] = 'red'" ).raise_error() output = _plot() @@ -204,9 +213,9 @@ def test_savefig_facecolor_rcparam_is_ignored(backend: PositronShell): matplotlib.rcParams["savefig.facecolor"] = matplotlib.rcParamsDefault["savefig.facecolor"] -def test_preference_survives_backend_round_trip(backend: PositronShell): +def test_preference_survives_backend_round_trip(notebook_backend: PositronShell): """A format selection survives deactivating and reactivating the notebook backend.""" - _set_formats(backend, "set_matplotlib_formats('svg')") + _set_formats(notebook_backend, "set_matplotlib_formats('svg')") # Simulate switching to a non-Positron backend and back (e.g. `%matplotlib qt` then # `%matplotlib inline`). @@ -216,3 +225,20 @@ def test_preference_survives_backend_round_trip(backend: PositronShell): output = _plot() assert "image/svg+xml" in output.data + + +def test_is_noop_in_console(console_backend: PositronShell): + """ + `set_matplotlib_formats` is a silent no-op in console mode. + + The console backend routes figures to the Plots pane via `canvas.render`, which + negotiates its own format; running IPython's `select_figure_formats` here would + register stray inline `image/*` display formatters alongside it (see + `formats.install_set_matplotlib_formats_patch`). + """ + from matplotlib.figure import Figure + + _set_formats(console_backend, "set_matplotlib_formats('png')") + + for formatter in console_backend.display_formatter.formatters.values(): + assert Figure not in formatter From 78083c2a468cd46a6d14da27be09e7d1b35dc955 Mon Sep 17 00:00:00 2001 From: seem Date: Thu, 30 Jul 2026 15:45:52 +0200 Subject: [PATCH 17/26] split matplotlib_backend package into backend/compat/registry modules the two module-level globals become instance state on a PositronBackendRegistry singleton, mirroring matplotlib's own backend_registry. __init__.py is now docs-only; every import site points at the new modules. also splits test_switching.py into test_backend.py + test_enable_matplotlib.py and renames 'flavor' to 'backend' throughout. --- .../positron/matplotlib_backend/__init__.py | 272 +----------------- .../positron/matplotlib_backend/backend.py | 123 ++++++++ .../positron/matplotlib_backend/compat.py | 47 +++ .../positron/matplotlib_backend/console.py | 7 +- .../positron/matplotlib_backend/formats.py | 23 +- .../positron/matplotlib_backend/notebook.py | 8 +- .../positron/matplotlib_backend/registry.py | 174 +++++++++++ .../posit/positron/positron_ipkernel.py | 7 +- .../tests/test_matplotlib_backend/conftest.py | 16 +- .../test_matplotlib_backend/test_backend.py | 93 ++++++ .../test_matplotlib_backend/test_console.py | 5 +- ...switching.py => test_enable_matplotlib.py} | 206 +++++-------- .../test_matplotlib_backend/test_formats.py | 8 +- .../test_matplotlib_backend/test_notebook.py | 2 +- 14 files changed, 569 insertions(+), 422 deletions(-) create mode 100644 extensions/positron-python/python_files/posit/positron/matplotlib_backend/backend.py create mode 100644 extensions/positron-python/python_files/posit/positron/matplotlib_backend/compat.py create mode 100644 extensions/positron-python/python_files/posit/positron/matplotlib_backend/registry.py create mode 100644 extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_backend.py rename extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/{test_switching.py => test_enable_matplotlib.py} (71%) diff --git a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/__init__.py b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/__init__.py index 70e553c445a..6068b00166e 100644 --- a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/__init__.py +++ b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/__init__.py @@ -7,269 +7,13 @@ Positron ships one backend per session mode, each registered under its own short name (`positron-console`, `positron-notebook`) via the `matplotlib.backend` entry point in -`python_files/posit/positron_matplotlib-.dist-info`. -This module holds the names they share and the switch-time lifecycle called by -`PositronShell.enable_matplotlib`. - -Keep this module free of matplotlib imports. It's imported by the kernel, which must -work in environments without matplotlib installed; the backend modules are imported +`python_files/posit/positron_matplotlib-.dist-info`. `backend.py` holds the +names they share, `registry.py` the switch-time lifecycle called by +`PositronShell.enable_matplotlib`, and `compat.py` the shims for matplotlib and IPython +versions without backend-registry support. + +Keep `backend.py`, `compat.py` and `registry.py` free of matplotlib imports, and this +module free of code entirely. The kernel imports them, and must work in environments +without matplotlib installed; `console.py`, `notebook.py` and `formats.py` are imported lazily, and only once they're actually needed. """ - -from __future__ import annotations - -import importlib -import logging -import sys -from enum import Enum -from typing import TYPE_CHECKING, Protocol, cast - -from IPython.core.getipython import get_ipython - -from ..session_mode import SessionMode - -if TYPE_CHECKING: - from IPython.core.interactiveshell import InteractiveShell - -logger = logging.getLogger(__name__) - -_MODULE_PREFIX = "module://" - - -class Backend(Enum): - """ - One of Positron's matplotlib backends, and the names that select it. - - `short_name` is the name registered via the `matplotlib.backend` entry point; - `full_name` is matplotlib's `module://` spelling of `module_name`. Both select the - same backend: matplotlib resolves the short name through its backend registry - (matplotlib >= 3.9), while the `module://` spelling works on any version. - """ - - CONSOLE = ("positron.matplotlib_backend.console", "positron-console") - NOTEBOOK = ("positron.matplotlib_backend.notebook", "positron-notebook") - - def __init__(self, module_name: str, short_name: str) -> None: - self.module_name = module_name - self.short_name = short_name - - @classmethod - def for_session_mode(cls, session_mode: SessionMode) -> Backend: - """The backend for a session started in `session_mode`.""" - # BACKGROUND sessions get the console backend: they have no notebook to render - # figures into, and the console backend routes them to the plots pane. - return cls.NOTEBOOK if session_mode == SessionMode.NOTEBOOK else cls.CONSOLE - - @classmethod - def from_name(cls, backend: str) -> Backend | None: - """ - The Positron backend that `backend` selects, or None if `backend` isn't ours. - - Matches either spelling (short name or `module://` name). That matters at a - backend module's self-activation time (see the trailer at the bottom of - `console.py`/`notebook.py`): when matplotlib switches to a short name, - `matplotlib.get_backend()` at module-import time can still report the short - name rather than the `module://` spelling it eventually settles into. - """ - # Short names are case-insensitive in matplotlib's registry; `module://` names - # aren't, since everything after the prefix is an importable module path. - normalized = backend if backend.startswith(_MODULE_PREFIX) else backend.lower() - for candidate in cls: - if normalized in (candidate.full_name, candidate.short_name): - return candidate - return None - - @property - def full_name(self) -> str: - """Matplotlib's `module://` spelling of `module_name`; works on any matplotlib version.""" - return _MODULE_PREFIX + self.module_name - - @property - def preferred_name(self) -> str: - """ - The name to select this backend with. - - Prefers the short name (`positron-console` or `positron-notebook`): it's what - `%matplotlib` prints, and users can round-trip it into - `%matplotlib positron-console` or `matplotlib.use("positron-console")`. Falls - back to `full_name` when the entry point isn't discoverable, which is always - the case before matplotlib 3.9 since it has no backend registry. - """ - registry = _get_backend_registry() - # `list_all` also loads the entry points, which `switch_backend` relies on having - # been loaded to resolve the short name to a module. - if registry and self.short_name in registry.list_all(): - return self.short_name - return self.full_name - - def import_module(self) -> BackendModule: - """Import this Positron matplotlib backend module.""" - return cast("BackendModule", importlib.import_module(self.module_name)) - - -class BackendModule(Protocol): - """ - The hooks each Positron backend module implements. - - Called only by `configure_positron_support`, which owns activation state and - guarantees pairing and ordering: the previously active backend is uninstalled - before the next one is installed. `install` and `uninstall` are therefore not - idempotent and hold no activation state of their own. - """ - - def install(self, shell: InteractiveShell) -> None: ... - - def uninstall(self, shell: InteractiveShell) -> None: ... - - -def register_with_legacy_ipython() -> None: - """ - Make Positron's short names visible to IPython versions without registry support. - - IPython 8.24 started resolving `%matplotlib ` and building `%matplotlib -l` - from matplotlib's backend registry (matplotlib >= 3.9), which discovers Positron's - backends through their entry points. Earlier IPython reads the static - `pylabtools.backends` table for both, so add the short names there. Uses the - `module://` spelling as the value so the entries resolve on any matplotlib version. - """ - from IPython.core import pylabtools as pt - - # The registry-based lister arrived in the same release that stopped reading the - # static table, so its presence tells the two resolution schemes apart. Touching - # `pt.backends` on newer IPython would also trip its deprecation warning. - if hasattr(pt, "_list_matplotlib_backends_and_gui_loops"): - return - for backend in Backend: - pt.backends.setdefault(backend.short_name, backend.full_name) - - -def _get_backend_registry(): - """The matplotlib backend registry, or None if matplotlib < 3.9.""" - try: - from matplotlib.backends.registry import backend_registry - - return backend_registry - except ImportError: - # No backend registry before matplotlib 3.9. - return None - - -# The Positron backend whose hooks are currently installed. Owned exclusively by -# `configure_positron_support`; backend modules hold no activation state of their own. -_active_backend: Backend | None = None - - -def configure_positron_support(backend: str | Backend) -> None: - """ - Activate or deactivate Positron's matplotlib backend for a switch to `backend`. - - Mirrors `matplotlib_inline.backend_inline.configure_inline_support`, the de facto - IPython interface for switch-time lifecycle: called on every backend switch, and - self-dispatching on whether the new backend is its own. - - Sole owner of activation state: the previously active backend's hooks are - uninstalled before the new backend's are installed, and a switch to the already - active backend is a no-op. - """ - global _active_backend - - target = backend if isinstance(backend, Backend) else Backend.from_name(backend) - if target is _active_backend: - return - - shell = get_ipython() - if shell is None: - logger.warning("No IPython shell found; Positron matplotlib support not configured") - return - - if _active_backend is not None: - _active_backend.import_module().uninstall(shell) - _active_backend = None - - if target is not None: - # Enable interactive mode (i.e. redraw after every plotting command). - import matplotlib - - matplotlib.interactive(True) # noqa: FBT003 - target.import_module().install(shell) - _active_backend = target - - # The `set_matplotlib_formats` patch is flavor-agnostic (it dispatches at call - # time on the live backend), so it's owned here rather than by the flavors: - # installed while any Positron backend is active, removed when none is. - from . import formats - - if target is not None: - formats.install_set_matplotlib_formats_patch() - else: - formats.uninstall_set_matplotlib_formats_patch() - - -_MATPLOTLIB_INLINE_MODULE = "matplotlib_inline.backend_inline" -_MATPLOTLIB_INLINE_BACKENDS = ("inline", f"module://{_MATPLOTLIB_INLINE_MODULE}") - - -def _needs_inline_support(shell: InteractiveShell, backend: str) -> bool: - """ - Whether matplotlib-inline's `configure_inline_support` should be called for `backend`. - - Only when matplotlib-inline is the backend being switched to, or when it's currently - active and needs tearing down. Calling it in any other case would instantiate - `InlineBackend` into `shell.configurables`, arming a traitlets observer that re-runs - `select_figure_formats` -- popping Positron's figure formatter -- on any later - `%config InlineBackend.*` assignment. - """ - if backend in _MATPLOTLIB_INLINE_BACKENDS: - return True - - # matplotlib-inline is active if its post execute hook is registered, whether by an - # earlier switch through here or by its backend module self-activating on import. - module = sys.modules.get(_MATPLOTLIB_INLINE_MODULE) - flush_figures = getattr(module, "flush_figures", None) - return flush_figures is not None and flush_figures in shell.events.callbacks.get( - "post_execute", [] - ) - - -# The real `configure_inline_support`, captured when the switch hook is installed. -_original_configure_inline_support = None - - -def install_backend_switch_hook() -> None: - """ - Wrap `matplotlib_inline.backend_inline.configure_inline_support` with Positron's seam. - - IPython's `enable_matplotlib` imports that function from the module at call time on - every backend switch, so wrapping the module attribute makes every switch -- including - ones to foreign backends via `super()` -- run `configure_matplotlib_support`. - Idempotent. Installed lazily (not at kernel init) because importing matplotlib_inline - pulls in matplotlib, which may not be installed until the user actually plots. - Never uninstalled, matching the kernel's other third-party patches; the wrapper - preserves upstream behavior whenever Positron isn't involved. - """ - global _original_configure_inline_support - if _original_configure_inline_support is not None: - return - - import matplotlib_inline.backend_inline as backend_inline - - _original_configure_inline_support = backend_inline.configure_inline_support - backend_inline.configure_inline_support = configure_matplotlib_support - - -def configure_matplotlib_support(shell: InteractiveShell, backend: str) -> None: - """ - Configure inline figure display after a matplotlib backend switch. - - matplotlib-inline is configured only when it's involved (see - `_needs_inline_support`): calling it for any other switch would instantiate - `InlineBackend` into `shell.configurables`, arming a traitlets observer that pops - Positron's figure formatter on any later `%config InlineBackend.*` assignment. - Positron's support is configured on every switch, and goes last: tearing down - matplotlib-inline re-runs `select_figure_formats`, which pops the figure formatter - that activating Positron's backend then registers. - """ - install_backend_switch_hook() - if _needs_inline_support(shell, backend): - _original_configure_inline_support(shell, backend) - configure_positron_support(backend) diff --git a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/backend.py b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/backend.py new file mode 100644 index 00000000000..a633e544b71 --- /dev/null +++ b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/backend.py @@ -0,0 +1,123 @@ +# +# Copyright (C) 2026 Posit Software, PBC. All rights reserved. +# Licensed under the Elastic License 2.0. See LICENSE.txt for license information. +# +""" +The names of Positron's matplotlib backends, and the interface their modules implement. + +Keep this module free of matplotlib imports; see the package docstring. +""" + +from __future__ import annotations + +import importlib +from enum import Enum +from typing import TYPE_CHECKING, Protocol, cast + +from ..session_mode import SessionMode +from .compat import get_backend_registry + +if TYPE_CHECKING: + from IPython.core.interactiveshell import InteractiveShell + +_MODULE_PREFIX = "module://" + + +class Backend(Enum): + """ + One of Positron's matplotlib backends. + + Both names select the same backend: matplotlib resolves the short name through its + backend registry (matplotlib >= 3.9), while the `module://` spelling works on any + version. + + Attributes: + module_name: The importable path of the backend's module. + short_name: The name the backend is registered under via the `matplotlib.backend` + entry point. + """ + + CONSOLE = ("positron.matplotlib_backend.console", "positron-console") + NOTEBOOK = ("positron.matplotlib_backend.notebook", "positron-notebook") + + def __init__(self, module_name: str, short_name: str) -> None: + self.module_name = module_name + self.short_name = short_name + + @classmethod + def for_session_mode(cls, session_mode: SessionMode) -> Backend: + """The backend for a session started in `session_mode`.""" + # BACKGROUND sessions get the console backend: they have no notebook to render + # figures into, and the console backend routes them to the plots pane. + return cls.NOTEBOOK if session_mode == SessionMode.NOTEBOOK else cls.CONSOLE + + @classmethod + def from_name(cls, backend: str) -> Backend | None: + """ + The Positron backend that `backend` selects, or None if `backend` isn't ours. + + Matches either spelling (short name or `module://` name). That matters at a + backend module's self-activation time (see the trailer at the bottom of + `console.py`/`notebook.py`): when matplotlib switches to a short name, + `matplotlib.get_backend()` at module-import time can still report the short + name rather than the `module://` spelling it eventually settles into. + """ + # Short names are case-insensitive in matplotlib's registry; `module://` names + # aren't, since everything after the prefix is an importable module path. + normalized = backend if backend.startswith(_MODULE_PREFIX) else backend.lower() + for candidate in cls: + if normalized in (candidate.full_name, candidate.short_name): + return candidate + return None + + @property + def full_name(self) -> str: + """Matplotlib's `module://` spelling of `module_name`; works on any matplotlib version.""" + return _MODULE_PREFIX + self.module_name + + @property + def preferred_name(self) -> str: + """ + The name to select this backend with. + + Prefers the short name (`positron-console` or `positron-notebook`): it's what + `%matplotlib` prints, and users can round-trip it into + `%matplotlib positron-console` or `matplotlib.use("positron-console")`. Falls + back to `full_name` when the entry point isn't discoverable, which is always + the case before matplotlib 3.9 since it has no backend registry. + """ + registry = get_backend_registry() + # `list_all` also loads the entry points, which `switch_backend` relies on having + # been loaded to resolve the short name to a module. Loading them doesn't import + # any backend module: matplotlib only reads each entry point's name and value + # strings (see `backends/registry.py::_read_entry_points`), so this stays cheap + # and can't trigger a premature self-activation. + if registry and self.short_name in registry.list_all(): + return self.short_name + return self.full_name + + def import_module(self) -> BackendModule: + """Import this Positron matplotlib backend module.""" + return cast("BackendModule", importlib.import_module(self.module_name)) + + +class BackendModule(Protocol): + """Interface for Positron matplotlib backend modules.""" + + def install(self, shell: InteractiveShell) -> None: + """ + Install this backend's shell hooks. + + Never called twice without an intervening `uninstall`, so implementations hold + no activation state of their own and needn't be idempotent. + """ + ... + + def uninstall(self, shell: InteractiveShell) -> None: + """ + Remove the shell hooks installed by `install`. + + Only ever called after a matching `install`, and before another backend's + `install`. + """ + ... diff --git a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/compat.py b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/compat.py new file mode 100644 index 00000000000..785fef42f11 --- /dev/null +++ b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/compat.py @@ -0,0 +1,47 @@ +# +# Copyright (C) 2026 Posit Software, PBC. All rights reserved. +# Licensed under the Elastic License 2.0. See LICENSE.txt for license information. +# +""" +Shims for the matplotlib and IPython versions that predate backend-registry support. + +Keep this module free of matplotlib imports; see the package docstring. +""" + +from __future__ import annotations + + +def get_backend_registry(): + """The matplotlib backend registry, or None if matplotlib < 3.9.""" + try: + from matplotlib.backends.registry import backend_registry + + return backend_registry + except ImportError: + # No backend registry before matplotlib 3.9. + return None + + +def register_with_legacy_ipython() -> None: + """ + Make Positron's short names visible to IPython versions without registry support. + + IPython 8.24 started resolving `%matplotlib ` and building `%matplotlib -l` + from matplotlib's backend registry (matplotlib >= 3.9), which discovers Positron's + backends through their entry points. Earlier IPython reads the static + `pylabtools.backends` table for both, so add the short names there. Uses the + `module://` spelling as the value so the entries resolve on any matplotlib version. + """ + from IPython.core import pylabtools as pt + + # Imported here rather than at module scope: `backend.py` imports this module for + # `get_backend_registry`, so a module-level import would be circular. + from .backend import Backend + + # The registry-based lister arrived in the same release that stopped reading the + # static table, so its presence tells the two resolution schemes apart. Touching + # `pt.backends` on newer IPython would also trip its deprecation warning. + if hasattr(pt, "_list_matplotlib_backends_and_gui_loops"): + return + for backend in Backend: + pt.backends.setdefault(backend.short_name, backend.full_name) diff --git a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/console.py b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/console.py index 25ece5ace63..57773db740b 100644 --- a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/console.py +++ b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/console.py @@ -29,7 +29,8 @@ from matplotlib.backends.backend_agg import FigureCanvasAgg from ..execute_request import PositronExecuteRequest -from . import Backend, configure_positron_support +from .backend import Backend +from .registry import registry if TYPE_CHECKING: from IPython.core.interactiveshell import InteractiveShell @@ -426,10 +427,10 @@ def uninstall(shell: InteractiveShell) -> None: # If we are the selected backend, activate through the registry, which also tears -# down the other flavor if it was active. This runs when matplotlib imports the +# down the other backend if it was active. This runs when matplotlib imports the # module to switch to it. See the note at the top of the file. `Backend.from_name` # matches both spellings, which matters here: matplotlib can still report our short # name via `get_backend()` at this point, before it settles into the `module://` # spelling. if Backend.from_name(matplotlib.get_backend()) is Backend.CONSOLE: - configure_positron_support(Backend.CONSOLE) + registry.activate(Backend.CONSOLE) diff --git a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/formats.py b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/formats.py index 8be27062774..f05c111cbc9 100644 --- a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/formats.py +++ b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/formats.py @@ -8,12 +8,12 @@ That function (and its sibling `select_figure_formats`) operate entirely inside matplotlib-inline/IPython, with no Positron code on the stack, so the only way to intercept them is to patch `set_matplotlib_formats` itself. The patch is installed -by `configure_positron_support` whenever a Positron backend (console or notebook) is -active, and removed once none is -- see `install_set_matplotlib_formats_patch` for why -a shared, call-time dispatch is needed instead of one patch per flavor. +by `PositronBackendRegistry.activate` whenever a Positron backend (console or notebook) +is active, and removed once none is -- see `install_set_matplotlib_formats_patch` for +why a shared, call-time dispatch is needed instead of one patch per backend. -NOTE: only ever imported from `configure_positron_support`, by which point matplotlib -is guaranteed to be importable. +NOTE: only ever imported from `PositronBackendRegistry.activate`, by which point +matplotlib is guaranteed to be importable. """ from __future__ import annotations @@ -33,7 +33,7 @@ from IPython.core.pylabtools import print_figure from matplotlib.figure import Figure -from . import Backend +from .backend import Backend if TYPE_CHECKING: from IPython.core.interactiveshell import InteractiveShell @@ -96,6 +96,8 @@ def _display_figure( if format_ == "svg": # `print_figure` returns `str` only for svg; narrowed by the `format_` check above. return cast("str", data), {} + # Same encoding `print_figure(base64=True)` would have applied, done here instead so + # the raw bytes stay available for `_pngxy`/`_jpegxy` below. decoded = b2a_base64(cast("bytes", data), newline=False).decode("ascii") metadata = {} @@ -173,9 +175,10 @@ def install_set_matplotlib_formats_patch() -> None: """ Patch `matplotlib_inline.backend_inline.set_matplotlib_formats`. Safe to call repeatedly. - Installed by `configure_positron_support` whenever a Positron backend is active, - and dispatches at call time on `matplotlib.get_backend()`, so it stays correct - however the active flavor changes between install and call time. Dispatching on + Installed by `PositronBackendRegistry.activate` whenever a Positron backend is + active, and dispatches at call time on `matplotlib.get_backend()`, so it stays + correct however the active backend changes between install and call time. + Dispatching on the live backend string rather than the registry's state is deliberate: a user can switch backends with a bare `matplotlib.use(...)` that never goes through the registry. @@ -198,7 +201,7 @@ def set_matplotlib_formats(*formats, **kwargs) -> None: # The Plots pane negotiates its own format via `canvas.render(format_=...)`; # running `select_figure_formats` here would emit stray inline outputs # alongside it. Silent no-op, matching the deliberate - # `%config InlineBackend.*` no-op precedent (see test_switching.py). + # `%config InlineBackend.*` no-op precedent (see test_enable_matplotlib.py). logger.debug("set_matplotlib_formats is a no-op in Positron's console backend") else: # A non-Positron backend is active (e.g. the user called diff --git a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/notebook.py b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/notebook.py index 30d17a75d0d..195cdbcc502 100644 --- a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/notebook.py +++ b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/notebook.py @@ -32,7 +32,9 @@ from matplotlib.figure import Figure from ..execute_request import current_execute_request -from . import Backend, configure_positron_support, formats +from . import formats +from .backend import Backend +from .registry import registry if TYPE_CHECKING: from IPython.core.interactiveshell import InteractiveShell @@ -129,10 +131,10 @@ def uninstall(shell: InteractiveShell) -> None: # If we are the selected backend, activate through the registry, which also tears -# down the other flavor if it was active. This runs when matplotlib imports the +# down the other backend if it was active. This runs when matplotlib imports the # module to switch to it. See the note at the top of the file. `Backend.from_name` # matches both spellings, which matters here: matplotlib can still report our short # name via `get_backend()` at this point, before it settles into the `module://` # spelling. if Backend.from_name(matplotlib.get_backend()) is Backend.NOTEBOOK: - configure_positron_support(Backend.NOTEBOOK) + registry.activate(Backend.NOTEBOOK) diff --git a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/registry.py b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/registry.py new file mode 100644 index 00000000000..24b29376ea7 --- /dev/null +++ b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/registry.py @@ -0,0 +1,174 @@ +# +# Copyright (C) 2026 Posit Software, PBC. All rights reserved. +# Licensed under the Elastic License 2.0. See LICENSE.txt for license information. +# +""" +Which Positron matplotlib backend is active, and the switch-time lifecycle around it. + +Keep this module free of matplotlib imports; see the package docstring. +""" + +from __future__ import annotations + +import logging +import sys +from typing import TYPE_CHECKING, Callable + +from IPython.core.getipython import get_ipython + +from .backend import Backend + +if TYPE_CHECKING: + from IPython.core.interactiveshell import InteractiveShell + + ConfigureInlineSupport = Callable[[InteractiveShell, str], None] + +logger = logging.getLogger(__name__) + +_MATPLOTLIB_INLINE_MODULE = "matplotlib_inline.backend_inline" +_MATPLOTLIB_INLINE_BACKENDS = ("inline", f"module://{_MATPLOTLIB_INLINE_MODULE}") + + +def _needs_inline_support(shell: InteractiveShell, backend: str) -> bool: + """ + Whether matplotlib-inline's `configure_inline_support` should be called for `backend`. + + Only when matplotlib-inline is the backend being switched to, or when it's currently + active and needs tearing down. Calling it in any other case would instantiate + `InlineBackend` into `shell.configurables`, arming a traitlets observer that re-runs + `select_figure_formats` -- popping Positron's figure formatter -- on any later + `%config InlineBackend.*` assignment. + """ + if backend in _MATPLOTLIB_INLINE_BACKENDS: + return True + + # matplotlib-inline is active if its post execute hook is registered, whether by an + # earlier switch through here or by its backend module self-activating on import. + module = sys.modules.get(_MATPLOTLIB_INLINE_MODULE) + flush_figures = getattr(module, "flush_figures", None) + return flush_figures is not None and flush_figures in shell.events.callbacks.get( + "post_execute", [] + ) + + +class PositronBackendRegistry: + """ + Sole owner of which Positron matplotlib backend is active. + + Instantiated once as the module-level `registry`, mirroring matplotlib's own + `backend_registry`: a process has a single active matplotlib backend, so it has a + single activation state. Backend modules hold no activation state of their own. + """ + + def __init__(self) -> None: + # The Positron backend whose hooks are currently installed. + self._active_backend: Backend | None = None + + # The real `configure_inline_support`, captured when the switch hook is installed. + self._original_configure_inline_support: ConfigureInlineSupport | None = None + + def activate(self, backend: str | Backend) -> None: + """ + Activate the Positron backend that `backend` selects, deactivating the previous one. + + Called on every matplotlib backend switch and self-dispatching on whether the + new backend is one of ours, so a `backend` that isn't Positron's deactivates + Positron's support entirely. Switching to the already active backend is a no-op. + """ + target = backend if isinstance(backend, Backend) else Backend.from_name(backend) + if target is self._active_backend: + return + + shell = get_ipython() + if shell is None: + logger.warning("No IPython shell found; Positron matplotlib support not configured") + return + + if self._active_backend is not None: + # `_active_backend` is only ever set after we imported that module and + # installed its hooks, so this `import_module()` is a cached `sys.modules` + # lookup rather than a fresh import. + self._active_backend.import_module().uninstall(shell) + self._active_backend = None + + if target is not None: + # Enable interactive mode (i.e. redraw after every plotting command). + import matplotlib + + matplotlib.interactive(True) # noqa: FBT003 + target.import_module().install(shell) + self._active_backend = target + + # The `set_matplotlib_formats` patch is installed once for any Positron backend + # rather than per backend (it dispatches on the live backend at call time), so + # it's owned here: installed while any Positron backend is active, removed when + # none is. + from . import formats + + if target is not None: + formats.install_set_matplotlib_formats_patch() + else: + formats.uninstall_set_matplotlib_formats_patch() + + def install_switch_hook(self) -> ConfigureInlineSupport: + """ + Wrap `matplotlib_inline.backend_inline.configure_inline_support` with Positron's seam. + + IPython's `enable_matplotlib` imports that function from the module at call time + on every backend switch, so wrapping the module attribute makes every switch -- + including ones to foreign backends via `super()` -- run + `configure_matplotlib_support`. It also covers the switch that never reaches + `enable_matplotlib` at all: `matplotlib_inline.backend_inline` self-activates at + import (`_enable_matplotlib_integration`) and calls `configure_inline_support` + directly, so a bare `matplotlib.use("inline")` only goes through Positron here. + + Idempotent, and returns the real `configure_inline_support` the seam wraps. + Installed lazily (not at kernel init) because importing matplotlib_inline pulls + in matplotlib, which may not be installed until the user actually plots. Never + uninstalled, matching the kernel's other third-party patches; the wrapper + preserves upstream behavior whenever Positron isn't involved. + """ + if self._original_configure_inline_support is None: + import matplotlib_inline.backend_inline as backend_inline + + self._original_configure_inline_support = backend_inline.configure_inline_support + backend_inline.configure_inline_support = configure_matplotlib_support + return self._original_configure_inline_support + + def configure_switch(self, shell: InteractiveShell, backend: str) -> None: + """ + Configure inline figure display after a matplotlib backend switch. + + matplotlib-inline is configured only when it's involved (see + `_needs_inline_support`): calling it for any other switch would instantiate + `InlineBackend` into `shell.configurables`, arming a traitlets observer that pops + Positron's figure formatter on any later `%config InlineBackend.*` assignment. + Positron's support is configured on every switch, and goes last: tearing down + matplotlib-inline re-runs `select_figure_formats`, which pops the figure formatter + that activating Positron's backend then registers. + """ + original_configure_inline_support = self.install_switch_hook() + if _needs_inline_support(shell, backend): + original_configure_inline_support(shell, backend) + self.activate(backend) + + +registry = PositronBackendRegistry() + + +def install_backend_switch_hook() -> None: + """Install `registry`'s switch hook. See `PositronBackendRegistry.install_switch_hook`.""" + registry.install_switch_hook() + + +def configure_matplotlib_support(shell: InteractiveShell, backend: str) -> None: + """ + Configure Positron's matplotlib support after a backend switch. + + The name and signature mirror `matplotlib_inline.backend_inline.configure_inline_support`, + the de facto IPython interface for switch-time lifecycle, since this replaces it. A + module-level function rather than a bound method so the object installed by + `PositronBackendRegistry.install_switch_hook` is stable and comparable by identity. + See `PositronBackendRegistry.configure_switch`. + """ + registry.configure_switch(shell, backend) diff --git a/extensions/positron-python/python_files/posit/positron/positron_ipkernel.py b/extensions/positron-python/python_files/posit/positron/positron_ipkernel.py index 2f91355e200..c3650d2eaeb 100644 --- a/extensions/positron-python/python_files/posit/positron/positron_ipkernel.py +++ b/extensions/positron-python/python_files/posit/positron/positron_ipkernel.py @@ -40,7 +40,8 @@ from .formatters.display_formatter import PositronDisplayFormatter from .help import HelpService, _distribution_to_modules, help # noqa: A004 from .lsp import LSPService -from .matplotlib_backend import Backend, register_with_legacy_ipython +from .matplotlib_backend.backend import Backend +from .matplotlib_backend.compat import register_with_legacy_ipython from .patch.bokeh import handle_bokeh_output, patch_bokeh_no_access from .patch.haystack import patch_haystack_is_in_jupyter from .patch.holoviews import set_holoviews_extension @@ -351,13 +352,13 @@ def enable_matplotlib(self, gui=None): Overrides IPython so that bare `%matplotlib`, `%matplotlib inline`, and Positron's own backend names activate the Positron backend that suits the - session mode (or the explicitly named flavor). Every other backend is + session mode (or the explicitly named backend). Every other backend is delegated to IPython, whose switch path runs through the hook installed by `install_backend_switch_hook`, so switching away tears Positron's hooks down. """ from IPython.core import pylabtools as pt - from .matplotlib_backend import ( + from .matplotlib_backend.registry import ( configure_matplotlib_support, install_backend_switch_hook, ) diff --git a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/conftest.py b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/conftest.py index af1a198c20b..937c6621aaa 100644 --- a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/conftest.py +++ b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/conftest.py @@ -10,20 +10,26 @@ import matplotlib -from positron.matplotlib_backend import Backend, configure_positron_support +from positron.matplotlib_backend.registry import registry if TYPE_CHECKING: - from typing import Iterator + from typing import Generator + + from positron.matplotlib_backend.backend import Backend @contextlib.contextmanager -def active_backend(backend: Backend) -> Iterator[None]: +def active_backend(backend: Backend) -> Generator[None, None, None]: """Activate a Positron matplotlib backend, restoring the previous backend on exit.""" prev = matplotlib.get_backend() matplotlib.use(backend.full_name) - configure_positron_support(backend) + # matplotlib has no switch-callback API (`pyplot.switch_backend` notifies nothing), so + # in production the backend module's import-time trailer is what activates Positron's + # support. `matplotlib.use` here often finds the module already imported, so that + # trailer doesn't re-run; activate manually to simulate it. + registry.activate(backend) try: yield finally: - configure_positron_support(prev) + registry.activate(prev) matplotlib.use(prev) diff --git a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_backend.py b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_backend.py new file mode 100644 index 00000000000..cb633cc42d6 --- /dev/null +++ b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_backend.py @@ -0,0 +1,93 @@ +# +# Copyright (C) 2026 Posit Software, PBC. All rights reserved. +# Licensed under the Elastic License 2.0. See LICENSE.txt for license information. +# +""" +Tests for `Backend`, the names of Positron's matplotlib backends. + +Pure name resolution, no shell or matplotlib backend switching involved: which session +mode maps to which backend, which spellings `from_name` accepts, and which spelling +`preferred_name` hands to matplotlib. +""" + +from __future__ import annotations + +import pytest + +from positron.matplotlib_backend import backend as backend_module +from positron.matplotlib_backend.backend import Backend +from positron.session_mode import SessionMode + +# A backend that isn't ours. Headless, matching the rest of the suite. +OTHER_BACKEND_NAME = "agg" + + +@pytest.fixture( + params=[ + pytest.param(Backend.CONSOLE, id="console"), + pytest.param(Backend.NOTEBOOK, id="notebook"), + ] +) +def backend(request: pytest.FixtureRequest) -> Backend: + """Each of Positron's matplotlib backends.""" + return request.param + + +def _other_backend(backend: Backend) -> Backend: + """The Positron backend that isn't `backend`.""" + return Backend.NOTEBOOK if backend is Backend.CONSOLE else Backend.CONSOLE + + +@pytest.mark.parametrize( + ("session_mode", "expected"), + [ + (SessionMode.CONSOLE, Backend.CONSOLE), + (SessionMode.NOTEBOOK, Backend.NOTEBOOK), + # BACKGROUND sessions have no notebook to render into, so they get the console + # backend, which routes figures to the plots pane. + (SessionMode.BACKGROUND, Backend.CONSOLE), + ], +) +def test_for_session_mode(session_mode: SessionMode, expected: Backend): + """Each session mode maps to the Positron backend that suits it.""" + assert Backend.for_session_mode(session_mode) is expected + + +def test_from_name_recognizes_own_names(backend: Backend): + """`Backend.from_name` accepts a backend's own short name and its `module://` name.""" + assert Backend.from_name(backend.short_name) is backend + assert Backend.from_name(backend.full_name) is backend + + +def test_from_name_rejects_other_names(backend: Backend): + """`Backend.from_name` resolves neither the other backend's names nor a foreign backend to `backend`.""" + other = _other_backend(backend) + + assert Backend.from_name(other.short_name) is not backend + assert Backend.from_name(other.full_name) is not backend + assert Backend.from_name(OTHER_BACKEND_NAME) is None + + +def test_from_name_short_name_case_insensitive(backend: Backend): + """Short names match case-insensitively, like matplotlib's backend registry.""" + assert Backend.from_name(backend.short_name.upper()) is backend + + +def test_from_name_full_name_case_sensitive(backend: Backend): + """The path after `module://` is an importable module path, so case matters.""" + assert Backend.from_name(f"module://{backend.module_name.upper()}") is None + + +def test_preferred_name_prefers_short_name(backend: Backend): + """The backend's short name is preferred when matplotlib's backend registry knows it.""" + assert backend.preferred_name == backend.short_name + + +def test_preferred_name_falls_back_to_module_name( + backend: Backend, monkeypatch: pytest.MonkeyPatch +): + """Falls back to the `module://` name before matplotlib 3.9, which has no registry.""" + # Patched on `backend.py`, which imported the shim by name, not on `compat.py`. + monkeypatch.setattr(backend_module, "get_backend_registry", lambda: None) + + assert backend.preferred_name == backend.full_name diff --git a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_console.py b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_console.py index 191eb99a6df..29d4ee8710a 100644 --- a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_console.py +++ b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_console.py @@ -12,7 +12,8 @@ import pytest from PIL import Image -from positron.matplotlib_backend import Backend, configure_positron_support +from positron.matplotlib_backend.backend import Backend +from positron.matplotlib_backend.registry import registry from positron.plot_comm import PlotRenderFormat, PlotSize, PlotUnit from positron.plots import PlotsService from positron.positron_ipkernel import PositronIPyKernel, _CommTarget @@ -673,7 +674,7 @@ def test_mpl_deactivate_restores_gca() -> None: installed_gca = plt.gca original_gca = console._original_gca # noqa: SLF001 - configure_positron_support("agg") + registry.activate("agg") assert console._installed_gca is None # noqa: SLF001 assert plt.gca is original_gca diff --git a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_switching.py b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_enable_matplotlib.py similarity index 71% rename from extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_switching.py rename to extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_enable_matplotlib.py index 5953a6d0915..7f84d9b0cdd 100644 --- a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_switching.py +++ b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_enable_matplotlib.py @@ -7,10 +7,10 @@ Exercises `PositronShell.enable_matplotlib`: bare `%matplotlib` and `%matplotlib inline` keep the session's own Positron backend active, an explicit `positron-console` or -`positron-notebook` activates that flavor outright (even across session modes), another +`positron-notebook` activates that backend outright (even across session modes), another backend tears Positron's hooks down, and switching back restores them. `agg` is the -"other" backend throughout so the tests run headless. Also covers `Backend`'s -name-resolution helper (`from_name`) behind all of this. +"other" backend throughout so the tests run headless. `Backend`'s name resolution behind +all of this is covered by `test_backend.py`. """ from __future__ import annotations @@ -26,12 +26,12 @@ from IPython.utils.capture import capture_output from matplotlib.figure import Figure -from positron import matplotlib_backend -from positron.matplotlib_backend import ( - Backend, - console, - formats, - notebook, +from positron.matplotlib_backend import compat, console, formats, notebook +from positron.matplotlib_backend.backend import Backend +from positron.matplotlib_backend.registry import ( + configure_matplotlib_support, + install_backend_switch_hook, + registry, ) from positron.session_mode import SessionMode @@ -50,13 +50,13 @@ OTHER_BACKEND_NAME = "agg" -class Flavor(NamedTuple): +class BackendCase(NamedTuple): """One of Positron's matplotlib backends, and how to detect that it's installed.""" session_mode: SessionMode module: ModuleType backend: Backend - # Whether the flavor's non-hook integration is installed: the console backend's + # Whether the backend's non-hook integration is installed: the console backend's # `plt.gca` redirect, or the notebook backend's figure formatter. integration_installed: Callable[[PositronShell], bool] @@ -77,13 +77,13 @@ def _notebook_integration_installed(shell: PositronShell) -> bool: return False -CONSOLE = Flavor( +CONSOLE = BackendCase( SessionMode.CONSOLE, console, Backend.CONSOLE, _console_integration_installed, ) -NOTEBOOK = Flavor( +NOTEBOOK = BackendCase( SessionMode.NOTEBOOK, notebook, Backend.NOTEBOOK, @@ -91,53 +91,55 @@ def _notebook_integration_installed(shell: PositronShell) -> bool: ) -def _other_flavor(flavor: Flavor) -> Flavor: - """The flavor that isn't `flavor`.""" - return NOTEBOOK if flavor is CONSOLE else CONSOLE +def _other_case(case: BackendCase) -> BackendCase: + """The Positron backend that isn't `case`.""" + return NOTEBOOK if case is CONSOLE else CONSOLE @pytest.fixture(params=[pytest.param(CONSOLE, id="console"), pytest.param(NOTEBOOK, id="notebook")]) -def flavor(request: pytest.FixtureRequest) -> Flavor: +def backend_case(request: pytest.FixtureRequest) -> BackendCase: """Each of Positron's matplotlib backends, one per session mode.""" return request.param @pytest.fixture def positron_backend( - flavor: Flavor, shell: PositronShell, monkeypatch: pytest.MonkeyPatch -) -> Iterator[Flavor]: - """A session with the parametrized flavor's Positron backend active.""" - yield from _session_with_backend(flavor, shell, monkeypatch) + backend_case: BackendCase, shell: PositronShell, monkeypatch: pytest.MonkeyPatch +) -> Iterator[BackendCase]: + """A session with the parametrized Positron backend active.""" + yield from _session_with_backend(backend_case, shell, monkeypatch) @pytest.fixture -def notebook_backend(shell: PositronShell, monkeypatch: pytest.MonkeyPatch) -> Iterator[Flavor]: +def notebook_backend( + shell: PositronShell, monkeypatch: pytest.MonkeyPatch +) -> Iterator[BackendCase]: """A notebook session with Positron's notebook backend active.""" yield from _session_with_backend(NOTEBOOK, shell, monkeypatch) @pytest.fixture -def console_backend(shell: PositronShell, monkeypatch: pytest.MonkeyPatch) -> Iterator[Flavor]: +def console_backend(shell: PositronShell, monkeypatch: pytest.MonkeyPatch) -> Iterator[BackendCase]: """A console session with Positron's console backend active.""" yield from _session_with_backend(CONSOLE, shell, monkeypatch) def _session_with_backend( - flavor: Flavor, shell: PositronShell, monkeypatch: pytest.MonkeyPatch -) -> Iterator[Flavor]: + case: BackendCase, shell: PositronShell, monkeypatch: pytest.MonkeyPatch +) -> Iterator[BackendCase]: """ - Fixture body: a session with `flavor`'s backend active, as it is at kernel startup. + Fixture body: a session with `case`'s backend active, as it is at kernel startup. - Restores matplotlib's backend, both flavors' hooks and any matplotlib-inline state + Restores matplotlib's backend, both backends' hooks and any matplotlib-inline state afterwards, since matplotlib and the shell are process-wide singletons. """ - monkeypatch.setattr(shell, "session_mode", flavor.session_mode) + monkeypatch.setattr(shell, "session_mode", case.session_mode) # Entering a gui event loop needs a running kernel application, which tests don't have. monkeypatch.setattr(shell, "enable_gui", lambda gui=None: None) # noqa: ARG005 - with active_backend(flavor.backend): - yield flavor + with active_backend(case.backend): + yield case _reset_matplotlib_inline(shell) shell.kernel.plots_service.shutdown() plt.close("all") @@ -180,34 +182,34 @@ def _hook_count(shell: PositronShell, module_name: str) -> int: ) -def _state(shell: PositronShell, flavor: Flavor) -> dict: +def _state(shell: PositronShell, case: BackendCase) -> dict: """A snapshot of the backend state that a `%matplotlib` switch should manage.""" return { "backend": matplotlib.get_backend(), - "positron_hooks": _hook_count(shell, flavor.module.__name__), - "positron_integration": flavor.integration_installed(shell), + "positron_hooks": _hook_count(shell, case.module.__name__), + "positron_integration": case.integration_installed(shell), "inline_hooks": _hook_count(shell, INLINE_MODULE_NAME), } -def _positron_active(flavor: Flavor) -> dict: - """The backend state after activating `flavor`'s Positron backend, inline uninvolved.""" +def _positron_active(case: BackendCase) -> dict: + """The backend state after activating `case`'s Positron backend, inline uninvolved.""" return { - "backend": flavor.backend.short_name, + "backend": case.backend.short_name, "positron_hooks": 1, "positron_integration": True, "inline_hooks": 0, } -def test_inline_keeps_positron_backend(shell: PositronShell, positron_backend: Flavor): +def test_inline_keeps_positron_backend(shell: PositronShell, positron_backend: BackendCase): """`%matplotlib inline`, the boilerplate first cell of countless notebooks, is ours.""" shell.run_cell("%matplotlib inline").raise_error() assert _state(shell, positron_backend) == _positron_active(positron_backend) -def test_inline_is_idempotent(shell: PositronShell, positron_backend: Flavor): +def test_inline_is_idempotent(shell: PositronShell, positron_backend: BackendCase): """Repeated `%matplotlib inline` registers the post execute hook exactly once.""" shell.run_cell("%matplotlib inline").raise_error() shell.run_cell("%matplotlib inline").raise_error() @@ -215,7 +217,7 @@ def test_inline_is_idempotent(shell: PositronShell, positron_backend: Flavor): assert _state(shell, positron_backend) == _positron_active(positron_backend) -def test_inline_still_creates_positron_figures(shell: PositronShell, positron_backend: Flavor): +def test_inline_still_creates_positron_figures(shell: PositronShell, positron_backend: BackendCase): """Figures created after `%matplotlib inline` still go through Positron's backend.""" shell.run_cell("%matplotlib inline").raise_error() # Record the canvas type in the cell that creates the figure: the notebook backend @@ -227,7 +229,7 @@ def test_inline_still_creates_positron_figures(shell: PositronShell, positron_ba assert shell.user_ns["canvas_type"] is positron_backend.module.FigureCanvas -def test_inline_keeps_figure_sizing(shell: PositronShell, notebook_backend: Flavor): # noqa: ARG001 +def test_inline_keeps_figure_sizing(shell: PositronShell, notebook_backend: BackendCase): # noqa: ARG001 """`#| fig-width` and `#| fig-height` still size figures after `%matplotlib inline`.""" shell.run_cell("%matplotlib inline").raise_error() run_with_metadata( @@ -238,7 +240,7 @@ def test_inline_keeps_figure_sizing(shell: PositronShell, notebook_backend: Flav assert shell.user_ns["fig"].get_size_inches().tolist() == [8.0, 4.0] -def test_bare_magic_selects_positron(shell: PositronShell, positron_backend: Flavor): +def test_bare_magic_selects_positron(shell: PositronShell, positron_backend: BackendCase): """Bare `%matplotlib` selects Positron's backend, and prints a name users can reuse.""" with capture_output() as captured: shell.run_cell("%matplotlib").raise_error() @@ -249,29 +251,31 @@ def test_bare_magic_selects_positron(shell: PositronShell, positron_backend: Fla assert _state(shell, positron_backend) == _positron_active(positron_backend) -def test_explicit_short_name_selects_own_flavor(shell: PositronShell, positron_backend: Flavor): - """`%matplotlib ` selects that flavor.""" +def test_explicit_short_name_selects_own_backend( + shell: PositronShell, positron_backend: BackendCase +): + """`%matplotlib ` selects that backend.""" shell.run_cell(f"%matplotlib {positron_backend.backend.short_name}").raise_error() assert _state(shell, positron_backend) == _positron_active(positron_backend) -def test_explicit_short_name_selects_other_flavor_across_modes( - shell: PositronShell, positron_backend: Flavor +def test_explicit_short_name_selects_other_backend_across_modes( + shell: PositronShell, positron_backend: BackendCase ): """ - `%matplotlib ` activates that flavor outright. + `%matplotlib ` activates that backend outright. - Cross-mode selection is intentional now that each flavor has its own short name: + Cross-mode selection is intentional now that each backend has its own short name: unlike the session-relative `positron` name it replaces, an explicit short name always wins over the session's own mode. """ - other = _other_flavor(positron_backend) + other = _other_case(positron_backend) shell.run_cell(f"%matplotlib {other.backend.short_name}").raise_error() assert _state(shell, other) == _positron_active(other) - # The flavor that was active coming in has been torn down. + # The backend that was active coming in has been torn down. assert _state(shell, positron_backend) == { "backend": other.backend.short_name, "positron_hooks": 0, @@ -280,9 +284,9 @@ def test_explicit_short_name_selects_other_flavor_across_modes( } -def test_console_flavor_in_notebook_session_routes_to_plots_pane( +def test_console_backend_in_notebook_session_routes_to_plots_pane( shell: PositronShell, - notebook_backend: Flavor, # noqa: ARG001 + notebook_backend: BackendCase, # noqa: ARG001 ): """ In a notebook session, `%matplotlib positron-console` routes figures to the Plots pane. @@ -300,9 +304,9 @@ def test_console_flavor_in_notebook_session_routes_to_plots_pane( assert not captured.outputs -def test_notebook_flavor_in_console_session_displays_inline( +def test_notebook_backend_in_console_session_displays_inline( shell: PositronShell, - console_backend: Flavor, # noqa: ARG001 + console_backend: BackendCase, # noqa: ARG001 ): """In a console session, `%matplotlib positron-notebook` displays figures inline.""" shell.run_cell(f"%matplotlib {Backend.NOTEBOOK.short_name}").raise_error() @@ -328,7 +332,7 @@ def test_positron_short_names_are_listed_backends(shell: PositronShell): def test_explicit_short_name_without_registry_resolution( - shell: PositronShell, positron_backend: Flavor, monkeypatch: pytest.MonkeyPatch + shell: PositronShell, positron_backend: BackendCase, monkeypatch: pytest.MonkeyPatch ): """ Explicit short names select the backend even when IPython can't resolve them. @@ -359,7 +363,7 @@ def test_register_with_legacy_ipython_adds_short_names(monkeypatch: pytest.Monke legacy_table = {"inline": INLINE_BACKEND_NAME} monkeypatch.setattr(pt, "backends", legacy_table) - matplotlib_backend.register_with_legacy_ipython() + compat.register_with_legacy_ipython() assert legacy_table == { "inline": INLINE_BACKEND_NAME, @@ -368,7 +372,7 @@ def test_register_with_legacy_ipython_adds_short_names(monkeypatch: pytest.Monke } -def test_other_backend_deactivates(shell: PositronShell, positron_backend: Flavor): +def test_other_backend_deactivates(shell: PositronShell, positron_backend: BackendCase): """Switching to another backend removes Positron's hooks instead of leaving them.""" shell.run_cell(f"%matplotlib {OTHER_BACKEND_NAME}").raise_error() @@ -380,7 +384,7 @@ def test_other_backend_deactivates(shell: PositronShell, positron_backend: Flavo } -def test_switching_back_reactivates(shell: PositronShell, positron_backend: Flavor): +def test_switching_back_reactivates(shell: PositronShell, positron_backend: BackendCase): """`%matplotlib inline` after another backend switches back instead of staying stuck.""" shell.run_cell(f"%matplotlib {OTHER_BACKEND_NAME}").raise_error() shell.run_cell("%matplotlib inline").raise_error() @@ -388,10 +392,10 @@ def test_switching_back_reactivates(shell: PositronShell, positron_backend: Flav assert _state(shell, positron_backend) == _positron_active(positron_backend) -def test_deactivate_twice_is_noop(shell: PositronShell, positron_backend: Flavor): +def test_deactivate_twice_is_noop(shell: PositronShell, positron_backend: BackendCase): """A second teardown for a non-Positron backend is a no-op, not an error.""" - matplotlib_backend.configure_positron_support(OTHER_BACKEND_NAME) - matplotlib_backend.configure_positron_support(OTHER_BACKEND_NAME) + registry.activate(OTHER_BACKEND_NAME) + registry.activate(OTHER_BACKEND_NAME) assert _state(shell, positron_backend) == { "backend": positron_backend.backend.full_name, @@ -401,7 +405,7 @@ def test_deactivate_twice_is_noop(shell: PositronShell, positron_backend: Flavor } -def test_inline_backend_config_stays_inert(shell: PositronShell, positron_backend: Flavor): +def test_inline_backend_config_stays_inert(shell: PositronShell, positron_backend: BackendCase): """ `%config InlineBackend.*` stays a no-op across switches that never target inline. @@ -420,7 +424,7 @@ def test_inline_backend_config_stays_inert(shell: PositronShell, positron_backen assert _state(shell, positron_backend) == _positron_active(positron_backend) -def test_matplotlib_inline_escape_hatch(shell: PositronShell, positron_backend: Flavor): +def test_matplotlib_inline_escape_hatch(shell: PositronShell, positron_backend: BackendCase): """The real matplotlib-inline backend stays reachable by its `module://` name.""" shell.run_cell(f"%matplotlib {INLINE_BACKEND_NAME}").raise_error() inline_state = _state(shell, positron_backend) @@ -436,8 +440,8 @@ def test_matplotlib_inline_escape_hatch(shell: PositronShell, positron_backend: assert _state(shell, positron_backend) == _positron_active(positron_backend) -def test_set_matplotlib_formats_patch_installed_while_active(positron_backend: Flavor): # noqa: ARG001 - """The shared `set_matplotlib_formats` patch is installed while a Positron flavor is active.""" +def test_set_matplotlib_formats_patch_installed_while_active(positron_backend: BackendCase): # noqa: ARG001 + """The shared `set_matplotlib_formats` patch is installed while a Positron backend is active.""" import matplotlib_inline.backend_inline as backend_inline assert backend_inline.set_matplotlib_formats is formats._installed_set_matplotlib_formats # noqa: SLF001 @@ -445,7 +449,7 @@ def test_set_matplotlib_formats_patch_installed_while_active(positron_backend: F def test_set_matplotlib_formats_patch_restored_after_switch( shell: PositronShell, - positron_backend: Flavor, # noqa: ARG001 + positron_backend: BackendCase, # noqa: ARG001 ): """Switching to a non-Positron backend restores the original `set_matplotlib_formats`.""" import matplotlib_inline.backend_inline as backend_inline @@ -459,14 +463,14 @@ def test_set_matplotlib_formats_patch_restored_after_switch( def test_notebook_to_console_switch_keeps_patch_installed( shell: PositronShell, - notebook_backend: Flavor, # noqa: ARG001 + notebook_backend: BackendCase, # noqa: ARG001 ): """ A notebook -> console switch keeps the same `set_matplotlib_formats` patch installed. - The patch's lifecycle is owned by `configure_positron_support` ("a Positron backend - is active"), not by the individual flavors, so a cross-flavor switch -- which never - lets `_active_backend` become `None` in between -- leaves the same patch object in + The patch's lifecycle is owned by `PositronBackendRegistry` ("a Positron backend is + active"), not by the individual backends, so a cross-backend switch -- which never + lets the active backend become `None` in between -- leaves the same patch object in place instead of tearing it down and reinstalling a fresh one. """ import matplotlib_inline.backend_inline as backend_inline @@ -479,69 +483,15 @@ def test_notebook_to_console_switch_keeps_patch_installed( assert backend_inline.set_matplotlib_formats is formats._installed_set_matplotlib_formats # noqa: SLF001 -def test_from_name_recognizes_own_names(flavor: Flavor): - """`Backend.from_name` accepts a flavor's own short name and its `module://` name.""" - assert Backend.from_name(flavor.backend.short_name) is flavor.backend - assert Backend.from_name(flavor.backend.full_name) is flavor.backend - - -def test_from_name_rejects_other_names(flavor: Flavor): - """`Backend.from_name` doesn't resolve the other flavor's names to this flavor, or a foreign backend at all.""" - other = _other_flavor(flavor) - - assert Backend.from_name(other.backend.short_name) is not flavor.backend - assert Backend.from_name(other.backend.full_name) is not flavor.backend - assert Backend.from_name(OTHER_BACKEND_NAME) is None - - -def test_from_name_short_name_case_insensitive(flavor: Flavor): - """Short names match case-insensitively, like matplotlib's backend registry.""" - assert Backend.from_name(flavor.backend.short_name.upper()) is flavor.backend - - -def test_from_name_full_name_case_sensitive(flavor: Flavor): - """The path after `module://` is an importable module path, so case matters.""" - assert Backend.from_name(f"module://{flavor.module.__name__.upper()}") is None - - -@pytest.mark.parametrize( - ("session_mode", "expected"), - [ - (SessionMode.CONSOLE, Backend.CONSOLE), - (SessionMode.NOTEBOOK, Backend.NOTEBOOK), - # BACKGROUND sessions have no notebook to render into, so they get the console - # backend, which routes figures to the plots pane. - (SessionMode.BACKGROUND, Backend.CONSOLE), - ], -) -def test_backend_for_session_mode(session_mode: SessionMode, expected: Backend): - """Each session mode maps to the Positron backend that suits it.""" - assert Backend.for_session_mode(session_mode) is expected - - -def test_preferred_name_prefers_short_name(flavor: Flavor): - """The flavor's short name is preferred when matplotlib's backend registry knows it.""" - assert flavor.backend.preferred_name == flavor.backend.short_name - - -def test_preferred_name_falls_back_to_module_name(flavor: Flavor, monkeypatch: pytest.MonkeyPatch): - """Falls back to the `module://` name before matplotlib 3.9, which has no registry.""" - monkeypatch.setattr(matplotlib_backend, "_get_backend_registry", lambda: None) - - assert flavor.backend.preferred_name == flavor.backend.full_name - - def test_install_backend_switch_hook_is_idempotent(): """Calling `install_backend_switch_hook` twice doesn't double-wrap `configure_inline_support`.""" import matplotlib_inline.backend_inline as backend_inline - matplotlib_backend.install_backend_switch_hook() - matplotlib_backend.install_backend_switch_hook() + install_backend_switch_hook() + install_backend_switch_hook() + assert backend_inline.configure_inline_support is configure_matplotlib_support assert ( - backend_inline.configure_inline_support is matplotlib_backend.configure_matplotlib_support - ) - assert ( - matplotlib_backend._original_configure_inline_support # noqa: SLF001 - is not matplotlib_backend.configure_matplotlib_support + registry._original_configure_inline_support # noqa: SLF001 + is not configure_matplotlib_support ) diff --git a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_formats.py b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_formats.py index 2ff2a064143..51760177549 100644 --- a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_formats.py +++ b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_formats.py @@ -24,7 +24,9 @@ from matplotlib.backends.backend_agg import FigureCanvasAgg from PIL import Image -from positron.matplotlib_backend import Backend, configure_positron_support, formats +from positron.matplotlib_backend import formats +from positron.matplotlib_backend.backend import Backend +from positron.matplotlib_backend.registry import registry from positron.session_mode import SessionMode from ..utils import run_with_metadata @@ -210,8 +212,8 @@ def test_preference_survives_backend_round_trip(backend: PositronShell): # Simulate switching to a non-Positron backend and back (e.g. `%matplotlib qt` then # `%matplotlib inline`). - configure_positron_support("agg") - configure_positron_support(Backend.NOTEBOOK) + registry.activate("agg") + registry.activate(Backend.NOTEBOOK) output = _plot() diff --git a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_notebook.py b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_notebook.py index a8c719875c8..0a7863ac8c2 100644 --- a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_notebook.py +++ b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_notebook.py @@ -13,7 +13,7 @@ from IPython.core.display import _pngxy from IPython.utils.capture import RichOutput, capture_output -from positron.matplotlib_backend import Backend +from positron.matplotlib_backend.backend import Backend from positron.session_mode import SessionMode from ..utils import run_with_metadata From e76de25313a13c71df7b999b498f9bed1e2c5272 Mon Sep 17 00:00:00 2001 From: seem Date: Thu, 30 Jul 2026 15:49:10 +0200 Subject: [PATCH 18/26] pin the no-matplotlib import invariant with a subprocess test --- .../test_no_matplotlib.py | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_no_matplotlib.py diff --git a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_no_matplotlib.py b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_no_matplotlib.py new file mode 100644 index 00000000000..5719c287c6a --- /dev/null +++ b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_no_matplotlib.py @@ -0,0 +1,66 @@ +# +# Copyright (C) 2026 Posit Software, PBC. All rights reserved. +# Licensed under the Elastic License 2.0. See LICENSE.txt for license information. +# +""" +Test that the kernel still imports in an environment without matplotlib. + +The kernel imports `matplotlib_backend.backend` and `.compat` unconditionally, so those +modules -- and `.registry`, which they must not drag matplotlib in through either -- have +to stay matplotlib-free; `console.py`, `notebook.py` and `formats.py` may only be +imported lazily. That invariant is documented in each module, but a stray convenience +import would break every matplotlib-less session silently, so pin it down here. +""" + +from __future__ import annotations + +import subprocess +import sys +from pathlib import Path + +import positron + +# The kernel's import root, so the subprocess resolves `positron` from this checkout. +_IMPORT_ROOT = Path(positron.__file__).parent.parent + +# Run in a subprocess: this test process has matplotlib loaded already, and a meta path +# finder can only block a module that isn't in `sys.modules` yet. +_SCRIPT = """ +import sys + + +class BlockMatplotlib: + "Meta path finder that makes matplotlib and friends look uninstalled." + + def find_spec(self, fullname, path=None, target=None): + if fullname.split(".")[0] in {"matplotlib", "matplotlib_inline", "mpl_toolkits"}: + raise ImportError(fullname) + return None + + +sys.meta_path.insert(0, BlockMatplotlib()) + +# Whatever the kernel imports at startup, plus the one module it defers to first use, so +# breaking the invariant in any of the three fails here rather than only in the field. +import positron.matplotlib_backend.registry # noqa: F401 +import positron.positron_ipkernel # noqa: F401 + +# Guard against a vacuous pass: the kernel really did import the package, and none of +# these imports pulled matplotlib in. +assert "positron.matplotlib_backend.backend" in sys.modules +assert "positron.matplotlib_backend.compat" in sys.modules +assert "matplotlib" not in sys.modules +""" + + +def test_kernel_imports_without_matplotlib(): + """Importing the kernel with matplotlib blocked succeeds and doesn't import matplotlib.""" + result = subprocess.run( + [sys.executable, "-c", _SCRIPT], + cwd=_IMPORT_ROOT, + capture_output=True, + text=True, + check=False, + ) + + assert result.returncode == 0, result.stderr From b92a7522f5171ba123150fbae289e4895a699745 Mon Sep 17 00:00:00 2001 From: seem Date: Thu, 30 Jul 2026 15:51:04 +0200 Subject: [PATCH 19/26] name the inline self-activation bypass in the switch hook docstring --- .../posit/positron/matplotlib_backend/registry.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/registry.py b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/registry.py index 24b29376ea7..cac73087e86 100644 --- a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/registry.py +++ b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/registry.py @@ -117,10 +117,11 @@ def install_switch_hook(self) -> ConfigureInlineSupport: IPython's `enable_matplotlib` imports that function from the module at call time on every backend switch, so wrapping the module attribute makes every switch -- including ones to foreign backends via `super()` -- run - `configure_matplotlib_support`. It also covers the switch that never reaches - `enable_matplotlib` at all: `matplotlib_inline.backend_inline` self-activates at - import (`_enable_matplotlib_integration`) and calls `configure_inline_support` - directly, so a bare `matplotlib.use("inline")` only goes through Positron here. + `configure_matplotlib_support`. Wrapping this function rather than overriding the + `%matplotlib` magic is also the only way to catch the switch that bypasses + IPython entirely: `matplotlib_inline.backend_inline` self-activates at import + (`_enable_matplotlib_integration`) and calls `configure_inline_support` directly, + so a bare `matplotlib.use("inline")` never reaches `enable_matplotlib`. Idempotent, and returns the real `configure_inline_support` the seam wraps. Installed lazily (not at kernel init) because importing matplotlib_inline pulls From acf8ad326141d7ac863961fa63f9a66e4342d51c Mon Sep 17 00:00:00 2001 From: seem Date: Thu, 30 Jul 2026 15:59:11 +0200 Subject: [PATCH 20/26] move legacy backend registration back to init_magics, with a regression test --- .../posit/positron/positron_ipkernel.py | 16 +++++---- .../test_matplotlib_backend/test_switching.py | 35 +++++++++++++++++++ 2 files changed, 44 insertions(+), 7 deletions(-) diff --git a/extensions/positron-python/python_files/posit/positron/positron_ipkernel.py b/extensions/positron-python/python_files/posit/positron/positron_ipkernel.py index 9c34085554e..9c945b84955 100644 --- a/extensions/positron-python/python_files/posit/positron/positron_ipkernel.py +++ b/extensions/positron-python/python_files/posit/positron/positron_ipkernel.py @@ -320,6 +320,13 @@ def init_magics(self): # Register Positron's custom magics. self.register_magics(PositronMagics) + # On IPython versions that don't read matplotlib's backend registry, add + # Positron's backends to the static table that `%matplotlib -l` lists. This + # belongs here, next to the magic that reads the table: `-l` is handled inside + # `%matplotlib` and never reaches `enable_matplotlib`, so registering lazily on + # the switch path would leave the names out of `-l` until after a switch. + register_with_legacy_ipython() + def init_user_ns(self): super().init_user_ns() @@ -363,14 +370,9 @@ def enable_matplotlib(self, gui=None): install_backend_switch_hook, ) - # Both calls are idempotent bootstrap steps that only have to be done before the - # first switch, so they're done lazily here rather than at shell init. - # Install the seam before any switch below can need it; lazy because it imports - # matplotlib (see its docstring). + # Install the seam before any switch below can need it. Idempotent; lazy + # because it imports matplotlib (see its docstring). install_backend_switch_hook() - # On IPython versions that don't read matplotlib's backend registry, add - # Positron's backends to the static table that `%matplotlib -l` lists. - register_with_legacy_ipython() # Only the sentinel check case-folds: `Backend.from_name` normalizes short names # itself, and keeps `module://` paths case-sensitive since they're module paths. diff --git a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_switching.py b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_switching.py index 5953a6d0915..81141167d2e 100644 --- a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_switching.py +++ b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_switching.py @@ -368,6 +368,41 @@ def test_register_with_legacy_ipython_adds_short_names(monkeypatch: pytest.Monke } +def test_legacy_ipython_lists_short_names_without_a_switch( + shell: PositronShell, monkeypatch: pytest.MonkeyPatch +): + """ + On IPython < 8.24, `%matplotlib -l` lists Positron's backends before any switch. + + Legacy `%matplotlib -l` prints `list(pylabtools.backends)` from inside the magic + (verified in IPython 8.23's `core/magics/pylab.py`), so it never reaches + `enable_matplotlib`. Registering the short names on the switch path instead of at + shell init would therefore leave them out of `-l` until the user had already + switched backends, which is why `PositronShell.init_magics` is the registration site. + + The magic's own `-l` branch can't be driven here, since the installed IPython lists + from matplotlib's registry rather than the static table; this asserts on the table + that legacy `-l` reads. + """ + from IPython.core import pylabtools as pt + + # Simulate IPython < 8.24: no registry-based lister, `-l` reads `pt.backends`. + if hasattr(pt, "_list_matplotlib_backends_and_gui_loops"): + monkeypatch.delattr(pt, "_list_matplotlib_backends_and_gui_loops") + monkeypatch.setattr(pt, "backends", {"inline": INLINE_BACKEND_NAME}) + + # The shell was constructed before the simulation was in place, so re-run the hook + # that registers the names. Notably not `enable_matplotlib`: nothing here switches + # backends, because `-l` has to list them without one. + shell.init_magics() + + assert set(pt.backends) == { + "inline", + Backend.CONSOLE.short_name, + Backend.NOTEBOOK.short_name, + } + + def test_other_backend_deactivates(shell: PositronShell, positron_backend: Flavor): """Switching to another backend removes Positron's hooks instead of leaving them.""" shell.run_cell(f"%matplotlib {OTHER_BACKEND_NAME}").raise_error() From 308544a2cefb111ff0e30408f5268e4c2ce94b4a Mon Sep 17 00:00:00 2001 From: seem Date: Thu, 30 Jul 2026 16:00:41 +0200 Subject: [PATCH 21/26] don't duplicate the output of an explicit fig.show() the post-execute hook now skips figures a cell already displayed, instead of showing every figure again --- .../positron/matplotlib_backend/notebook.py | 39 ++++++++++++++++--- .../test_matplotlib_backend/test_notebook.py | 31 +++++++++++++-- 2 files changed, 61 insertions(+), 9 deletions(-) diff --git a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/notebook.py b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/notebook.py index 94458fd2b05..49d749860de 100644 --- a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/notebook.py +++ b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/notebook.py @@ -75,12 +75,18 @@ def new_figure_manager( class FigureManagerPositronNotebook(FigureManagerBase): canvas: FigureCanvasPositronNotebook # type: ignore + # Whether this figure has already been displayed during the current cell. Only the + # post-execute hook reads it, so an explicit show always displays. Public because + # `_show_figures` reads it off another object. + displayed = False + def show(self): """Called by matplotlib when a figure is shown via `plt.show()` or `figure.show()`. Displays the figure inline, as an output of the currently executing cell. """ display(self.canvas.figure) + self.displayed = True @classmethod def pyplot_show(cls, *, block: bool | None = None) -> None: @@ -88,9 +94,7 @@ def pyplot_show(cls, *, block: bool | None = None) -> None: try: super().pyplot_show(block=block) finally: - # Close all figures after showing them. - if Gcf.get_all_fig_managers(): - plt.close("all") + _close_all_figures() class FigureCanvasPositronNotebook(FigureCanvasAgg): @@ -110,12 +114,37 @@ def set_device_pixel_ratio(self, ratio: float) -> None: FigureManager = FigureManagerPositronNotebook +def _close_all_figures() -> None: + """Close all figures, so that they don't accumulate across cells.""" + # `close("all")` triggers a gc collect, which can be slow, so skip it if there's + # nothing to close. + if Gcf.get_all_fig_managers(): + plt.close("all") + + def _show_figures(): - """Post execute hook to show all figures and log errors.""" + """Post execute hook to show the cell's figures and log errors.""" try: - return FigureManagerPositronNotebook.pyplot_show() + # Don't reuse `pyplot_show` here: it shows every figure, which would emit a second + # output for a figure that user code already displayed via `plt.show()`/`fig.show()`. + # Matplotlib's own loop adds nothing else for a non-GUI backend - it only handles + # `NonGuiException` (our `show` displays instead of raising) and blocks on a main + # loop we don't have. + for manager in Gcf.get_all_fig_managers(): + if not isinstance(manager, FigureManagerPositronNotebook): + manager.show() + continue + + if not manager.displayed: + manager.show() + + # Reset even though the figure is about to be closed: should one ever outlive + # the cell, showing it again beats silently swallowing its output. + manager.displayed = False except Exception: logger.exception("Error showing figures in post execute hook") + finally: + _close_all_figures() def install(shell: InteractiveShell) -> None: diff --git a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_notebook.py b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_notebook.py index d9bbef0e9eb..62d029cf14a 100644 --- a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_notebook.py +++ b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_notebook.py @@ -207,8 +207,8 @@ def test_plt_show_displays_figure(backend): assert png.startswith(b"\x89PNG") -def test_figure_show_displays_figure(backend): - """An explicit `fig.show()` displays the figure inline.""" +def test_figure_show_displays_figure_once(backend): + """An explicit `fig.show()` displays the figure inline, exactly once.""" outputs = backend.run( "import matplotlib.pyplot as plt", "fig, ax = plt.subplots()", @@ -216,11 +216,34 @@ def test_figure_show_displays_figure(backend): "fig.show()", ) - # Two outputs: the explicit `fig.show()`, then the post-execute hook, which still finds - # the figure since `Figure.show` doesn't close it. + # `Figure.show` doesn't close the figure, so the post-execute hook still finds it; it + # skips the figure since the explicit show already displayed it. + assert len(outputs) == 1 + + +def test_repeated_figure_show_displays_each_time(backend): + """Only the automatic end-of-cell display is skipped; explicit shows always display.""" + outputs = backend.run( + "import matplotlib.pyplot as plt", + "fig, ax = plt.subplots()", + "fig.show()", + "ax.plot([0, 1], [0, 1])", + "fig.show()", + ) + assert len(outputs) == 2 +def test_explicit_show_does_not_suppress_a_later_cell(backend): + """A cell's explicit show doesn't suppress the automatic display in the next cell.""" + shown = backend.run("import matplotlib.pyplot as plt", "fig, ax = plt.subplots()", "fig.show()") + assert len(shown) == 1 + + # No explicit show in this cell, so the post-execute hook must display its figure. A + # "displayed" flag that outlived the previous cell would silently swallow the output. + assert len(backend.run("fig, ax = plt.subplots()", "ax.plot([0, 1], [0, 1])")) == 1 + + def test_figures_do_not_accumulate_across_cells(backend): """Figures are closed once shown, so a later cell doesn't re-display earlier ones.""" backend.plot() From e3ae6f1fafd2fd6b56c42121dcc19c5c0746e6fc Mon Sep 17 00:00:00 2001 From: seem Date: Thu, 30 Jul 2026 16:28:45 +0200 Subject: [PATCH 22/26] test current_execute_request; fix python 3.9 annotations in test_execute_request --- .../posit/positron/execute_request.py | 3 ++ .../positron/tests/test_execute_request.py | 35 ++++++++++++++++++- 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/extensions/positron-python/python_files/posit/positron/execute_request.py b/extensions/positron-python/python_files/posit/positron/execute_request.py index fd95be8963e..dc1e2480ea5 100644 --- a/extensions/positron-python/python_files/posit/positron/execute_request.py +++ b/extensions/positron-python/python_files/posit/positron/execute_request.py @@ -92,6 +92,9 @@ def from_message(cls, message: dict) -> "PositronExecuteRequest": def current_execute_request() -> PositronExecuteRequest: """The Positron `execute_request` currently being handled, if any.""" + # No contextvar of our own needed here: ipykernel's `get_parent("shell")` is + # already backed by a `ContextVar` (see `_shell_parent` in kernelbase.py), so + # this is async/thread-safe as-is. # Imported lazily to avoid a circular import. from .positron_ipkernel import PositronIPyKernel diff --git a/extensions/positron-python/python_files/posit/positron/tests/test_execute_request.py b/extensions/positron-python/python_files/posit/positron/tests/test_execute_request.py index f52ae2c80c3..74dcf772e03 100644 --- a/extensions/positron-python/python_files/posit/positron/tests/test_execute_request.py +++ b/extensions/positron-python/python_files/posit/positron/tests/test_execute_request.py @@ -3,9 +3,18 @@ # Licensed under the Elastic License 2.0. See LICENSE.txt for license information. # +from __future__ import annotations + +from typing import TYPE_CHECKING + import pytest -from positron.execute_request import PositronExecuteRequest +from positron.execute_request import PositronExecuteRequest, current_execute_request + +if TYPE_CHECKING: + from positron.positron_ipkernel import PositronIPyKernel + + from .conftest import MockSession def test_parses_code_location() -> None: @@ -149,3 +158,27 @@ def test_figure_size( meta = PositronExecuteRequest.from_message(message) assert meta.figure_size == expected + + +def test_current_execute_request_reads_figure_size( + kernel: PositronIPyKernel, session: MockSession +) -> None: + """`current_execute_request` parses the kernel's current shell parent message.""" + message = session.msg( + "execute_request", content={"positron": {"fig-width": 6.4, "fig-height": 4.8}} + ) + + kernel.set_parent([], message, channel="shell") + + assert current_execute_request().figure_size == (6.4, 4.8) + + +def test_current_execute_request_with_no_metadata( + kernel: PositronIPyKernel, session: MockSession +) -> None: + """A shell parent message without `content.positron` yields an empty model.""" + message = session.msg("execute_request", content={}) + + kernel.set_parent([], message, channel="shell") + + assert current_execute_request().figure_size is None From d04ba1fff01703510484db3b93ced4b4db577bd6 Mon Sep 17 00:00:00 2001 From: seem Date: Fri, 31 Jul 2026 14:21:26 +0200 Subject: [PATCH 23/26] skip module:// escape hatch test on IPython < 8.24 --- .../test_enable_matplotlib.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_enable_matplotlib.py b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_enable_matplotlib.py index fda3757cbfb..fec2fd7e96c 100644 --- a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_enable_matplotlib.py +++ b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_enable_matplotlib.py @@ -23,6 +23,7 @@ import matplotlib import matplotlib.pyplot as plt import pytest +from IPython.core import pylabtools from IPython.utils.capture import capture_output from matplotlib.figure import Figure @@ -459,8 +460,19 @@ def test_inline_backend_config_stays_inert(shell: PositronShell, positron_backen assert _state(shell, positron_backend) == _positron_active(positron_backend) +@pytest.mark.skipif( + not hasattr(pylabtools, "_list_matplotlib_backends_and_gui_loops"), + reason="`%matplotlib module://...` needs IPython >= 8.24's registry-based resolution", +) def test_matplotlib_inline_escape_hatch(shell: PositronShell, positron_backend: BackendCase): - """The real matplotlib-inline backend stays reachable by its `module://` name.""" + """ + The real matplotlib-inline backend stays reachable by its `module://` name. + + Only via `%matplotlib` on IPython >= 8.24: earlier versions resolve the magic's + argument through a static table that raises KeyError for `module://` names, matching + upstream behavior. `matplotlib.use(INLINE_BACKEND_NAME)` is the escape hatch there, + on any IPython, via the backend module's self-activation at import. + """ shell.run_cell(f"%matplotlib {INLINE_BACKEND_NAME}").raise_error() inline_state = _state(shell, positron_backend) From c14230164ba07bd4f11b5ce437969d230411c2c6 Mon Sep 17 00:00:00 2001 From: seem Date: Fri, 31 Jul 2026 14:28:33 +0200 Subject: [PATCH 24/26] skip module:// escape hatch test without registry backend resolution --- .../test_enable_matplotlib.py | 28 +++++++++++++++---- 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_enable_matplotlib.py b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_enable_matplotlib.py index fec2fd7e96c..65cbe6aff8a 100644 --- a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_enable_matplotlib.py +++ b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_enable_matplotlib.py @@ -50,6 +50,11 @@ # The backend to switch away to. Headless, so these tests don't need a display. OTHER_BACKEND_NAME = "agg" +# Whether `%matplotlib ` resolves through matplotlib's backend registry, which +# needs IPython >= 8.24 *and* matplotlib >= 3.9 -- IPython 8.24 kept the static-table +# fallback for older matplotlib. This private helper encodes both halves. +REGISTRY_BACKEND_RESOLUTION = getattr(pylabtools, "_matplotlib_manages_backends", lambda: False)() + class BackendCase(NamedTuple): """One of Positron's matplotlib backends, and how to detect that it's installed.""" @@ -461,17 +466,28 @@ def test_inline_backend_config_stays_inert(shell: PositronShell, positron_backen @pytest.mark.skipif( - not hasattr(pylabtools, "_list_matplotlib_backends_and_gui_loops"), - reason="`%matplotlib module://...` needs IPython >= 8.24's registry-based resolution", + not REGISTRY_BACKEND_RESOLUTION, + reason="`%matplotlib module://...` needs registry-based backend resolution", ) def test_matplotlib_inline_escape_hatch(shell: PositronShell, positron_backend: BackendCase): """ The real matplotlib-inline backend stays reachable by its `module://` name. - Only via `%matplotlib` on IPython >= 8.24: earlier versions resolve the magic's - argument through a static table that raises KeyError for `module://` names, matching - upstream behavior. `matplotlib.use(INLINE_BACKEND_NAME)` is the escape hatch there, - on any IPython, via the backend module's self-activation at import. + Only via `%matplotlib` where backends resolve through matplotlib's registry: without + it, IPython resolves the magic's argument through a static table that has `inline` + but not the `module://` spelling, so the switch raises KeyError before reaching + Positron -- upstream behavior for any `module://` name, not something the override + introduces. `matplotlib.use(INLINE_BACKEND_NAME)` is the escape hatch there instead, + on every version: the backend module self-activates at import, flowing through + Positron's `configure_inline_support` seam. + + Skipped rather than shimmed on the table-based versions (IPython < 8.24 or + matplotlib < 3.9, both 2+ years old; all of Python 3.9, EOL October 2025, is capped + at IPython 8.18): `matplotlib.use` fully covers the escape, the KeyError matches + stock IPython, and the affected stacks only shrink. If a user on one of them ever + needs the magic spelling, `PositronShell.enable_matplotlib` already switches + Positron's own names by hand on exactly these versions, and that recipe extends to + `module://` names in a few lines. """ shell.run_cell(f"%matplotlib {INLINE_BACKEND_NAME}").raise_error() inline_state = _state(shell, positron_backend) From 962293016ef3337cbe683c1d0c09646708b9d674 Mon Sep 17 00:00:00 2001 From: seem Date: Fri, 31 Jul 2026 14:45:53 +0200 Subject: [PATCH 25/26] fix numpy array figure sizes --- .../posit/positron/matplotlib_backend/notebook.py | 5 ++++- .../test_matplotlib_backend/test_notebook.py | 15 +++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/notebook.py b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/notebook.py index 2725e51fd8d..2f28f95b2bc 100644 --- a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/notebook.py +++ b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/notebook.py @@ -57,7 +57,10 @@ def new_figure_manager( # Sizing precedence: an explicit size from user code (`plt.figure(figsize=...)`, # `plt.subplots(figsize=...)`) wins, else the cell's `#| fig-width`/`#| fig-height`, # else matplotlib's `figure.figsize` rcParam, which `Figure` applies for a None figsize. - figsize = figsize or execute_request.figure_size + # Compare against None rather than truthiness: matplotlib accepts any array-like + # figsize, and a numpy array raises on `bool()`. + if figsize is None: + figsize = execute_request.figure_size # `Figure` only accepts the 3-tuple form in matplotlib >= 3.11, but we pass whatever # matplotlib handed us straight back to it, so the pair always matches at runtime. # The cast keeps pyright quiet against the older pins (CI type-checks on 3.9). diff --git a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_notebook.py b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_notebook.py index 87d3145d99d..cd88ff39310 100644 --- a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_notebook.py +++ b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_notebook.py @@ -126,6 +126,21 @@ def test_explicit_figsize_wins(backend): assert result.figure_size == [1.0, 1.0] +def test_explicit_array_like_figsize_wins(backend): + """An array-like figsize also beats the pending default. + + matplotlib accepts any array-like figsize, e.g. a numpy array, which raises on `bool()`. + """ + run_with_metadata( + "import numpy as np\n" + "import matplotlib.pyplot as plt\n" + "fig, ax = plt.subplots(figsize=np.array([4.0, 3.0]) * 2)", + {"fig-width": 8, "fig-height": 4}, + ) + figure: Figure = backend.shell.user_ns["fig"] + assert figure.get_size_inches().tolist() == [8.0, 6.0] + + def test_figure_size_does_not_leak_to_later_cell(backend): """A sized cell does not leak into a later unsized cell.""" backend.plot(meta={"fig-width": 8, "fig-height": 4}) From bd23bf0644158777363ec35cb1fe01c057dc9af4 Mon Sep 17 00:00:00 2001 From: seem Date: Fri, 31 Jul 2026 17:11:47 +0200 Subject: [PATCH 26/26] apply a lone fig-width or fig-height, filling the other from rcParams --- .../posit/positron/execute_request.py | 18 ++++++++----- .../positron/matplotlib_backend/notebook.py | 26 +++++++++++++++--- .../positron/tests/test_execute_request.py | 17 +++++++----- .../test_matplotlib_backend/test_notebook.py | 27 ++++++++++++++++--- 4 files changed, 68 insertions(+), 20 deletions(-) diff --git a/extensions/positron-python/python_files/posit/positron/execute_request.py b/extensions/positron-python/python_files/posit/positron/execute_request.py index dc1e2480ea5..d458fda931a 100644 --- a/extensions/positron-python/python_files/posit/positron/execute_request.py +++ b/extensions/positron-python/python_files/posit/positron/execute_request.py @@ -59,12 +59,18 @@ class PositronExecuteRequest(BaseModel): ) @property - def figure_size(self) -> Optional[tuple[float, float]]: - """The figure size in inches, if specified.""" - w, h = self.fig_width, self.fig_height - if w is not None and h is not None and w > 0 and h > 0: - return (w, h) - return None + def figure_size(self) -> Optional[tuple[Optional[float], Optional[float]]]: + """The figure size in inches, if specified. + + Either dimension may be set alone; a missing or non-positive dimension is + None, for the caller to fill from its own default. None when neither + dimension is specified. + """ + w = self.fig_width if self.fig_width is not None and self.fig_width > 0 else None + h = self.fig_height if self.fig_height is not None and self.fig_height > 0 else None + if w is None and h is None: + return None + return (w, h) @classmethod def from_message(cls, message: dict) -> "PositronExecuteRequest": diff --git a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/notebook.py b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/notebook.py index 2f28f95b2bc..8c6e5ec9358 100644 --- a/extensions/positron-python/python_files/posit/positron/matplotlib_backend/notebook.py +++ b/extensions/positron-python/python_files/posit/positron/matplotlib_backend/notebook.py @@ -31,7 +31,7 @@ from matplotlib.backends.backend_agg import FigureCanvasAgg from matplotlib.figure import Figure -from ..execute_request import current_execute_request +from ..execute_request import PositronExecuteRequest, current_execute_request from . import formats from .backend import Backend from .registry import registry @@ -55,12 +55,13 @@ def new_figure_manager( execute_request = current_execute_request() # Sizing precedence: an explicit size from user code (`plt.figure(figsize=...)`, - # `plt.subplots(figsize=...)`) wins, else the cell's `#| fig-width`/`#| fig-height`, - # else matplotlib's `figure.figsize` rcParam, which `Figure` applies for a None figsize. + # `plt.subplots(figsize=...)`) wins, else the cell's `#| fig-width`/`#| fig-height` + # (either may be set alone, per dimension), else matplotlib's `figure.figsize` + # rcParam, which `Figure` applies for a None figsize. # Compare against None rather than truthiness: matplotlib accepts any array-like # figsize, and a numpy array raises on `bool()`. if figsize is None: - figsize = execute_request.figure_size + figsize = _requested_figsize(execute_request) # `Figure` only accepts the 3-tuple form in matplotlib >= 3.11, but we pass whatever # matplotlib handed us straight back to it, so the pair always matches at runtime. # The cast keeps pyright quiet against the older pins (CI type-checks on 3.9). @@ -77,6 +78,23 @@ def new_figure_manager( return manager +def _requested_figsize(execute_request: PositronExecuteRequest) -> tuple[float, float] | None: + """The figsize requested via the cell's `#| fig-width`/`#| fig-height`, if any. + + Either option may be set alone, matching `quarto render`; the missing dimension + falls back to the current `figure.figsize` rcParam. + """ + size = execute_request.figure_size + if size is None: + return None + width, height = size + default_width, default_height = matplotlib.rcParams["figure.figsize"] + return ( + width if width is not None else default_width, + height if height is not None else default_height, + ) + + class FigureManagerPositronNotebook(FigureManagerBase): canvas: FigureCanvasPositronNotebook # type: ignore diff --git a/extensions/positron-python/python_files/posit/positron/tests/test_execute_request.py b/extensions/positron-python/python_files/posit/positron/tests/test_execute_request.py index 74dcf772e03..d04bb96fb0b 100644 --- a/extensions/positron-python/python_files/posit/positron/tests/test_execute_request.py +++ b/extensions/positron-python/python_files/posit/positron/tests/test_execute_request.py @@ -142,17 +142,22 @@ def test_unknown_keys_are_ignored() -> None: ("fig_width", "fig_height", "expected"), [ (6.4, 4.8, (6.4, 4.8)), - (None, 4.8, None), - (6.4, None, None), + # A lone dimension passes through; the caller fills the other from its default. + (None, 4.8, (None, 4.8)), + (6.4, None, (6.4, None)), (None, None, None), - (-1.0, 4.8, None), - (6.4, -1.0, None), + # A non-positive dimension counts as unspecified. + (-1.0, 4.8, (None, 4.8)), + (6.4, -1.0, (6.4, None)), + (-1.0, -1.0, None), ], ) def test_figure_size( - fig_width: float | None, fig_height: float | None, expected: tuple[float, float] | None + fig_width: float | None, + fig_height: float | None, + expected: tuple[float | None, float | None] | None, ) -> None: - """The `figure_size` property returns a tuple of (width, height) if both are positive.""" + """`figure_size` keeps each positive dimension and is None only when neither is set.""" message = {"content": {"positron": {"fig-width": fig_width, "fig-height": fig_height}}} meta = PositronExecuteRequest.from_message(message) diff --git a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_notebook.py b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_notebook.py index cd88ff39310..a537bf4971b 100644 --- a/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_notebook.py +++ b/extensions/positron-python/python_files/posit/positron/tests/test_matplotlib_backend/test_notebook.py @@ -150,9 +150,6 @@ def test_figure_size_does_not_leak_to_later_cell(backend): @pytest.mark.parametrize( "meta", [ - # Lone dimension is a no-op. - {"fig-width": 1}, - {"fig-height": 1}, # Non-positive dimension is a no-op. {"fig-width": 0}, {"fig-height": 0}, @@ -160,10 +157,32 @@ def test_figure_size_does_not_leak_to_later_cell(backend): ], ) def test_figure_size_noop(backend, meta): - """A lone fig-height leaves the figure at the default size.""" + """Invalid sizing metadata leaves the figure at the default size.""" assert backend.plot(meta=meta).figure_size == DEFAULT_FIGSIZE +@pytest.mark.parametrize( + ("meta", "expected"), + [ + ({"fig-width": 8}, [8.0, DEFAULT_FIGSIZE[1]]), + ({"fig-height": 4}, [DEFAULT_FIGSIZE[0], 4.0]), + # A non-positive dimension counts as unspecified. + ({"fig-width": 0, "fig-height": 4}, [DEFAULT_FIGSIZE[0], 4.0]), + ], +) +def test_lone_figure_dimension_uses_default_for_other(backend, meta, expected): + """A lone `fig-width`/`fig-height` applies; the other falls back to the default.""" + assert backend.plot(meta=meta).figure_size == expected + + +def test_lone_figure_dimension_falls_back_to_rc_params(backend): + """The unspecified dimension falls back to the current rcParam, not the stock default.""" + with matplotlib.rc_context({"figure.figsize": (3.0, 2.0)}): + result = backend.plot(meta={"fig-width": 8}) + + assert result.figure_size == [8.0, 2.0] + + def test_pixel_ratio_scales_png_size_and_attaches_metadata(backend): """A pixel ratio scales the pixels, not the figure's inch size.""" base = backend.plot(meta={"fig-width": 8, "fig-height": 4, "output_pixel_ratio": 1})