From c8d9b1d1959cf97f8679d28466d141253217fed2 Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Mon, 15 Apr 2024 16:16:37 -0500 Subject: [PATCH 1/2] Add table display name to get_state RPC, show in editor tab label --- .../positron_ipykernel/data_explorer.py | 33 ++++++++++++++----- .../positron_ipykernel/data_explorer_comm.py | 4 +++ .../tests/test_data_explorer.py | 8 ++--- .../positron/positron_ipykernel/utils.py | 18 +++++++++- .../positron/positron_ipykernel/variables.py | 8 ++++- .../comms/data_explorer-backend-openrpc.json | 5 +++ .../positronDataExplorer.tsx | 1 + ...positronDataExplorerEditor.contribution.ts | 1 + .../browser/positronDataExplorerEditor.tsx | 4 +++ .../positronDataExplorerEditorInput.ts | 11 +++++-- .../common/positronDataExplorerComm.ts | 5 +++ .../browser/positronDataExplorerService.ts | 11 ++++++- 12 files changed, 90 insertions(+), 19 deletions(-) diff --git a/extensions/positron-python/python_files/positron/positron_ipykernel/data_explorer.py b/extensions/positron-python/python_files/positron/positron_ipykernel/data_explorer.py index c88c33e724c..773c26ca0c5 100644 --- a/extensions/positron-python/python_files/positron/positron_ipykernel/data_explorer.py +++ b/extensions/positron-python/python_files/positron/positron_ipykernel/data_explorer.py @@ -8,7 +8,6 @@ import abc import logging import operator -import uuid from typing import ( TYPE_CHECKING, Callable, @@ -63,6 +62,8 @@ ) from .positron_comm import CommMessage, PositronComm from .third_party import pd_ +from .utils import guid + if TYPE_CHECKING: import pandas as pd @@ -87,10 +88,13 @@ class DataExplorerTableView(abc.ABC): def __init__( self, + display_name: str, table, filters: Optional[List[RowFilter]], sort_keys: Optional[List[ColumnSortKey]], ): + self.display_name = display_name + # Note: we must not ever modify the user's data self.table = table @@ -269,11 +273,12 @@ class PandasView(DataExplorerTableView): def __init__( self, + display_name: str, table, filters: Optional[List[RowFilter]], sort_keys: Optional[List[ColumnSortKey]], ): - super().__init__(table, filters, sort_keys) + super().__init__(display_name, table, filters, sort_keys) self._dtypes = None @@ -731,6 +736,7 @@ def _get_state(self) -> BackendState: num_rows = self.table.shape[0] return BackendState( + display_name=self.display_name, table_shape=TableShape(num_rows=num_rows, num_columns=self.table.shape[1]), row_filters=self.filters, sort_keys=self.sort_keys, @@ -762,8 +768,9 @@ class PyArrowView(DataExplorerTableView): pass -def _get_table_view(table, filters=None, sort_keys=None): - return PandasView(table, filters, sort_keys) +def _get_table_view(table, filters=None, sort_keys=None, name=None): + name = name or guid() + return PandasView(name, table, filters, sort_keys) def _value_type_is_supported(value): @@ -828,9 +835,14 @@ def register_table( raise TypeError(type(table)) if comm_id is None: - comm_id = str(uuid.uuid4()) + comm_id = guid() + + if variable_path is not None: + full_title = ", ".join([str(decode_access_key(k)) for k in variable_path]) + else: + full_title = title - self.table_views[comm_id] = _get_table_view(table) + self.table_views[comm_id] = _get_table_view(table, name=full_title) base_comm = comm.create_comm( target_name=self.comm_target, @@ -948,6 +960,8 @@ def _update_explorer_for_comm(self, comm_id: str, path: PathKey, new_variable): comm = self.comms[comm_id] table_view = self.table_views[comm_id] + full_title = ", ".join([str(decode_access_key(k)) for k in path]) + # When detecting namespace assignments or changes, the first # level of the path has already been resolved. If there is a # data explorer open for a nested value, then we need to use @@ -955,7 +969,7 @@ def _update_explorer_for_comm(self, comm_id: str, path: PathKey, new_variable): if len(path) > 1: is_found, new_table = _resolve_value_from_path(new_variable, path[1:]) if not is_found: - raise KeyError(f"Path {', '.join(path)} not found in value") + raise KeyError(f"Path {full_title} not found in value") else: new_table = new_variable @@ -980,7 +994,7 @@ def _fire_schema_update(discard_state=False): # start over. At some point we can return here and # selectively preserve state if we feel it is safe enough # to do so. - self.table_views[comm_id] = _get_table_view(new_table) + self.table_views[comm_id] = _get_table_view(new_table, name=full_title) return _fire_schema_update(discard_state=True) # New value for data explorer is the same. For now, we just @@ -1004,12 +1018,13 @@ def _fire_schema_update(discard_state=False): ) = table_view.ui_should_update_schema(new_table) if should_discard_state: - self.table_views[comm_id] = _get_table_view(new_table) + self.table_views[comm_id] = _get_table_view(new_table, name=full_title) else: self.table_views[comm_id] = _get_table_view( new_table, filters=table_view.filters, sort_keys=table_view.sort_keys, + name=full_title, ) if should_update_schema: diff --git a/extensions/positron-python/python_files/positron/positron_ipykernel/data_explorer_comm.py b/extensions/positron-python/python_files/positron/positron_ipykernel/data_explorer_comm.py index fd06b9ad1d1..1cc8132ee97 100644 --- a/extensions/positron-python/python_files/positron/positron_ipykernel/data_explorer_comm.py +++ b/extensions/positron-python/python_files/positron/positron_ipykernel/data_explorer_comm.py @@ -172,6 +172,10 @@ class BackendState(BaseModel): The current backend state for the data explorer """ + display_name: str = Field( + description="Variable name or other string to display for tab name in UI", + ) + table_shape: TableShape = Field( description="Provides number of rows and columns in table", ) diff --git a/extensions/positron-python/python_files/positron/positron_ipykernel/tests/test_data_explorer.py b/extensions/positron-python/python_files/positron/positron_ipykernel/tests/test_data_explorer.py index 76c7e271ded..53deba30512 100644 --- a/extensions/positron-python/python_files/positron/positron_ipykernel/tests/test_data_explorer.py +++ b/extensions/positron-python/python_files/positron/positron_ipykernel/tests/test_data_explorer.py @@ -2,7 +2,6 @@ # Copyright (C) 2023-2024 Posit Software, PBC. All rights reserved. # -import uuid from typing import Any, Dict, List, Optional, Type, cast import numpy as np @@ -22,6 +21,7 @@ ) from .conftest import DummyComm, PositronShell from .test_variables import BIG_ARRAY_LENGTH +from ..utils import guid from .utils import json_rpc_notification, json_rpc_request, json_rpc_response TARGET_NAME = "positron.dataExplorer" @@ -30,10 +30,6 @@ # pytest fixtures -def guid(): - return str(uuid.uuid4()) - - def get_new_comm( de_service: DataExplorerService, table: Any, @@ -261,6 +257,7 @@ def test_explorer_variable_updates( dxf = DataExplorerFixture(de_service) new_state = dxf.get_state("x") + assert new_state["display_name"] == "x" assert new_state["table_shape"]["num_rows"] == 5 assert new_state["table_shape"]["num_columns"] == 1 assert new_state["sort_keys"] == [ColumnSortKey(**k) for k in x_sort_keys] @@ -441,6 +438,7 @@ def _wrap_json(model: Type[BaseModel], data: JsonRecords): def test_pandas_get_state(dxf: DataExplorerFixture): result = dxf.get_state("simple") + assert result["display_name"] == "simple" assert result["table_shape"]["num_rows"] == 5 assert result["table_shape"]["num_columns"] == 6 diff --git a/extensions/positron-python/python_files/positron/positron_ipykernel/utils.py b/extensions/positron-python/python_files/positron/positron_ipykernel/utils.py index 710bbe39d26..6cd031f5041 100644 --- a/extensions/positron-python/python_files/positron/positron_ipykernel/utils.py +++ b/extensions/positron-python/python_files/positron/positron_ipykernel/utils.py @@ -8,10 +8,22 @@ import pprint import sys import types +import uuid from binascii import b2a_base64 from datetime import datetime from pathlib import Path -from typing import Any, Coroutine, Dict, List, Optional, Set, Tuple, TypeVar, Union, cast +from typing import ( + Any, + Coroutine, + Dict, + List, + Optional, + Set, + Tuple, + TypeVar, + Union, + cast, +) JsonData = Union[Dict[str, "JsonData"], List["JsonData"], str, int, float, bool, None] JsonRecord = Dict[str, JsonData] @@ -260,6 +272,10 @@ def alias_home(path: Path) -> Path: return path +def guid(): + return str(uuid.uuid4()) + + def positron_ipykernel_usage(): """ diff --git a/extensions/positron-python/python_files/positron/positron_ipykernel/variables.py b/extensions/positron-python/python_files/positron/positron_ipykernel/variables.py index a9d2b514436..503bc5e35a5 100644 --- a/extensions/positron-python/python_files/positron/positron_ipykernel/variables.py +++ b/extensions/positron-python/python_files/positron/positron_ipykernel/variables.py @@ -15,7 +15,13 @@ from .access_keys import decode_access_key, encode_access_key from .inspectors import get_inspector from .positron_comm import CommMessage, JsonRpcErrorCode, PositronComm -from .utils import JsonData, JsonRecord, cancel_tasks, create_task, get_qualname +from .utils import ( + JsonData, + JsonRecord, + cancel_tasks, + create_task, + get_qualname, +) from .variables_comm import ( ClearRequest, ClipboardFormatFormat, diff --git a/positron/comms/data_explorer-backend-openrpc.json b/positron/comms/data_explorer-backend-openrpc.json index a41ec4e57df..8449a63ba5b 100644 --- a/positron/comms/data_explorer-backend-openrpc.json +++ b/positron/comms/data_explorer-backend-openrpc.json @@ -243,12 +243,17 @@ "name": "backend_state", "description": "The current backend state for the data explorer", "required": [ + "display_name", "table_shape", "row_filters", "sort_keys", "supported_features" ], "properties": { + "display_name": { + "type": "string", + "description": "Variable name or other string to display for tab name in UI" + }, "table_shape": { "type": "object", "name": "table_shape", diff --git a/src/vs/workbench/browser/positronDataExplorer/positronDataExplorer.tsx b/src/vs/workbench/browser/positronDataExplorer/positronDataExplorer.tsx index ea52a86a22d..a72663a184e 100644 --- a/src/vs/workbench/browser/positronDataExplorer/positronDataExplorer.tsx +++ b/src/vs/workbench/browser/positronDataExplorer/positronDataExplorer.tsx @@ -17,6 +17,7 @@ import { PositronActionBarServices } from 'vs/platform/positronActionBar/browser import { PositronDataExplorerContextProvider } from 'vs/workbench/browser/positronDataExplorer/positronDataExplorerContext'; import { DataExplorerPanel } from 'vs/workbench/browser/positronDataExplorer/components/dataExplorerPanel/dataExplorerPanel'; import { IPositronDataExplorerInstance } from 'vs/workbench/services/positronDataExplorer/browser/interfaces/positronDataExplorerInstance'; +import { PositronDataExplorerEditorInput } from 'vs/workbench/contrib/positronDataExplorerEditor/browser/positronDataExplorerEditorInput'; /** * PositronDataExplorerServices interface. diff --git a/src/vs/workbench/contrib/positronDataExplorerEditor/browser/positronDataExplorerEditor.contribution.ts b/src/vs/workbench/contrib/positronDataExplorerEditor/browser/positronDataExplorerEditor.contribution.ts index 9a4b49f5fa2..6e1e5ab2fa5 100644 --- a/src/vs/workbench/contrib/positronDataExplorerEditor/browser/positronDataExplorerEditor.contribution.ts +++ b/src/vs/workbench/contrib/positronDataExplorerEditor/browser/positronDataExplorerEditor.contribution.ts @@ -31,6 +31,7 @@ class PositronDataExplorerContribution extends Disposable { `${Schemas.positronDataExplorer}:**/**`, { id: PositronDataExplorerEditorInput.EditorID, + // Label will be overwritten elsewhere label: localize('positronDataExplorer', "Positron Data Explorer"), priority: RegisteredEditorPriority.builtin }, diff --git a/src/vs/workbench/contrib/positronDataExplorerEditor/browser/positronDataExplorerEditor.tsx b/src/vs/workbench/contrib/positronDataExplorerEditor/browser/positronDataExplorerEditor.tsx index ba68278d6aa..16a5c335eb6 100644 --- a/src/vs/workbench/contrib/positronDataExplorerEditor/browser/positronDataExplorerEditor.tsx +++ b/src/vs/workbench/contrib/positronDataExplorerEditor/browser/positronDataExplorerEditor.tsx @@ -278,6 +278,10 @@ export class PositronDataExplorerEditor extends EditorPane implements IReactComp // Logging. console.log(`PositronDataExplorerEditor ${this._instance} create PositronReactRenderer`); + // Hack -- this is usually set by setInput but we're setting it temporarily to be + // able to edit the editor tab name + this._input = input; + // Success. return; } diff --git a/src/vs/workbench/contrib/positronDataExplorerEditor/browser/positronDataExplorerEditorInput.ts b/src/vs/workbench/contrib/positronDataExplorerEditor/browser/positronDataExplorerEditorInput.ts index c5ca723e796..ca9d3f666fa 100644 --- a/src/vs/workbench/contrib/positronDataExplorerEditor/browser/positronDataExplorerEditorInput.ts +++ b/src/vs/workbench/contrib/positronDataExplorerEditor/browser/positronDataExplorerEditorInput.ts @@ -2,7 +2,6 @@ * Copyright (C) 2023-2024 Posit Software, PBC. All rights reserved. *--------------------------------------------------------------------------------------------*/ -import { localize } from 'vs/nls'; import { URI } from 'vs/base/common/uri'; import { IUntypedEditorInput } from 'vs/workbench/common/editor'; import { EditorInput } from 'vs/workbench/common/editor/editorInput'; @@ -25,6 +24,8 @@ export class PositronDataExplorerEditorInput extends EditorInput { //#endregion Static Properties + _name: string = 'Data Explorer'; + //#region Constructor & Dispose /** @@ -67,7 +68,13 @@ export class PositronDataExplorerEditorInput extends EditorInput { * @returns The display name of this input. */ override getName(): string { - return localize('positronDataExplorerInputName', "Positron Data Explorer"); + // This is where the tab name comes from + return this._name; + } + + setName(name: string) { + this._name = name; + this._onDidChangeLabel.fire(); } /** diff --git a/src/vs/workbench/services/languageRuntime/common/positronDataExplorerComm.ts b/src/vs/workbench/services/languageRuntime/common/positronDataExplorerComm.ts index a5b609e6506..545b3da086c 100644 --- a/src/vs/workbench/services/languageRuntime/common/positronDataExplorerComm.ts +++ b/src/vs/workbench/services/languageRuntime/common/positronDataExplorerComm.ts @@ -57,6 +57,11 @@ export interface FilterResult { * The current backend state for the data explorer */ export interface BackendState { + /** + * Variable name or other string to display for tab name in UI + */ + display_name: string; + /** * Provides number of rows and columns in table */ diff --git a/src/vs/workbench/services/positronDataExplorer/browser/positronDataExplorerService.ts b/src/vs/workbench/services/positronDataExplorer/browser/positronDataExplorerService.ts index 10093a603cc..221cd8e3da6 100644 --- a/src/vs/workbench/services/positronDataExplorer/browser/positronDataExplorerService.ts +++ b/src/vs/workbench/services/positronDataExplorer/browser/positronDataExplorerService.ts @@ -192,10 +192,19 @@ class PositronDataExplorerService extends Disposable implements IPositronDataExp const start = new Date(); // Open the editor. - await this._editorService.openEditor({ + const editor = await this._editorService.openEditor({ resource: PositronDataExplorerUri.generate(dataExplorerClientInstance.identifier) }); + // PositronDataExplorerEditorInput + dataExplorerClientInstance.getState().then((state) => { + // Hack to be able to call PositronDataExplorerEditorInput.setName without eslint errors; + const dxInput = editor?.input as any; + if (state.display_name !== undefined) { + dxInput.setName?.(`Data: ${state.display_name}`); + } + }); + const end = new Date(); console.log(`this._editorService.openEditor took ${end.getTime() - start.getTime()}ms`); From 6c7b4004bd0e181ac4035276b7abf1fd5c5258c4 Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Mon, 15 Apr 2024 16:35:45 -0500 Subject: [PATCH 2/2] flakes --- .../browser/positronDataExplorer/positronDataExplorer.tsx | 1 - .../browser/positronDataExplorerService.ts | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/vs/workbench/browser/positronDataExplorer/positronDataExplorer.tsx b/src/vs/workbench/browser/positronDataExplorer/positronDataExplorer.tsx index a72663a184e..ea52a86a22d 100644 --- a/src/vs/workbench/browser/positronDataExplorer/positronDataExplorer.tsx +++ b/src/vs/workbench/browser/positronDataExplorer/positronDataExplorer.tsx @@ -17,7 +17,6 @@ import { PositronActionBarServices } from 'vs/platform/positronActionBar/browser import { PositronDataExplorerContextProvider } from 'vs/workbench/browser/positronDataExplorer/positronDataExplorerContext'; import { DataExplorerPanel } from 'vs/workbench/browser/positronDataExplorer/components/dataExplorerPanel/dataExplorerPanel'; import { IPositronDataExplorerInstance } from 'vs/workbench/services/positronDataExplorer/browser/interfaces/positronDataExplorerInstance'; -import { PositronDataExplorerEditorInput } from 'vs/workbench/contrib/positronDataExplorerEditor/browser/positronDataExplorerEditorInput'; /** * PositronDataExplorerServices interface. diff --git a/src/vs/workbench/services/positronDataExplorer/browser/positronDataExplorerService.ts b/src/vs/workbench/services/positronDataExplorer/browser/positronDataExplorerService.ts index 221cd8e3da6..4c0be3f3a80 100644 --- a/src/vs/workbench/services/positronDataExplorer/browser/positronDataExplorerService.ts +++ b/src/vs/workbench/services/positronDataExplorer/browser/positronDataExplorerService.ts @@ -196,9 +196,9 @@ class PositronDataExplorerService extends Disposable implements IPositronDataExp resource: PositronDataExplorerUri.generate(dataExplorerClientInstance.identifier) }); - // PositronDataExplorerEditorInput dataExplorerClientInstance.getState().then((state) => { - // Hack to be able to call PositronDataExplorerEditorInput.setName without eslint errors; + // Hack to be able to call PositronDataExplorerEditorInput.setName without + // eslint errors; const dxInput = editor?.input as any; if (state.display_name !== undefined) { dxInput.setName?.(`Data: ${state.display_name}`);