diff --git a/extensions/positron-r/package.json b/extensions/positron-r/package.json index d9b215965c5..0149720b0a0 100644 --- a/extensions/positron-r/package.json +++ b/extensions/positron-r/package.json @@ -635,7 +635,7 @@ }, "positron": { "binaryDependencies": { - "ark": "0.1.147" + "ark": "0.1.148" }, "minimumRVersion": "4.2.0", "minimumRenvVersion": "1.0.9" diff --git a/src/vs/workbench/services/languageRuntime/common/languageRuntimeDataExplorerClient.ts b/src/vs/workbench/services/languageRuntime/common/languageRuntimeDataExplorerClient.ts index 1592782a4f7..bc26fc96d87 100644 --- a/src/vs/workbench/services/languageRuntime/common/languageRuntimeDataExplorerClient.ts +++ b/src/vs/workbench/services/languageRuntime/common/languageRuntimeDataExplorerClient.ts @@ -402,9 +402,9 @@ export class DataExplorerClientInstance extends Disposable { this._asyncTasks.set(callbackId, promise); await this._backendClient.getColumnProfiles(callbackId, profiles, this._profileFormatOptions); - const timeout = 10000; + const timeout = 60000; - // Don't leave unfulfilled promise indefinitely; reject after 10 seconds pass + // Don't leave unfulfilled promise indefinitely; reject after one minute // for now just in case setTimeout(() => { // If the promise has already been resolved, do nothing. diff --git a/src/vs/workbench/services/positronDataExplorer/browser/tableSummaryDataGridInstance.tsx b/src/vs/workbench/services/positronDataExplorer/browser/tableSummaryDataGridInstance.tsx index 90d6ac50b93..f5239ad45e1 100644 --- a/src/vs/workbench/services/positronDataExplorer/browser/tableSummaryDataGridInstance.tsx +++ b/src/vs/workbench/services/positronDataExplorer/browser/tableSummaryDataGridInstance.tsx @@ -33,11 +33,6 @@ const PROFILE_LINE_HEIGHT = 20; export class TableSummaryDataGridInstance extends DataGridInstance { //#region Private Properties - /** - * Gets or sets the last row filters. - */ - private _lastRowFilters: string = '[]'; - /** * The onDidSelectColumn event emitter. */ @@ -132,18 +127,8 @@ export class TableSummaryDataGridInstance extends DataGridInstance { // Update the layout entries. await updateLayoutEntries(state); - // Stringify the row filters. - const rowFilters = JSON.stringify(state.row_filters); - - // If the row filters have changed, refresh the column profiles because they rely on the - // data - if (this._lastRowFilters !== rowFilters) { - this._lastRowFilters = rowFilters; - await this._tableSummaryCache.refreshColumnProfiles(); - } - - // Fetch data. - await this.fetchData(true); + // Invalidate cache and fetch data, profiles + await this.fetchData(/* invalidateCache=*/true); })); // Add the table summary cache onDidUpdate event handler. diff --git a/src/vs/workbench/services/positronDataExplorer/common/tableSummaryCache.ts b/src/vs/workbench/services/positronDataExplorer/common/tableSummaryCache.ts index d27e32efa79..d98b8f57b27 100644 --- a/src/vs/workbench/services/positronDataExplorer/common/tableSummaryCache.ts +++ b/src/vs/workbench/services/positronDataExplorer/common/tableSummaryCache.ts @@ -319,109 +319,134 @@ export class TableSummaryCache extends Disposable { const histogramSupported = this.isHistogramSupported(); const frequencyTableSupported = this.isFrequencyTableSupported(); - // Load the column profiles. - const columnProfileResults = await this._dataExplorerClientInstance.getColumnProfiles( - columnIndices.map((column_index): ColumnProfileRequest => { - // Get the column schema. - const columnSchema = this._columnSchemaCache.get(column_index); - - // Build the array of column profiles to load. Always load the null count. - const profiles: ColumnProfileSpec[] = [{ - profile_type: ColumnProfileType.NullCount - }]; - - // Determine whether the column is expanded. - const columnExpanded = this._expandedColumns.has(column_index); - - // If the column is expanded, load the summary stats. - if (columnExpanded) { - profiles.push({ profile_type: ColumnProfileType.SummaryStats }); - } + const columnRequests = columnIndices.map((column_index): ColumnProfileRequest => { + // Get the column schema. + const columnSchema = this._columnSchemaCache.get(column_index); + + // Build the array of column profiles to load. Always load the null count. + const profiles: ColumnProfileSpec[] = [{ + profile_type: ColumnProfileType.NullCount + }]; + + // Determine whether the column is expanded. + const columnExpanded = this._expandedColumns.has(column_index); + + // If the column is expanded, load the summary stats. + if (columnExpanded) { + profiles.push({ profile_type: ColumnProfileType.SummaryStats }); + } - // Determine whether to load the histogram or the frequency table for the column. - switch (columnSchema?.type_display) { - // Number. - case ColumnDisplayType.Number: { - // If histograms are supported, load them. - if (histogramSupported) { - // Load the small histogram. + // Determine whether to load the histogram or the frequency table for the column. + switch (columnSchema?.type_display) { + // Number. + case ColumnDisplayType.Number: { + // If histograms are supported, load them. + if (histogramSupported) { + // Load the small histogram. + profiles.push({ + profile_type: ColumnProfileType.SmallHistogram, + params: { + method: ColumnHistogramParamsMethod.FreedmanDiaconis, + num_bins: SMALL_HISTOGRAM_NUM_BINS, + } + }); + + // If the column is expanded, load the large histogram. + if (columnExpanded) { profiles.push({ - profile_type: ColumnProfileType.SmallHistogram, + profile_type: ColumnProfileType.LargeHistogram, params: { method: ColumnHistogramParamsMethod.FreedmanDiaconis, - num_bins: SMALL_HISTOGRAM_NUM_BINS, + num_bins: LARGE_HISTOGRAM_NUM_BINS, } }); - - // If the column is expanded, load the large histogram. - if (columnExpanded) { - profiles.push({ - profile_type: ColumnProfileType.LargeHistogram, - params: { - method: ColumnHistogramParamsMethod.FreedmanDiaconis, - num_bins: LARGE_HISTOGRAM_NUM_BINS, - } - }); - } } - break; } + break; + } - // Boolean. - case ColumnDisplayType.Boolean: { - // If frequency tables are supported, load them. - if (frequencyTableSupported) { - // Load the small frequency table. Note that we do not load the large - // frequency table because there are only two possible values. - profiles.push({ - profile_type: ColumnProfileType.SmallFrequencyTable, - params: { - limit: 2 - } - }); + // Boolean. + case ColumnDisplayType.Boolean: { + // If frequency tables are supported, load them. + if (frequencyTableSupported) { + // Load the small frequency table. Note that we do not load the large + // frequency table because there are only two possible values. + profiles.push({ + profile_type: ColumnProfileType.SmallFrequencyTable, + params: { + limit: 2 + } + }); - } - break; } + break; + } - // String. - case ColumnDisplayType.String: { - // If frequency tables are supported, load them. - if (frequencyTableSupported) { - // Load the small frequency table. + // String. + case ColumnDisplayType.String: { + // If frequency tables are supported, load them. + if (frequencyTableSupported) { + // Load the small frequency table. + profiles.push({ + profile_type: ColumnProfileType.SmallFrequencyTable, + params: { + limit: SMALL_FREQUENCY_TABLE_LIMIT + } + }); + + // If the column is expanded, load the large frequency table. + if (columnExpanded) { profiles.push({ - profile_type: ColumnProfileType.SmallFrequencyTable, + profile_type: ColumnProfileType.LargeFrequencyTable, params: { - limit: SMALL_FREQUENCY_TABLE_LIMIT + limit: LARGE_FREQUENCY_TABLE_LIMIT } }); - - // If the column is expanded, load the large frequency table. - if (columnExpanded) { - profiles.push({ - profile_type: ColumnProfileType.LargeFrequencyTable, - params: { - limit: LARGE_FREQUENCY_TABLE_LIMIT - } - }); - } } - break; } + break; } + } - // Return the column profile request. - return { column_index, profiles }; - }) - ); + // Return the column profile request. + return { column_index, profiles }; + }); - // Cache the column profiles that were returned. - for (let i = 0; i < columnProfileResults.length; i++) { - this._columnProfileCache.set(columnIndices[i], columnProfileResults[i]); - } + const tableState = await this._dataExplorerClientInstance.getBackendState(); - // Fire the onDidUpdate event. - this._onDidUpdateEmitter.fire(); + // For more than 10 million rows, we request profiles one by one rather than as a batch for + // better responsiveness + const BATCHING_THRESHOLD = 5_000_000; + if (tableState.table_shape.num_rows > BATCHING_THRESHOLD) { + const BATCH_SIZE = 4; + for (let i = 0; i < columnIndices.length; i += BATCH_SIZE) { + // Get the next batch of up to 4 requests + const batchColumnRequests = columnRequests.slice(i, i + BATCH_SIZE); + const batchColumnIndices = columnIndices.slice(i, i + BATCH_SIZE); + + // Send the batch of requests to getColumnProfiles + const results = await this._dataExplorerClientInstance.getColumnProfiles(batchColumnRequests); + + // Cache the returned column profiles for each index in the batch + for (let j = 0; j < results.length; j++) { + this._columnProfileCache.set(batchColumnIndices[j], results[j]); + } + + // Fire the onDidUpdate event so things update as soon as they are returned + this._onDidUpdateEmitter.fire(); + } + } else { + // Load the column profiles as a batch + const columnProfileResults = await this._dataExplorerClientInstance.getColumnProfiles( + columnRequests + ); + // Cache the column profiles that were returned. + for (let i = 0; i < columnProfileResults.length; i++) { + this._columnProfileCache.set(columnIndices[i], columnProfileResults[i]); + } + // Fire the onDidUpdate event. + this._onDidUpdateEmitter.fire(); + } } /** diff --git a/test/automation/src/positron/positronConsole.ts b/test/automation/src/positron/positronConsole.ts index 9c3736032d7..e7f96cf216a 100644 --- a/test/automation/src/positron/positronConsole.ts +++ b/test/automation/src/positron/positronConsole.ts @@ -283,4 +283,8 @@ export class PositronConsole { return suggestionList; } + + async clickConsoleTab() { + await this.code.driver.page.locator('.basepanel').getByRole('tab', { name: 'Console', exact: true }).locator('a').click(); + } } diff --git a/test/automation/src/positron/positronDataExplorer.ts b/test/automation/src/positron/positronDataExplorer.ts index 90ef6b98436..2a8d056daf7 100644 --- a/test/automation/src/positron/positronDataExplorer.ts +++ b/test/automation/src/positron/positronDataExplorer.ts @@ -214,11 +214,22 @@ export class PositronDataExplorer { await this.code.driver.getLocator(EXPAND_COLLASPE_ICON).nth(rowNumber).click(); } - async maximizeDataExplorer(): Promise { + async maximizeDataExplorer(collapseSummary: boolean = false): Promise { await this.workbench.positronLayouts.enterLayout('stacked'); await this.workbench.quickaccess.runCommand('workbench.action.toggleSidebarVisibility'); await this.workbench.quickaccess.runCommand('workbench.action.toggleAuxiliaryBar'); await this.workbench.quickaccess.runCommand('workbench.action.togglePanel'); + + if (collapseSummary) { + await this.collapseSummary(); + } } + async collapseSummary(): Promise { + await this.workbench.quickaccess.runCommand('workbench.action.positronDataExplorer.collapseSummary'); + } + + async expandSummary(): Promise { + await this.workbench.quickaccess.runCommand('workbench.action.positronDataExplorer.expandSummary'); + } } diff --git a/test/smoke/src/areas/positron/dataexplorer/dataexplorer.test.ts b/test/smoke/src/areas/positron/dataexplorer/dataexplorer.test.ts index b660528c225..c2c2ef59ad8 100644 --- a/test/smoke/src/areas/positron/dataexplorer/dataexplorer.test.ts +++ b/test/smoke/src/areas/positron/dataexplorer/dataexplorer.test.ts @@ -11,7 +11,7 @@ import { setupAndStartApp } from '../../../test-runner/test-hooks'; let logger; -describe('Data Explorer #web', () => { +describe('Data Explorer #web #win', () => { logger = setupAndStartApp(); describe('Python Pandas Data Explorer #pr', () => { @@ -32,7 +32,7 @@ describe('Data Explorer #web', () => { }); - it('Python Pandas - Verifies basic data explorer functionality [C557556] #win', async function () { + it('Python Pandas - Verifies basic data explorer functionality [C557556]', async function () { const app = this.app as Application; this.timeout(120000); @@ -96,9 +96,7 @@ df2 = pd.DataFrame(data)`; }).toPass(); // Need to make sure the data explorer is visible before we can interact with it - await app.workbench.positronSideBar.closeSecondarySideBar(); - await app.workbench.quickaccess.runCommand('workbench.action.closePanel'); - await app.workbench.quickaccess.runCommand('workbench.action.toggleSidebarVisibility'); + await app.workbench.positronDataExplorer.maximizeDataExplorer(true); await expect(async () => { const tableData = await app.workbench.positronDataExplorer.getDataExplorerTableData(); @@ -111,9 +109,12 @@ df2 = pd.DataFrame(data)`; expect(tableData.length).toBe(5); }).toPass({ timeout: 60000 }); + // Need to expand summary for next test + await app.workbench.positronDataExplorer.expandSummary(); }); - it('Python Pandas - Verifies data explorer column info functionality [C734263] #win', async function () { + // Cannot be run by itself, relies on the previous test + it('Python Pandas - Verifies data explorer column info functionality [C734263]', async function () { const app = this.app as Application; this.timeout(120000); @@ -149,7 +150,7 @@ df2 = pd.DataFrame(data)`; }); // This test is not dependent on the previous test, so it refreshes the python environment - it('Python Pandas - Verifies data explorer after modification [C557574] #win', async function () { + it('Python Pandas - Verifies data explorer after modification [C557574]', async function () { const app = this.app as Application; this.timeout(120000); @@ -214,8 +215,7 @@ df2 = pd.DataFrame(data)`; const app = this.app as Application; - await app.workbench.positronDataExplorer.closeDataExplorer(); - await app.workbench.positronVariables.toggleVariablesView(); + await app.workbench.positronLayouts.enterLayout('stacked'); await app.workbench.quickaccess.runCommand('workbench.action.closeAllEditors', { keepOpen: false }); }); @@ -233,7 +233,7 @@ df2 = pd.DataFrame(data)`; await app.code.driver.getLocator('.label-name:has-text("Data: df")').innerText(); }).toPass(); - await app.workbench.positronSideBar.closeSecondarySideBar(); + await app.workbench.positronDataExplorer.maximizeDataExplorer(true); await expect(async () => { const tableData = await app.workbench.positronDataExplorer.getDataExplorerTableData(); @@ -251,12 +251,13 @@ df2 = pd.DataFrame(data)`; }).toPass({ timeout: 60000 }); }); + // Cannot be run by itself, relies on the previous test it('Python Polars - Verifies basic data explorer column info functionality [C734264]', async function () { const app = this.app as Application; this.timeout(120000); - await app.workbench.positronLayouts.enterLayout('notebook'); + await app.workbench.positronDataExplorer.expandSummary(); expect(await app.workbench.positronDataExplorer.getColumnMissingPercent(1)).toBe('0%'); expect(await app.workbench.positronDataExplorer.getColumnMissingPercent(2)).toBe('0%'); @@ -265,6 +266,7 @@ df2 = pd.DataFrame(data)`; expect(await app.workbench.positronDataExplorer.getColumnMissingPercent(5)).toBe('33%'); expect(await app.workbench.positronDataExplorer.getColumnMissingPercent(6)).toBe('33%'); + const col1ProfileInfo = await app.workbench.positronDataExplorer.getColumnProfileInfo(1); expect(col1ProfileInfo).toStrictEqual({ 'Missing': '0', 'Min': '1.00', 'Median': '2.00', 'Mean': '2.00', 'Max': '3.00', 'SD': '1.00' }); @@ -283,12 +285,11 @@ df2 = pd.DataFrame(data)`; const col6ProfileInfo = await app.workbench.positronDataExplorer.getColumnProfileInfo(6); expect(col6ProfileInfo).toStrictEqual({ 'Missing': '1', 'True': '1', 'False': '1' }); - await app.workbench.positronLayouts.enterLayout('stacked'); - await app.workbench.positronSideBar.closeSecondarySideBar(); + await app.workbench.positronDataExplorer.collapseSummary(); }); - it('Python Polars - Add Simple Column filter [C557557] #win', async function () { + it('Python Polars - Add Simple Column filter [C557557]', async function () { const app = this.app as Application; this.timeout(120000); @@ -310,7 +311,7 @@ df2 = pd.DataFrame(data)`; }).toPass({ timeout: 60000 }); }); - it('Python Polars - Add Simple Column Sort [C557561] #win', async function () { + it('Python Polars - Add Simple Column Sort [C557561]', async function () { const app = this.app as Application; this.timeout(120000); @@ -346,7 +347,7 @@ df2 = pd.DataFrame(data)`; }); }); - describe('R Data Explorer #win', () => { + describe('R Data Explorer', () => { before(async function () { await PositronRFixtures.SetupFixtures(this.app as Application); @@ -373,7 +374,7 @@ df2 = pd.DataFrame(data)`; await app.code.driver.getLocator('.label-name:has-text("Data: Data_Frame")').innerText(); }).toPass(); - await app.workbench.positronLayouts.enterLayout('notebook'); + await app.workbench.positronDataExplorer.maximizeDataExplorer(true); await expect(async () => { const tableData = await app.workbench.positronDataExplorer.getDataExplorerTableData(); @@ -384,7 +385,6 @@ df2 = pd.DataFrame(data)`; expect(tableData.length).toBe(3); }).toPass({ timeout: 60000 }); - await app.workbench.positronLayouts.enterLayout('stacked'); }); @@ -393,6 +393,8 @@ df2 = pd.DataFrame(data)`; const app = this.app as Application; this.timeout(120000); + await app.workbench.positronDataExplorer.expandSummary(); + expect(await app.workbench.positronDataExplorer.getColumnMissingPercent(1)).toBe('0%'); expect(await app.workbench.positronDataExplorer.getColumnMissingPercent(2)).toBe('33%'); expect(await app.workbench.positronDataExplorer.getColumnMissingPercent(3)).toBe('0%'); diff --git a/test/smoke/src/areas/positron/viewer/viewer.test.ts b/test/smoke/src/areas/positron/viewer/viewer.test.ts index 664614bdc96..ccbe451136c 100644 --- a/test/smoke/src/areas/positron/viewer/viewer.test.ts +++ b/test/smoke/src/areas/positron/viewer/viewer.test.ts @@ -37,9 +37,11 @@ VetiverAPI(v).run()`; await theDoc.waitFor({ state: 'attached', timeout: 60000 }); - await app.workbench.positronConsole.activeConsole.click(); - // if click accidentally hits a link, use Escape to close the dialog - await app.workbench.positronConsole.sendKeyboardKey('Escape'); + // This is bad because it can end up clicking a link inside the console: + //await app.workbench.positronConsole.activeConsole.click(); + + await app.workbench.positronConsole.clickConsoleTab(); + await app.workbench.positronConsole.sendKeyboardKey('Control+C'); await app.workbench.positronConsole.waitForConsoleContents(buffer => buffer.some(line => line.includes('Application shutdown complete.')));