diff --git a/extensions/positron-python/python_files/.vscode/settings.json b/extensions/positron-python/python_files/.vscode/settings.json index 480631710e8..165fbc8b9fc 100644 --- a/extensions/positron-python/python_files/.vscode/settings.json +++ b/extensions/positron-python/python_files/.vscode/settings.json @@ -9,7 +9,8 @@ "editor.rulers": [100], }, "python.testing.pytestArgs": [ - "tests" + "tests", + "positron/positron_ipykernel/tests" ], "python.testing.unittestEnabled": false, "python.testing.pytestEnabled": true, diff --git a/extensions/positron-python/python_files/positron/positron_ipykernel/positron_ipkernel.py b/extensions/positron-python/python_files/positron/positron_ipykernel/positron_ipkernel.py index d3356795800..01b81885ead 100644 --- a/extensions/positron-python/python_files/positron/positron_ipykernel/positron_ipkernel.py +++ b/extensions/positron-python/python_files/positron/positron_ipykernel/positron_ipkernel.py @@ -11,7 +11,7 @@ import re import warnings from pathlib import Path -from typing import Any, Callable, Container, Dict, List, Optional, Type +from typing import Any, Callable, Container, Dict, List, Optional, Type, cast import traitlets from ipykernel.comm.manager import CommManager @@ -45,6 +45,26 @@ class _CommTarget(str, enum.Enum): Connections = "positron.connection" +class SessionMode(str, enum.Enum): + """ + The mode that the kernel application was started in. + """ + + Console = "console" + Notebook = "notebook" + Background = "background" + + Default = Console + + def __str__(self) -> str: + # Override for better display in argparse help. + return self.value + + @classmethod + def trait(cls) -> traitlets.Enum: + return traitlets.Enum(sorted(cls), help=cls.__doc__) + + logger = logging.getLogger(__name__) @@ -138,6 +158,18 @@ class PositronShell(ZMQInteractiveShell): help="Class to use to instantiate the shell inspector", ).tag(config=True) + # Positron-specific attributes: + session_mode: SessionMode = SessionMode.trait() # type: ignore + + def __init__(self, *args, **kwargs): + # Set custom attributes from the parent object. + # It would be better to pass these as explicit arguments, but there's no easy way + # to override the parent to do that. + parent = cast(PositronIPyKernel, kwargs["parent"]) + self.session_mode = parent.session_mode + + super().__init__(*args, **kwargs) + def init_events(self) -> None: super().init_events() @@ -250,6 +282,11 @@ def _showtraceback(self, etype, evalue: Exception, stb: List[str]): # type: ign """ Enhance tracebacks for the Positron frontend. """ + if self.session_mode == SessionMode.Notebook: + # Don't modify the traceback in a notebook. The frontend assumes that it's unformatted + # and applies its own formatting. + return super()._showtraceback(etype, evalue, stb) # type: ignore IPython type annotation is wrong + # Remove the first two lines of the traceback, which are the "---" header and the repeated # exception name and "Traceback (most recent call last)". # Also remove the last line of the traceback, which repeats f"{etype}: {evalue}". @@ -305,7 +342,16 @@ class PositronIPyKernel(IPythonKernel): klass=InteractiveShell, ) + # Positron-specific attributes: + session_mode: SessionMode = SessionMode.trait() # type: ignore + def __init__(self, **kwargs) -> None: + # Set custom attributes from the parent object. + # It would be better to pass these as explicit arguments, but there's no easy way + # to override the parent to do that. + parent = cast(PositronIPKernelApp, kwargs["parent"]) + self.session_mode = parent.session_mode + super().__init__(**kwargs) # Create Positron services @@ -383,6 +429,9 @@ class PositronIPKernelApp(IPKernelApp): # Use the PositronIPyKernel class. kernel_class: Type[PositronIPyKernel] = traitlets.Type(PositronIPyKernel) # type: ignore + # Positron-specific attributes: + session_mode: SessionMode = SessionMode.trait() # type: ignore + # # OSC8 functionality diff --git a/extensions/positron-python/python_files/positron/positron_ipykernel/tests/conftest.py b/extensions/positron-python/python_files/positron/positron_ipykernel/tests/conftest.py index 8bac479aaec..66bbb6affc9 100644 --- a/extensions/positron-python/python_files/positron/positron_ipykernel/tests/conftest.py +++ b/extensions/positron-python/python_files/positron/positron_ipykernel/tests/conftest.py @@ -3,13 +3,19 @@ # from typing import Iterable -from unittest.mock import Mock +from unittest.mock import MagicMock, Mock import comm import pytest +from traitlets.config import Config from positron_ipykernel.connections import ConnectionsService from positron_ipykernel.data_explorer import DataExplorerService -from positron_ipykernel.positron_ipkernel import PositronIPyKernel, PositronShell +from positron_ipykernel.positron_ipkernel import ( + PositronIPKernelApp, + PositronIPyKernel, + PositronShell, + SessionMode, +) from positron_ipykernel.variables import VariablesService @@ -43,10 +49,15 @@ def kernel() -> PositronIPyKernel: """ The Positron kernel, configured for testing purposes. """ - # Create a Positron kernel. The kernel calls shell_class.instance() to get the globally - # registered shell instance, and IPython registers a TerminalInteractiveShell instead of a - # PositronShell. This causes a traitlets validation error unless we pass the shell_class explicitly. - kernel = PositronIPyKernel.instance(shell_class=PositronShell) + # Mock the application object. We haven't needed to use it in tests yet, but we do need it to + # pass our custom attributes down to the shell. + app = MagicMock(PositronIPKernelApp) + app.config = Config() # Needed to avoid traitlets errors + + # Positron-specific attributes: + app.session_mode = SessionMode.Console + + kernel = PositronIPyKernel.instance(parent=app) return kernel diff --git a/extensions/positron-python/python_files/positron/positron_ipykernel/tests/test_positron_ipkernel.py b/extensions/positron-python/python_files/positron/positron_ipykernel/tests/test_positron_ipkernel.py index 966027c8599..a47cd53aa20 100644 --- a/extensions/positron-python/python_files/positron/positron_ipykernel/tests/test_positron_ipkernel.py +++ b/extensions/positron-python/python_files/positron/positron_ipykernel/tests/test_positron_ipkernel.py @@ -10,6 +10,7 @@ import pandas as pd import pytest from IPython.utils.syspathcontext import prepended_to_syspath +from positron_ipykernel.positron_ipkernel import SessionMode from positron_ipykernel.help import help from positron_ipykernel.utils import alias_home @@ -81,7 +82,12 @@ def g(): """ -def test_traceback(shell: PositronShell, tmp_path: Path, mock_displayhook: Mock) -> None: +def test_console_traceback( + shell: PositronShell, tmp_path: Path, mock_displayhook: Mock, monkeypatch +) -> None: + # Ensure that we're in console mode. + monkeypatch.setattr(shell, "session_mode", SessionMode.Console) + # We follow the approach of IPython's test_ultratb.py, which is to create a temporary module, # prepend its parent directory to sys.path, import it, then run a cell that calls a function # from it. @@ -167,6 +173,44 @@ def test_traceback(shell: PositronShell, tmp_path: Path, mock_displayhook: Mock) ) +def test_notebook_traceback( + shell: PositronShell, tmp_path: Path, mock_displayhook: Mock, monkeypatch +) -> None: + # Ensure that we're in notebook mode. + monkeypatch.setattr(shell, "session_mode", SessionMode.Notebook) + + # We follow the approach of IPython's test_ultratb.py, which is to create a temporary module, + # prepend its parent directory to sys.path, import it, then run a cell that calls a function + # from it. + + # Create a temporary module. + file = tmp_path / "test_traceback.py" + file.write_text(code) + + # Temporarily add the module to sys.path and call a function from it, which should error. + with prepended_to_syspath(str(tmp_path)): + shell.run_cell("import test_traceback; test_traceback.g()") + + # Check that a single message was sent to the frontend. + call_args_list = mock_displayhook.session.send.call_args_list + assert len(call_args_list) == 1 + + call_args = call_args_list[0] + + # Check that the message was sent over the "error" stream. + assert call_args.args[1] == "error" + + exc_content = call_args.args[2] + + # Check that we haven't removed any frames. + # We don't check the traceback contents in this case since that's tested in IPython. + assert len(exc_content["traceback"]) == 6 + + # Check that we haven't modified any other contents. + assert exc_content["ename"] == "Exception" + assert exc_content["evalue"] == "This is an error!" + + def assert_ansi_string_startswith(actual: str, expected: str) -> None: """ Assert that an ansi-formatted string starts with an expected string, in a way that gets pytest diff --git a/extensions/positron-python/python_files/positron/positron_language_server.py b/extensions/positron-python/python_files/positron/positron_language_server.py index 77da0b54b0c..ed1f304f712 100644 --- a/extensions/positron-python/python_files/positron/positron_language_server.py +++ b/extensions/positron-python/python_files/positron/positron_language_server.py @@ -9,9 +9,8 @@ import os import sys -from positron_ipykernel.positron_ipkernel import PositronIPKernelApp +from positron_ipykernel.positron_ipkernel import PositronIPKernelApp, SessionMode from positron_ipykernel.positron_jedilsp import POSITRON -from traitlets.config import Config logger = logging.getLogger(__name__) @@ -57,9 +56,9 @@ def parse_args() -> argparse.Namespace: parser.add_argument( "--session-mode", help="session mode in which the kernel is to be started", - type=str, - default="console", - choices=["console", "notebook", "background"], + type=SessionMode, + default=SessionMode.Default, + choices=sorted(SessionMode), ) args = parser.parse_args() args.loglevel = args.loglevel.upper() @@ -108,20 +107,17 @@ def parse_args() -> argparse.Namespace: } } - # Start Positron's IPyKernel as the interpreter for our console. # IPKernelApp expects an empty string if no connection_file is provided. if args.connection_file is None: args.connection_file = "" - config = Config( - IPKernelApp={ - "connection_file": args.connection_file, - "log_level": args.loglevel, - "logging_config": logging_config, - }, + # Start Positron's IPyKernel as the interpreter for our console. + app: PositronIPKernelApp = PositronIPKernelApp.instance( + connection_file=args.connection_file, + log_level=args.loglevel, + logging_config=logging_config, + session_mode=args.session_mode, ) - - app: PositronIPKernelApp = PositronIPKernelApp.instance(config=config) # Initialize with empty argv, otherwise BaseIPythonApplication.initialize reuses our # command-line arguments in unexpected ways (e.g. logfile instructs it to log executed code). app.initialize(argv=[]) diff --git a/src/vs/workbench/contrib/positronNotebook/browser/getOutputContents.ts b/src/vs/workbench/contrib/positronNotebook/browser/getOutputContents.ts index 7d02fd0bfe4..e8002c03538 100644 --- a/src/vs/workbench/contrib/positronNotebook/browser/getOutputContents.ts +++ b/src/vs/workbench/contrib/positronNotebook/browser/getOutputContents.ts @@ -122,7 +122,7 @@ export function parseOutputData(output: ICellOutput['outputs'][number]): ParsedO } if (mime === 'application/vnd.code.notebook.error') { - return { type: 'error', content: parsedMessage.message }; + return { type: 'error', content: parsedMessage.stack }; } } catch (e) { diff --git a/src/vs/workbench/contrib/positronNotebook/browser/notebookCells/CellTextOutput.css b/src/vs/workbench/contrib/positronNotebook/browser/notebookCells/CellTextOutput.css index 6c96a8dc199..4e15d70b414 100644 --- a/src/vs/workbench/contrib/positronNotebook/browser/notebookCells/CellTextOutput.css +++ b/src/vs/workbench/contrib/positronNotebook/browser/notebookCells/CellTextOutput.css @@ -4,4 +4,6 @@ .positron-notebook-text-output { font-family: var(--monaco-monospace-font, monospace); + font-size: var(--notebook-cell-output-font-size, 12px); + white-space: pre-wrap; }