diff --git a/news/3 Code Health/5066.md b/news/3 Code Health/5066.md new file mode 100644 index 00000000000..d32abe7541a --- /dev/null +++ b/news/3 Code Health/5066.md @@ -0,0 +1 @@ +Add tests for data viewer slice data functionality. \ No newline at end of file diff --git a/pythonFiles/vscode_datascience_helpers/dataframes/vscodeDataFrame.py b/pythonFiles/vscode_datascience_helpers/dataframes/vscodeDataFrame.py index 3210cc61585..9b6800900b5 100644 --- a/pythonFiles/vscode_datascience_helpers/dataframes/vscodeDataFrame.py +++ b/pythonFiles/vscode_datascience_helpers/dataframes/vscodeDataFrame.py @@ -91,14 +91,14 @@ def _VSCODE_convertTensorToDataFrame(tensor, start=None, end=None): def _VSCODE_convertToDataFrame(df, start=None, end=None): vartype = type(df) if isinstance(df, list): - df = _VSCODE_pd.DataFrame(df) + df = _VSCODE_pd.DataFrame(df).iloc[start:end] elif isinstance(df, _VSCODE_pd.Series): - df = _VSCODE_pd.Series.to_frame(df) + df = _VSCODE_pd.Series.to_frame(df).iloc[start:end] elif isinstance(df, dict): df = _VSCODE_pd.Series(df) - df = _VSCODE_pd.Series.to_frame(df) + df = _VSCODE_pd.Series.to_frame(df).iloc[start:end] elif hasattr(df, "toPandas"): - df = df.toPandas() + df = df.toPandas().iloc[start:end] elif ( hasattr(vartype, "__name__") and vartype.__name__ in _VSCODE_allowedTensorTypes ): @@ -109,7 +109,7 @@ def _VSCODE_convertToDataFrame(df, start=None, end=None): """Disabling bandit warning for try, except, pass. We want to swallow all exceptions here to not crash on variable fetching""" try: - temp = _VSCODE_pd.DataFrame(df) + temp = _VSCODE_pd.DataFrame(df).iloc[start:end] df = temp except: # nosec pass diff --git a/src/client/datascience/jupyter/kernelVariables.ts b/src/client/datascience/jupyter/kernelVariables.ts index d4e02b7adcc..912e193aad2 100644 --- a/src/client/datascience/jupyter/kernelVariables.ts +++ b/src/client/datascience/jupyter/kernelVariables.ts @@ -164,10 +164,6 @@ export class KernelVariables implements IJupyterVariables { // Import the data frame script directory if we haven't already await this.importDataFrameScripts(notebook); - if (targetVariable.rowCount) { - end = Math.min(end, targetVariable.rowCount); - } - let expression = targetVariable.name; if (sliceExpression) { expression = `${targetVariable.name}${sliceExpression}`; diff --git a/src/datascience-ui/data-explorer/index.tsx b/src/datascience-ui/data-explorer/index.tsx index fc3b639bd68..7c2a3b53cc5 100644 --- a/src/datascience-ui/data-explorer/index.tsx +++ b/src/datascience-ui/data-explorer/index.tsx @@ -11,7 +11,6 @@ import '../common/index.css'; import * as React from 'react'; import * as ReactDOM from 'react-dom'; -import { initializeIcons } from '@fluentui/react'; import { IVsCodeApi } from '../react-common/postOffice'; import { detectBaseTheme } from '../react-common/themeDetector'; @@ -21,7 +20,6 @@ import { MainPanel } from './mainPanel'; export declare function acquireVsCodeApi(): IVsCodeApi; const baseTheme = detectBaseTheme(); -initializeIcons(); /* eslint-disable */ ReactDOM.render( diff --git a/src/datascience-ui/data-explorer/mainPanel.tsx b/src/datascience-ui/data-explorer/mainPanel.tsx index 0e778ea95aa..dbc4db2bbcc 100644 --- a/src/datascience-ui/data-explorer/mainPanel.tsx +++ b/src/datascience-ui/data-explorer/mainPanel.tsx @@ -33,6 +33,9 @@ import '../react-common/seti/seti.less'; import { SliceControl } from './sliceControl'; import { debounce } from 'lodash'; +import { initializeIcons } from '@fluentui/react'; +initializeIcons(); // Register all FluentUI icons being used to prevent developer console errors + const SliceableTypes: Set = new Set(['ndarray', 'Tensor', 'EagerTensor']); // Our css has to come after in order to override body styles diff --git a/src/datascience-ui/data-explorer/sliceControl.tsx b/src/datascience-ui/data-explorer/sliceControl.tsx index 9a4031d7ed2..777819bab47 100644 --- a/src/datascience-ui/data-explorer/sliceControl.tsx +++ b/src/datascience-ui/data-explorer/sliceControl.tsx @@ -35,6 +35,7 @@ const styleOverrides = { fontFamily: 'var(--vscode-font-family)', fontWeight: 'var(--vscode-font-weight)', fontSize: 'var(--vscode-font-size)', + border: 'var(--vscode-dropdown-border)', ':focus': { color: 'var(--vscode-dropdown-foreground)' }, diff --git a/src/test/datascience/dataviewer.functional.test.tsx b/src/test/datascience/dataviewer.functional.test.tsx index b60fc1ca78f..fbe181ad3fe 100644 --- a/src/test/datascience/dataviewer.functional.test.tsx +++ b/src/test/datascience/dataviewer.functional.test.tsx @@ -32,6 +32,13 @@ import { noop, sleep } from '../core'; import { DataScienceIocContainer } from './dataScienceIocContainer'; import { takeSnapshot, writeDiffSnapshot } from './helpers'; import { IMountedWebView } from './mountedWebView'; +import { SliceControl } from '../../datascience-ui/data-explorer/sliceControl'; +import { Dropdown } from '@fluentui/react'; + +interface ISliceControlTestInterface { + toggleEnablement: () => void; + applyDropdownsToInputBox: () => void; +} // import { asyncDump } from '../common/asyncDump'; suite('DataScience DataViewer tests', () => { @@ -95,7 +102,7 @@ suite('DataScience DataViewer tests', () => { delete (global as any).ascquireVsCodeApi; }); - function createJupyterVariable(variable: string, type: string): IJupyterVariable { + function createJupyterVariable(variable: string, type: string, shape: string): IJupyterVariable { return { name: variable, value: '', @@ -103,7 +110,7 @@ suite('DataScience DataViewer tests', () => { type, size: 0, truncated: true, - shape: '', + shape, count: 0 }; } @@ -118,8 +125,12 @@ suite('DataScience DataViewer tests', () => { return dataViewerFactory.create(dataProvider, title); } - async function createJupyterVariableDataViewer(variable: string, type: string): Promise { - const jupyterVariable: IJupyterVariable = createJupyterVariable(variable, type); + async function createJupyterVariableDataViewer( + variable: string, + type: string, + shape: string = '' + ): Promise { + const jupyterVariable: IJupyterVariable = createJupyterVariable(variable, type, shape); const jupyterVariableDataProvider: IDataViewerDataProvider = await createJupyterVariableDataProvider( jupyterVariable ); @@ -515,6 +526,392 @@ suite('DataScience DataViewer tests', () => { verifyRows(wrapper.wrapper, [0, 4, 5, 6]); }); + suite('Data viewer slice data', async () => { + function findSliceControlPanel(wrapper: ReactWrapper, React.Component>) { + const sliceControlWrapper = wrapper.find(SliceControl); + sliceControlWrapper.update(); + assert.ok(sliceControlWrapper && sliceControlWrapper.length > 0, 'Slice control not found'); + return sliceControlWrapper; + } + + function verifyReadonlyIndicator( + wrapper: ReactWrapper, React.Component>, + currentSlice: string + ) { + const sliceControl = wrapper.find(SliceControl); + const html = sliceControl.html(); + const root = parse(html) as any; + wrapper.render(); + const cells = root.querySelectorAll('.current-slice') as HTMLSpanElement[]; + assert.ok(cells.length === 1, 'No readonly indicator found'); + assert.ok(cells[0].innerHTML === currentSlice, 'Readonly indicator contents did not match'); + } + + function verifyDropdowns(wrapper: ReactWrapper, React.Component>, rows: (string | number)[]) { + const sliceControl = wrapper.find(SliceControl); + const html = sliceControl.html(); + const root = parse(html) as any; + const cells = root.querySelectorAll('.ms-Dropdown-title'); + assert.ok(cells.length >= rows.length, 'Not enough dropdowns found'); + // Now verify the list of dropdowns have the expected values + for (let i = 0; i < rows.length; i += 1) { + // Span reflects the dropdown's current selection + const span = cells[i] as HTMLSpanElement; + assert.ok(span, `Span ${i} not found`); + const val = rows[i].toString(); + assert.equal(span.innerHTML, val, `Dropdown ${i} selection not matching. ${span.innerHTML} !== ${val}`); + } + } + + function toggleCheckbox(wrapper: ReactWrapper, React.Component>) { + const sliceControl = findSliceControlPanel(wrapper); + // Enable slicing by toggling checkbox + const instance = (sliceControl.instance() as any) as ISliceControlTestInterface; + instance.toggleEnablement(); // simulate('click') doesn't suffice: https://github.com/facebook/react/issues/4950#issuecomment-255408709 + wrapper.render(); + } + + function verifyControlsDisabled( + wrapper: ReactWrapper, React.Component>, + expectedNumberOfDropdowns: number, + initialReadonlyIndicator: string + ) { + // Open the slice panel + findSliceControlPanel(wrapper); + // Verify that all controls are initially disabled + let input = wrapper.find('.slice-data'); + const html = input.html(); + assert.ok(html.includes('disabled'), 'Input field was not initially disabled'); + const dropdowns = wrapper.find(Dropdown); + assert.ok(dropdowns.length === expectedNumberOfDropdowns, 'Unexpected number of dropdowns found'); + // Verify no readonly indicator as we're not slicing yet + assert.throws( + () => verifyReadonlyIndicator(wrapper, initialReadonlyIndicator), + 'Readonly indicator rendered when not slicing' + ); + } + + function editInputValue(wrapper: IMountedWebView, slice: string) { + const inputElement = wrapper.wrapper.find('.slice-data').getDOMNode() as HTMLInputElement; + inputElement.value = slice; + wrapper.wrapper.find('.slice-data').simulate('change'); + } + + async function applySliceAndVerifyReadonlyIndicator(wrapper: IMountedWebView, slice: string) { + // Apply a slice to input box + const gotSlice = getCompletedPromise(wrapper); + editInputValue(wrapper, slice); + wrapper.wrapper.find('form').first().simulate('submit'); + await gotSlice; + // Ensure readonly indicator updates after slicing + verifyReadonlyIndicator(wrapper.wrapper, slice); + } + + async function changeDropdown( + wrapper: IMountedWebView, + dropdownType: 'Axis' | 'Index', + dropdownRow: number, + newValue: number | string + ) { + const gotSlice = getCompletedPromise(wrapper); + const sliceControl = findSliceControlPanel(wrapper.wrapper); + // Do a setstate because we don't have direct access to the dropdown selection change handler + const newState = { [`selected${dropdownType}${dropdownRow}`]: newValue }; + sliceControl.setState(newState); + const instance = (sliceControl.instance() as any) as ISliceControlTestInterface; + // This is what gets called in the dropdown change handler. Manually call it because + // simulating a change event on the Dropdown node doesn't seem to do anything + instance.applyDropdownsToInputBox(); + wrapper.wrapper.render(); + await gotSlice; + } + + runMountedTest('Slice 2D', async (wrapper) => { + const code = `import torch +import numpy as np +arr = np.arange(6).reshape(2, 3) +foo = torch.tensor(arr)`; + + // Create data viewer + await injectCode(code); + const gotAllRows = getCompletedPromise(wrapper); + const dv = await createJupyterVariableDataViewer('foo', 'Tensor', '(2, 3)'); + assert.ok(dv, 'DataViewer not created'); + await gotAllRows; + verifyRows(wrapper.wrapper, [0, 0, 1, 2, 1, 3, 4, 5]); + verifyControlsDisabled(wrapper.wrapper, 2, ''); + + // Apply a slice via input box and verify that dropdowns update + toggleCheckbox(wrapper.wrapper); + await applySliceAndVerifyReadonlyIndicator(wrapper, '[1, :]'); + verifyRows(wrapper.wrapper, [0, 3, 1, 4, 2, 5]); + verifyDropdowns(wrapper.wrapper, [0, 1]); // Axis 0, index 1 + + // Change the dropdowns and verify that the slice expression updates + await changeDropdown(wrapper, 'Axis', 0, 1); + verifyReadonlyIndicator(wrapper.wrapper, '[:, 1]'); + verifyDropdowns(wrapper.wrapper, [1, 1]); + verifyRows(wrapper.wrapper, [0, 1, 1, 4]); + assert.ok( + (wrapper.wrapper.find('.slice-data').getDOMNode() as HTMLInputElement).value === '[:, 1]', + 'Input box did not update to match slice' + ); + + // Apply a slice with no corresponding dropdown + await applySliceAndVerifyReadonlyIndicator(wrapper, '[:, :2]'); + verifyRows(wrapper.wrapper, [0, 0, 1, 1, 3, 4]); + verifyDropdowns(wrapper.wrapper, ['', '']); // Dropdowns should be unset + + // Uncheck slice checkbox and verify original contents are restored + const disableSlicing = getCompletedPromise(wrapper); + toggleCheckbox(wrapper.wrapper); + await disableSlicing; + verifyRows(wrapper.wrapper, [0, 0, 1, 2, 1, 3, 4, 5]); + verifyControlsDisabled(wrapper.wrapper, 2, ''); + + // Recheck slice checkbox and verify slice expression is restored + const reenableSlicing = getCompletedPromise(wrapper); + toggleCheckbox(wrapper.wrapper); + await reenableSlicing; + verifyRows(wrapper.wrapper, [0, 0, 1, 1, 3, 4]); + + // Enter an invalid slice expression and verify error message is rendered + editInputValue(wrapper, '[:]'); + assert.ok( + wrapper.wrapper.find('.error-message').length === 1, + 'No error message rendered for invalid slice' + ); + }); + + runMountedTest('Slice 3D', async (wrapper) => { + const code = `import torch +import numpy as np +arr = np.arange(24).reshape(2,4,3) +foo = torch.tensor(arr)`; + // Create data viewer + await injectCode(code); + const gotAllRows = getCompletedPromise(wrapper); + const dv = await createJupyterVariableDataViewer('foo', 'Tensor', '(2, 4, 3)'); + assert.ok(dv, 'DataViewer not created'); + await gotAllRows; + verifyRows(wrapper.wrapper, [ + 0, + '[0, 1, 2]', + '[3, 4, 5]', + '[6, 7, 8]', + '[9, 10, 11]', + 1, + '[12, 13, 14]', + '[15, 16, 17]', + '[18, 19, 20]', + '[21, 22, 23]' + ]); + verifyControlsDisabled(wrapper.wrapper, 2, ''); + + // Toggle on slicing. Slice should immediately be applied + const enableSlicing = getCompletedPromise(wrapper); + toggleCheckbox(wrapper.wrapper); + await enableSlicing; + verifyReadonlyIndicator(wrapper.wrapper, '[0, :, :]'); + verifyRows(wrapper.wrapper, [0, 0, 1, 2, 1, 3, 4, 5, 2, 6, 7, 8, 3, 9, 10, 11]); + + // Change the dropdowns and verify that the slice expression updates + await changeDropdown(wrapper, 'Axis', 0, 1); + verifyReadonlyIndicator(wrapper.wrapper, '[:, 0, :]'); + verifyDropdowns(wrapper.wrapper, [1, 0]); + verifyRows(wrapper.wrapper, [0, 0, 1, 2, 1, 12, 13, 14]); + assert.ok( + (wrapper.wrapper.find('.slice-data').getDOMNode() as HTMLInputElement).value === '[:, 0, :]', + 'Input box did not update to match slice' + ); + + // Apply a slice via input box and verify that dropdowns update + await applySliceAndVerifyReadonlyIndicator(wrapper, '[:, :, 2]'); + verifyRows(wrapper.wrapper, [0, 2, 5, 8, 11, 1, 14, 17, 20, 23]); + verifyDropdowns(wrapper.wrapper, [2, 2]); // Axis 2, index 2 + + // Apply a slice with no corresponding dropdown + await applySliceAndVerifyReadonlyIndicator(wrapper, '[:, :1, :]'); + verifyRows(wrapper.wrapper, [0, '[0, 1, 2]', 1, '[12, 13, 14]']); + verifyDropdowns(wrapper.wrapper, ['', '']); // Dropdowns should be unset + + // Uncheck slice checkbox and verify original contents are restored + const disableSlicing = getCompletedPromise(wrapper); + toggleCheckbox(wrapper.wrapper); + await disableSlicing; + verifyRows(wrapper.wrapper, [ + 0, + '[0, 1, 2]', + '[3, 4, 5]', + '[6, 7, 8]', + '[9, 10, 11]', + 1, + '[12, 13, 14]', + '[15, 16, 17]', + '[18, 19, 20]', + '[21, 22, 23]' + ]); + verifyControlsDisabled(wrapper.wrapper, 2, ''); + + // Recheck slice checkbox and verify slice expression is restored + const reenableSlicing = getCompletedPromise(wrapper); + toggleCheckbox(wrapper.wrapper); + await reenableSlicing; + verifyRows(wrapper.wrapper, [0, '[0, 1, 2]', 1, '[12, 13, 14]']); + verifyReadonlyIndicator(wrapper.wrapper, '[:, :1, :]'); + + // Enter an invalid slice expression and verify error message is rendered + editInputValue(wrapper, '[:]'); + assert.ok( + wrapper.wrapper.find('.error-message').length === 1, + 'No error message rendered for invalid slice' + ); + }); + + runMountedTest('Slice 4D', async (wrapper) => { + const code = `import torch +import numpy as np +arr = np.arange(30).reshape(3, 5, 1, 2) +foo = torch.tensor(arr)`; + // Create data viewer + await injectCode(code); + const gotAllRows = getCompletedPromise(wrapper); + const dv = await createJupyterVariableDataViewer('foo', 'Tensor', '(3, 5, 1, 2)'); + assert.ok(dv, 'DataViewer not created'); + await gotAllRows; + verifyRows(wrapper.wrapper, [ + 0, + '[[0, 1]]', + `[[2, 3]]`, + '[[4, 5]]', + '[[6, 7]]', + '[[8, 9]]', + 1, + '[[10, 11]]', + '[[12, 13]]', + '[[14, 15]]', + '[[16, 17]]', + '[[18, 19]]', + 2, + '[[20, 21]]', + '[[22, 23]]', + '[[24, 25]]', + '[[26, 27]]', + '[[28, 29]]' + ]); + verifyControlsDisabled(wrapper.wrapper, 4, ''); + + // Toggle on slicing. Slice should immediately be applied + const enableSlicing = getCompletedPromise(wrapper); + toggleCheckbox(wrapper.wrapper); + await enableSlicing; + verifyReadonlyIndicator(wrapper.wrapper, '[0, 0, :, :]'); + verifyRows(wrapper.wrapper, [0, 0, 1]); + + // Change the dropdowns and verify that the slice expression updates + await changeDropdown(wrapper, 'Index', 1, 2); + verifyReadonlyIndicator(wrapper.wrapper, '[0, 2, :, :]'); + verifyDropdowns(wrapper.wrapper, [0, 0, 1, 2]); + verifyRows(wrapper.wrapper, [0, 4, 5]); + assert.ok( + (wrapper.wrapper.find('.slice-data').getDOMNode() as HTMLInputElement).value === '[0, 2, :, :]', + 'Input box did not update to match slice' + ); + + // Apply a slice via input box and verify that dropdowns update + await applySliceAndVerifyReadonlyIndicator(wrapper, '[:, 4, :, 1]'); + verifyRows(wrapper.wrapper, [0, 9, 1, 19, 2, 29]); + verifyDropdowns(wrapper.wrapper, [1, 4, 3, 1]); // Axis 1 index 4, axis 3 index 1 + + // Apply a slice with no corresponding dropdown + await applySliceAndVerifyReadonlyIndicator(wrapper, '[1, 2, 0, :]'); + verifyRows(wrapper.wrapper, [0, 14, 1, 15]); + verifyDropdowns(wrapper.wrapper, ['', '', '', '']); // Dropdowns should be unset + + // Uncheck slice checkbox and verify original contents are restored + const disableSlicing = getCompletedPromise(wrapper); + toggleCheckbox(wrapper.wrapper); + await disableSlicing; + verifyRows(wrapper.wrapper, [ + 0, + '[[0, 1]]', + `[[2, 3]]`, + '[[4, 5]]', + '[[6, 7]]', + '[[8, 9]]', + 1, + '[[10, 11]]', + '[[12, 13]]', + '[[14, 15]]', + '[[16, 17]]', + '[[18, 19]]', + 2, + '[[20, 21]]', + '[[22, 23]]', + '[[24, 25]]', + '[[26, 27]]', + '[[28, 29]]' + ]); + verifyControlsDisabled(wrapper.wrapper, 4, ''); + + // Recheck slice checkbox and verify slice expression is restored + const reenableSlicing = getCompletedPromise(wrapper); + toggleCheckbox(wrapper.wrapper); + await reenableSlicing; + verifyRows(wrapper.wrapper, [0, 14, 1, 15]); + verifyDropdowns(wrapper.wrapper, ['', '', '', '']); // Dropdowns should be unset + + // Enter an invalid slice expression and verify error message is rendered + editInputValue(wrapper, '[:]'); + assert.ok( + wrapper.wrapper.find('.error-message').length === 1, + 'No error message rendered for invalid slice' + ); + }); + + runMountedTest('Refresh with slice applied', async (wrapper) => { + // Same shape, old slice is still valid, ensure update in place + const code = `import torch +foo = torch.tensor([[[0, 1, 2], [3, 4, 5]]])`; + // Create data viewer + const notebook = await injectCode(code); + const gotAllRows = getCompletedPromise(wrapper); + const dv = await createJupyterVariableDataViewer('foo', 'Tensor', '(1, 2, 3)'); + assert.ok(dv, 'DataViewer not created'); + await gotAllRows; + + // Toggle on slicing. Slice should immediately be applied + const enableSlicing = getCompletedPromise(wrapper); + toggleCheckbox(wrapper.wrapper); + await enableSlicing; + verifyReadonlyIndicator(wrapper.wrapper, '[0, :, :]'); + verifyRows(wrapper.wrapper, [0, 0, 1, 2, 1, 3, 4, 5]); + + // Apply a slice via input box and verify that dropdowns update + await applySliceAndVerifyReadonlyIndicator(wrapper, '[:, 1, :]'); + verifyRows(wrapper.wrapper, [0, 3, 4, 5]); + verifyDropdowns(wrapper.wrapper, [1, 1]); // Axis 1 index 1 + + // New variable value but same shape. Ensure slice updates in-place + await executeCode('foo = torch.tensor([[[6, 7, 8], [9, 10, 11]]])', notebook!); + const refreshPromise = getCompletedPromise(wrapper); + await dv.refreshData(); + await refreshPromise; + verifyReadonlyIndicator(wrapper.wrapper, '[:, 1, :]'); + verifyRows(wrapper.wrapper, [0, 9, 10, 11]); + + // New variable shape invalidates old slice + await executeCode('foo = torch.tensor([[[0, 1]], [[2, 3]]])', notebook!); + // Ensure data updates + const invalidateSlicePromise = getCompletedPromise(wrapper); + await dv.refreshData(); + await invalidateSlicePromise; + // Preselected slice is applied + verifyReadonlyIndicator(wrapper.wrapper, '[0, :, :]'); + verifyRows(wrapper.wrapper, [0, 0, 1]); + }); + }); + // https://github.com/microsoft/vscode-jupyter/issues/4706 // Disabled for now. Root cause is that pd.replace isn't recursive over objects in DataFrames, // so our current inf/nan handling does not work for DataFrames whose cells are Series, ndarray, or list