diff --git a/extensions/positron-python/python_files/positron/positron_ipykernel/plot_comm.py b/extensions/positron-python/python_files/positron/positron_ipykernel/plot_comm.py index fdeaa8dcd67..57a160b890e 100644 --- a/extensions/positron-python/python_files/positron/positron_ipykernel/plot_comm.py +++ b/extensions/positron-python/python_files/positron/positron_ipykernel/plot_comm.py @@ -59,6 +59,10 @@ class RenderParams(BaseModel): description="The pixel ratio of the display device", ) + format: str = Field( + description="The requested plot format", + ) + class RenderRequest(BaseModel): """ diff --git a/extensions/positron-python/python_files/positron/positron_ipykernel/plots.py b/extensions/positron-python/python_files/positron/positron_ipykernel/plots.py index 9ecae76f816..03b061db47f 100644 --- a/extensions/positron-python/python_files/positron/positron_ipykernel/plots.py +++ b/extensions/positron-python/python_files/positron/positron_ipykernel/plots.py @@ -26,6 +26,13 @@ DEFAULT_HEIGHT_IN = 4.8 BASE_DPI = 100 +MIME_TYPE = { + "png": "image/png", + "svg": "image/svg+xml", + "pdf": "application/pdf", + "jpeg": "image/jpeg", +} + class PositronDisplayPublisherHook: def __init__(self, target_name: str, session_mode: SessionMode): @@ -112,12 +119,16 @@ def handle_msg(self, msg: CommMessage[PlotBackendMessageContent], raw_msg: JsonR width_px = request.params.width or 0 height_px = request.params.height or 0 pixel_ratio = request.params.pixel_ratio or 1.0 + format = request.params.format or "png" if width_px != 0 and height_px != 0: - format_dict = self._resize_pickled_figure(pickled, width_px, height_px, pixel_ratio) - data = format_dict["image/png"] - output = PlotResult(data=data, mime_type="image/png").dict() - figure_comm.send_result(data=output, metadata={"mime_type": "image/png"}) + format_dict = self._resize_pickled_figure( + pickled, width_px, height_px, pixel_ratio, [format] + ) + mime_type = MIME_TYPE[format] + data = format_dict[mime_type] + output = PlotResult(data=data, mime_type=mime_type).dict() + figure_comm.send_result(data=output, metadata={"mime_type": mime_type}) else: logger.warning(f"Unhandled request: {request}") @@ -170,7 +181,7 @@ def _resize_pickled_figure( new_width_px: int = 614, new_height_px: int = 460, pixel_ratio: float = 1.0, - formats: list = ["image/png"], + formats: list = ["png"], ) -> dict: # Delay importing matplotlib until the kernel and shell has been # initialized otherwise the graphics backend will be reset to the gui @@ -215,11 +226,12 @@ def _resize_pickled_figure( # Render the figure to a buffer # using format_display_data() crops the figure to smaller than requested size - figure.savefig(figure_buffer, format="png") + figure.savefig(figure_buffer, format=formats[0]) figure_buffer.seek(0) image_data = base64.b64encode(figure_buffer.read()).decode() + key = MIME_TYPE[formats[0]] - format_dict = {"image/png": image_data} + format_dict = {key: image_data} plt.close(figure) diff --git a/extensions/positron-python/python_files/positron/positron_ipykernel/tests/test_plots.py b/extensions/positron-python/python_files/positron/positron_ipykernel/tests/test_plots.py index 8f978066790..6462d49cdb5 100644 --- a/extensions/positron-python/python_files/positron/positron_ipykernel/tests/test_plots.py +++ b/extensions/positron-python/python_files/positron/positron_ipykernel/tests/test_plots.py @@ -162,7 +162,7 @@ def test_hook_call(hook: PositronDisplayPublisherHook, images_path: Path) -> Non def render_request(comm_id: str, width_px: int = 500, height_px: int = 500, pixel_ratio: int = 1): return json_rpc_request( "render", - {"width": width_px, "height": height_px, "pixel_ratio": pixel_ratio}, + {"width": width_px, "height": height_px, "pixel_ratio": pixel_ratio, "format": "png"}, comm_id=comm_id, ) diff --git a/positron/comms/plot-backend-openrpc.json b/positron/comms/plot-backend-openrpc.json index ae607cc8cc9..5a80970a01e 100644 --- a/positron/comms/plot-backend-openrpc.json +++ b/positron/comms/plot-backend-openrpc.json @@ -30,6 +30,14 @@ "schema": { "type": "number" } + }, + { + "name": "format", + "description": "The requested plot format", + "schema": { + "type": "string" + }, + "required": false } ], "result": { diff --git a/src/vs/workbench/contrib/positronPlots/browser/modalDialogs/savePlotModalDialog.css b/src/vs/workbench/contrib/positronPlots/browser/modalDialogs/savePlotModalDialog.css index f0c7fabe429..e6314346abc 100644 --- a/src/vs/workbench/contrib/positronPlots/browser/modalDialogs/savePlotModalDialog.css +++ b/src/vs/workbench/contrib/positronPlots/browser/modalDialogs/savePlotModalDialog.css @@ -4,9 +4,10 @@ .plot-preview-input { height: 100%; - grid-template-rows: 1fr 2fr 4px 9fr; + grid-template-rows: 1fr 1fr 2fr 4px 9fr; grid-template-areas: "browse" + "file" "plot-input" "preview-progress" "preview"; @@ -28,6 +29,17 @@ grid-area: input; } +.plot-preview-input .file { + display: flex; + flex-direction: row; + column-gap: 10px; + align-items: flex-end; +} + +.plot-preview-input .file button { + margin-top: 4px; +} + .plot-input div.error { padding-top: 4px; grid-column: 1 / span 3; @@ -40,10 +52,14 @@ width: auto; } -.plot-preview-input .labeled-text-input input { +.plot-preview-input .plot-input .labeled-text-input input { width: 100px; } +.plot-preview-input .file .labeled-text-input input { + width: 200px; +} + .plot-preview-input .preview-progress { grid-area: preview-progress; display: flex; diff --git a/src/vs/workbench/contrib/positronPlots/browser/modalDialogs/savePlotModalDialog.tsx b/src/vs/workbench/contrib/positronPlots/browser/modalDialogs/savePlotModalDialog.tsx index 1903bcdfdb2..b52a3165ad4 100644 --- a/src/vs/workbench/contrib/positronPlots/browser/modalDialogs/savePlotModalDialog.tsx +++ b/src/vs/workbench/contrib/positronPlots/browser/modalDialogs/savePlotModalDialog.tsx @@ -7,7 +7,7 @@ import * as React from 'react'; import { localize } from 'vs/nls'; import { IWorkbenchLayoutService } from 'vs/workbench/services/layout/browser/layoutService'; import { IRenderedPlot, PlotClientInstance } from 'vs/workbench/services/languageRuntime/common/languageRuntimePlotClient'; -import { IFileDialogService } from 'vs/platform/dialogs/common/dialogs'; +import { IDialogService, IFileDialogService } from 'vs/platform/dialogs/common/dialogs'; import { URI } from 'vs/base/common/uri'; import { ProgressBar } from 'vs/base/browser/ui/positronComponents/progressBar'; import { LabeledTextInput } from 'vs/workbench/browser/positronComponents/positronModalDialog/components/labeledTextInput'; @@ -18,12 +18,23 @@ import { OKCancelActionBar } from 'vs/workbench/browser/positronComponents/posit import { PositronModalDialog } from 'vs/workbench/browser/positronComponents/positronModalDialog/positronModalDialog'; import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding'; import { PositronModalReactRenderer } from 'vs/workbench/browser/positronModalReactRenderer/positronModalReactRenderer'; +import { FileFilter } from 'electron'; +import { DropDownListBox } from 'vs/workbench/browser/positronComponents/dropDownListBox/dropDownListBox'; +import { DropDownListBoxItem } from 'vs/workbench/browser/positronComponents/dropDownListBox/dropDownListBoxItem'; +import { IFileService } from 'vs/platform/files/common/files'; export interface SavePlotOptions { uri: string; path: URI; } +export enum PlotFormat { + PNG = 'png', + SVG = 'svg', + PDF = 'pdf', + JPEG = 'jpeg', +} + const SAVE_PLOT_MODAL_DIALOG_WIDTH = 500; const SAVE_PLOT_MODAL_DIALOG_HEIGHT = 600; const BASE_DPI = 100; // matplotlib default DPI @@ -32,6 +43,8 @@ const BASE_DPI = 100; // matplotlib default DPI * Show the save plot modal dialog for dynamic plots. * @param layoutService the layout service for the modal * @param keybindingService the keybinding service to intercept shortcuts + * @param dialogService the dialog service to confirm the save + * @param fileService the file service to check if paths exist * @param fileDialogService the file dialog service to prompt where to save the plot * @param plotClient the dynamic plot client to render previews and the final image * @param savePlotCallback the action to take when the dialog closes @@ -40,6 +53,8 @@ const BASE_DPI = 100; // matplotlib default DPI export const showSavePlotModalDialog = ( layoutService: IWorkbenchLayoutService, keybindingService: IKeybindingService, + dialogService: IDialogService, + fileService: IFileService, fileDialogService: IFileDialogService, plotClient: PlotClientInstance, savePlotCallback: (options: SavePlotOptions) => void, @@ -58,7 +73,10 @@ export const showSavePlotModalDialog = ( renderer.render( { - const [path, setPath] = React.useState({ value: props.suggestedPath ?? URI.file(''), valid: true }); + const [directory, setDirectory] = React.useState({ value: props.suggestedPath ?? URI.file(''), valid: true }); + const [name, setName] = React.useState({ value: 'plot', valid: true }); + const [format, setFormat] = React.useState(PlotFormat.PNG); const [width, setWidth] = React.useState({ value: props.plotWidth, valid: true }); const [height, setHeight] = React.useState({ value: props.plotHeight, valid: true }); const [dpi, setDpi] = React.useState({ value: 100, valid: true }); @@ -89,13 +118,18 @@ const SavePlotModalDialog = (props: SavePlotModalDialogProps) => { const [rendering, setRendering] = React.useState(false); const inputRef = React.useRef(null); + const filterEntries: FileFilter[] = []; + for (const filter in PlotFormat) { + filterEntries.push({ extensions: [filter.toLowerCase()], name: filter.toUpperCase() }); + } + React.useEffect(() => { setUri(props.plotClient.lastRender?.uri ?? ''); }, [props.plotClient.lastRender?.uri]); const validateInput = React.useCallback((): boolean => { - return path.valid && width.valid && height.valid && dpi.valid; - }, [path, width, height, dpi]); + return directory.valid && width.valid && height.valid && dpi.valid && name.valid; + }, [directory, width, height, dpi, name]); React.useEffect(() => { validateInput(); @@ -117,38 +151,58 @@ const SavePlotModalDialog = (props: SavePlotModalDialogProps) => { }; const updatePath = (pathString: string) => { - const newPath = URI.file(pathString); - setPath({ value: newPath, valid: !!newPath }); + try { + const newPath = URI.file(pathString); + props.fileService.exists(newPath).then(exists => { + setDirectory({ + value: newPath, valid: exists, + errorMessage: exists ? undefined : localize('positron.savePlotModalDialog.pathDoesNotExist', "Path does not exist.") + }); + }); + } catch (error) { + setDirectory({ value: URI.file(''), valid: false, errorMessage: error.message }); + } }; const browseHandler = async () => { - const uri = await props.fileDialogService.showSaveDialog({ + const uri = await props.fileDialogService.showOpenDialog({ title: localize('positron.savePlotModalDialog.title', "Save Plot"), - filters: - [ - { - extensions: ['png'], - name: 'PNG', - }, - ], + defaultUri: directory.value, + openLabel: localize('positron.savePlotModalDialog.select', "Select"), + canSelectFiles: false, + canSelectFolders: true, + canSelectMany: false, }); - if (uri?.fsPath.length) { - setPath({ value: uri, valid: true }); + if (uri && uri.length > 0) { + updatePath(uri[0].fsPath); } }; const acceptHandler = async () => { if (validateInput()) { - setRendering(true); - const plotResult = await generatePreview(); - - if (plotResult) { - props.savePlotCallback({ uri: plotResult.uri, path: path.value }); + const fileExists = await props.fileService.exists(directory.value); + if (fileExists) { + const confirmation = await props.dialogService.confirm({ + message: localize('positron.savePlotModalDialog.fileExists', "The file already exists. Do you want to overwrite it?"), + primaryButton: localize('positron.savePlotModalDialog.overwrite', "Overwrite"), + cancelButton: localize('positron.savePlotModalDialog.cancel', "Cancel"), + }); + if (!confirmation.confirmed) { + return; + } } + setRendering(true); - setRendering(false); - props.renderer.dispose(); + generatePreview(format) + .then(async (plotResult) => { + const filePath = URI.joinPath(directory.value, `${name.value}.${format}`); + props.savePlotCallback({ uri: plotResult.uri, path: filePath }); + }) + .finally(() => { + setRendering(false); + props.renderer.dispose(); + }); } }; @@ -162,15 +216,15 @@ const SavePlotModalDialog = (props: SavePlotModalDialogProps) => { } setRendering(true); try { - const plotResult = await generatePreview(); + const plotResult = await generatePreview(PlotFormat.PNG); setUri(plotResult.uri); } finally { setRendering(false); } }; - const generatePreview = async (): Promise => { - return props.plotClient.preview(height.value, width.value, dpi.value / BASE_DPI); + const generatePreview = async (format: PlotFormat): Promise => { + return props.plotClient.preview(height.value, width.value, dpi.value / BASE_DPI, format); }; const previewButton = () => { @@ -195,16 +249,46 @@ const SavePlotModalDialog = (props: SavePlotModalDialogProps) => {
localize( - 'positron.savePlotModalDialog.path', - "Path" + 'positron.savePlotModalDialog.directory', + "Directory" ))()} - value={path.value.fsPath} + value={directory.value.fsPath} onChange={e => updatePath(e.target.value)} onBrowse={browseHandler} readOnlyInput={false} - error={!path.valid} + error={!directory.valid} inputRef={inputRef} />
+
+ localize( + 'positron.savePlotModalDialog.name', + "Name" + ))()} + value={name.value} + onChange={e => setName({ value: e.target.value, valid: !!e.target.value })} + error={!name.valid} + /> +
+ +
+
localize( @@ -242,9 +326,9 @@ const SavePlotModalDialog = (props: SavePlotModalDialogProps) => { />
- {!path.valid && (() => localize( - 'positron.savePlotModalDialog.noPathMessage', - "Specify a path." + {!directory.valid && (() => localize( + 'positron.savePlotModalDialog.invalidPathError', + "Invalid path: {0}", directory.errorMessage ))()}
@@ -259,6 +343,12 @@ const SavePlotModalDialog = (props: SavePlotModalDialogProps) => { "DPI must be between 1 and 300." ))()}
+
+ {!name.valid && (() => localize( + 'positron.savePlotModalDialog.invalidNameError', + "Plot name cannot be empty." + ))()} +
diff --git a/src/vs/workbench/contrib/positronPlots/browser/positronPlotsService.ts b/src/vs/workbench/contrib/positronPlots/browser/positronPlotsService.ts index 89eb57c9526..f2eaa68e6d4 100644 --- a/src/vs/workbench/contrib/positronPlots/browser/positronPlotsService.ts +++ b/src/vs/workbench/contrib/positronPlots/browser/positronPlotsService.ts @@ -24,14 +24,14 @@ import { WebviewPlotClient } from 'vs/workbench/contrib/positronPlots/browser/we import { IPositronNotebookOutputWebviewService } from 'vs/workbench/contrib/positronOutputWebview/browser/notebookOutputWebviewService'; import { IPositronIPyWidgetsService } from 'vs/workbench/services/positronIPyWidgets/common/positronIPyWidgetsService'; import { Schemas } from 'vs/base/common/network'; -import { IFileDialogService } from 'vs/platform/dialogs/common/dialogs'; +import { IDialogService, IFileDialogService } from 'vs/platform/dialogs/common/dialogs'; import { decodeBase64 } from 'vs/base/common/buffer'; import { SavePlotOptions, showSavePlotModalDialog } from 'vs/workbench/contrib/positronPlots/browser/modalDialogs/savePlotModalDialog'; import { IWorkbenchLayoutService } from 'vs/workbench/services/layout/browser/layoutService'; -import { URI } from 'vs/base/common/uri'; import { IWorkspaceContextService } from 'vs/platform/workspace/common/workspace'; import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding'; import { IClipboardService } from 'vs/platform/clipboard/common/clipboardService'; +import { localize } from 'vs/nls'; /** The maximum number of recent executions to store. */ const MaxRecentExecutions = 10; @@ -109,7 +109,8 @@ export class PositronPlotsService extends Disposable implements IPositronPlotsSe @IWorkspaceContextService private readonly _workspaceContextService: IWorkspaceContextService, @IWorkbenchLayoutService private readonly _layoutService: IWorkbenchLayoutService, @IKeybindingService private readonly _keybindingService: IKeybindingService, - @IClipboardService private _clipboardService: IClipboardService) { + @IClipboardService private _clipboardService: IClipboardService, + @IDialogService private readonly _dialogService: IDialogService) { super(); // Register for language runtime service startups @@ -682,7 +683,7 @@ export class PositronPlotsService extends Disposable implements IPositronPlotsSe if (this._selectedPlotId) { const plot = this._plots.find(plot => plot.id === this._selectedPlotId); const workspaceFolder = this._workspaceContextService.getWorkspace().folders[0]?.uri; - const suggestedPath = workspaceFolder ? URI.joinPath(workspaceFolder, 'plot.png') : undefined; + const suggestedPath = workspaceFolder ?? undefined; if (plot) { let uri = ''; @@ -692,7 +693,7 @@ export class PositronPlotsService extends Disposable implements IPositronPlotsSe this.showSavePlotDialog(uri); } else if (plot instanceof PlotClientInstance) { // if it's a dynamic plot, present options dialog - showSavePlotModalDialog(this._layoutService, this._keybindingService, this._fileDialogService, plot, this.savePlotAs, suggestedPath); + showSavePlotModalDialog(this._layoutService, this._keybindingService, this._dialogService, this._fileService, this._fileDialogService, plot, this.savePlotAs, suggestedPath); } else { // if it's a webview plot, do nothing return; @@ -712,7 +713,8 @@ export class PositronPlotsService extends Disposable implements IPositronPlotsSe const data = matches[2]; htmlFileSystemProvider.writeFile(options.path, decodeBase64(data).buffer, { create: true, overwrite: true, unlock: true, atomic: false }) - .then(() => { + .catch((error: Error) => { + this._dialogService.error(localize('positronPlotsService.savePlotError.unknown', 'Error saving plot: {0}', error.message)); }); }; diff --git a/src/vs/workbench/services/languageRuntime/common/languageRuntimePlotClient.ts b/src/vs/workbench/services/languageRuntime/common/languageRuntimePlotClient.ts index 97baccfb878..5b19883716f 100644 --- a/src/vs/workbench/services/languageRuntime/common/languageRuntimePlotClient.ts +++ b/src/vs/workbench/services/languageRuntime/common/languageRuntimePlotClient.ts @@ -76,6 +76,9 @@ interface RenderRequest { /** The pixel ratio of the device for which the plot was rendered */ pixel_ratio: number; + + /** The format of the plot */ + format: string; } /** @@ -246,9 +249,10 @@ export class PlotClientInstance extends Disposable implements IPositronPlotClien * @param height The plot height, in pixels * @param width The plot width, in pixels * @param pixel_ratio The device pixel ratio (e.g. 1 for standard displays, 2 for retina displays) + * @param format The format of the plot ('png', 'svg') * @returns A promise that resolves to a rendered image, or rejects with an error. */ - public render(height: number, width: number, pixel_ratio: number): Promise { + public render(height: number, width: number, pixel_ratio: number, format = 'png'): Promise { // Deal with whole pixels only height = Math.floor(height); width = Math.floor(width); @@ -269,7 +273,8 @@ export class PlotClientInstance extends Disposable implements IPositronPlotClien const request: RenderRequest = { height, width, - pixel_ratio + pixel_ratio, + format }; const deferred = new DeferredRender(request); @@ -309,7 +314,7 @@ export class PlotClientInstance extends Disposable implements IPositronPlotClien * @param pixel_ratio The device pixel ratio (e.g. 1 for standard displays, 2 for retina displays) * @returns A promise that resolves when the render request is scheduled, or rejects with an error. */ - public preview(height: number, width: number, pixel_ratio: number): Promise { + public preview(height: number, width: number, pixel_ratio: number, format: string): Promise { // Deal with whole pixels only height = Math.floor(height); width = Math.floor(width); @@ -318,7 +323,8 @@ export class PlotClientInstance extends Disposable implements IPositronPlotClien const request: RenderRequest = { height, width, - pixel_ratio + pixel_ratio, + format }; const deferred = new DeferredRender(request); @@ -384,7 +390,8 @@ export class PlotClientInstance extends Disposable implements IPositronPlotClien const renderRequest = request.renderRequest; this._comm.render(renderRequest.height, renderRequest.width, - renderRequest.pixel_ratio).then((response) => { + renderRequest.pixel_ratio, + renderRequest.format).then((response) => { // Ignore if the request was cancelled or already fulfilled if (!request.isComplete) { @@ -490,7 +497,8 @@ export class PlotClientInstance extends Disposable implements IPositronPlotClien const req = new DeferredRender({ height: Math.floor(height!), width: Math.floor(width!), - pixel_ratio: pixel_ratio! + pixel_ratio: pixel_ratio!, + format: this._currentRender?.renderRequest.format ?? 'png' }); this.scheduleRender(req, 0); diff --git a/src/vs/workbench/services/languageRuntime/common/positronPlotComm.ts b/src/vs/workbench/services/languageRuntime/common/positronPlotComm.ts index e263c113b06..95836a370f3 100644 --- a/src/vs/workbench/services/languageRuntime/common/positronPlotComm.ts +++ b/src/vs/workbench/services/languageRuntime/common/positronPlotComm.ts @@ -51,11 +51,12 @@ export class PositronPlotComm extends PositronBaseComm { * @param height The requested plot height, in pixels * @param width The requested plot width, in pixels * @param pixelRatio The pixel ratio of the display device + * @param format The requested plot format * * @returns A rendered plot */ - render(height: number, width: number, pixelRatio: number): Promise { - return super.performRpc('render', ['height', 'width', 'pixel_ratio'], [height, width, pixelRatio]); + render(height: number, width: number, pixelRatio: number, format: string): Promise { + return super.performRpc('render', ['height', 'width', 'pixel_ratio', 'format'], [height, width, pixelRatio, format]); }