Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add table display name to Data Explorer get_state RPC, show variable name in editor tab label #2775

Merged
merged 2 commits into from
Apr 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import abc
import logging
import operator
import uuid
from typing import (
TYPE_CHECKING,
Callable,
Expand Down Expand Up @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -948,14 +960,16 @@ 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
# the same variables inspection logic to resolve it here.
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

Expand All @@ -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
Expand All @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"
Expand All @@ -30,10 +30,6 @@
# pytest fixtures


def guid():
return str(uuid.uuid4())


def get_new_comm(
de_service: DataExplorerService,
table: Any,
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -260,6 +272,10 @@ def alias_home(path: Path) -> Path:
return path


def guid():
return str(uuid.uuid4())


def positron_ipykernel_usage():
"""

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
5 changes: 5 additions & 0 deletions positron/comms/data_explorer-backend-openrpc.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -25,6 +24,8 @@ export class PositronDataExplorerEditorInput extends EditorInput {

//#endregion Static Properties

_name: string = 'Data Explorer';

//#region Constructor & Dispose

/**
Expand Down Expand Up @@ -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();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
});

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`);
Expand Down
Loading