Check vars on execution count (microsoft#5214)

Author: IanMatthewHuff

news/1 Enhancements/4948.md

@@ -0,0 +1 @@
+Variable explorer handles new cell submissions

pythonFiles/datascience/getJupyterVariableList.py

@@ -12,8 +12,7 @@
 print(_VSCODE_json.dumps([{'name': var,
  'type': type(eval(var)).__name__,
  'size': _VSCODE_getsizeof(var),
- 'supportsDataExplorer': type(eval(var)).__name__ in _VSCode_supportsDataExplorer,
- 'expensive': True
+ 'supportsDataExplorer': type(eval(var)).__name__ in _VSCode_supportsDataExplorer
  } for var in _VSCode_JupyterVars]))
 
 del _VSCode_supportsDataExplorer

src/client/datascience/history/history.ts

@@ -228,7 +228,7 @@ export class History extends WebViewHost<IHistoryMapping> implements IHistory {
  break;
 
  case HistoryMessages.GetVariablesRequest:
- this.requestVariables().ignoreErrors();
+ this.requestVariables(payload).ignoreErrors();
  break;
 
  case HistoryMessages.GetVariableValueRequest:
@@ -918,10 +918,15 @@ export class History extends WebViewHost<IHistoryMapping> implements IHistory {
  }
  }
 
- private requestVariables = async (): Promise<void> => {
+ private requestVariables = async (executionCount: number): Promise<void> => {
  // Request our new list of variables
  let vars: IJupyterVariable[] = await this.jupyterVariables.getVariables();
 
+ // Tag all of our jupyter variables with the execution count of the request
+ vars.forEach((value: IJupyterVariable) => {
+ value.executionCount = executionCount;
+ });
+
  const settings = this.configuration.getSettings();
  const excludeString = settings.datascience.variableExplorerExclude;
 

src/client/datascience/history/historyTypes.ts

@@ -96,7 +96,7 @@ export class IHistoryMapping {
  public [HistoryMessages.RemoteAddCode]: IRemoteAddCode;
  public [HistoryMessages.Activate] : never | undefined;
  public [HistoryMessages.ShowDataViewer]: string;
- public [HistoryMessages.GetVariablesRequest]: never | undefined;
+ public [HistoryMessages.GetVariablesRequest]: number;
  public [HistoryMessages.GetVariablesResponse]: IJupyterVariable[];
  public [HistoryMessages.GetVariableValueRequest]: IJupyterVariable;
  public [HistoryMessages.GetVariableValueResponse]: IJupyterVariable;

src/client/datascience/types.ts

@@ -262,13 +262,13 @@ export interface IDataScienceExtraSettings extends IDataScienceSettings {
 export interface IJupyterVariable {
  name: string;
  value: string | undefined;
+ executionCount?: number;
  supportsDataExplorer: boolean;
  type: string;
  size: number;
  shape: string;
  count: number;
  truncated: boolean;
- expensive: boolean;
  columns?: { key: string; type: string }[];
  rowCount?: number;
 }

src/datascience-ui/history-react/MainPanel.tsx

@@ -35,6 +35,7 @@ export class MainPanel extends React.Component<IMainPanelProps, IMainPanelState>
  private mainPanel: HTMLDivElement | null = null;
  private variableExplorerRef: React.RefObject<VariableExplorer>;
  private styleInjectorRef: React.RefObject<StyleInjector>;
+ private currentExecutionCount: number = 0;
 
  // tslint:disable-next-line:max-func-body-length
  constructor(props: IMainPanelProps, _state: IMainPanelState) {
@@ -667,8 +668,12 @@ export class MainPanel extends React.Component<IMainPanelProps, IMainPanelState>
  }
  }
 
+ // After the cell is finished update our current execution count
+ this.currentExecutionCount = this.getCurrentExecutionCount(this.state.cellVMs);
+
  // When a cell is finished refresh our variables
- if (getSettings && getSettings().showJupyterVariableExplorer) {
+ // Use the ref here to maintain var explorer independence
+ if (this.variableExplorerRef.current && this.variableExplorerRef.current.state.open) {
  this.refreshVariables();
  }
  }
@@ -693,9 +698,14 @@ export class MainPanel extends React.Component<IMainPanelProps, IMainPanelState>
  }
  }
 
- private getInputExecutionCount(cellVMs: ICellViewModel[]) : number {
+ // Check our list of cell vms to see what our current execution count is
+ private getCurrentExecutionCount = (cellVMs: ICellViewModel[]): number => {
  const realCells = cellVMs.filter(c => c.cell.data.cell_type === 'code' && !c.editable && c.cell.data.execution_count);
- return realCells && realCells.length > 0 ? parseInt(realCells[realCells.length - 1].cell.data.execution_count!.toString(), 10) + 1 : 1;
+ return realCells && realCells.length > 0 ? parseInt(realCells[realCells.length - 1].cell.data.execution_count!.toString(), 10) : 0;
+ }
+
+ private getInputExecutionCount = (cellVMs: ICellViewModel[]) : number => {
+ return this.getCurrentExecutionCount(cellVMs) + 1;
  }
 
  private submitInput = (code: string) => {
@@ -747,7 +757,7 @@ export class MainPanel extends React.Component<IMainPanelProps, IMainPanelState>
 
  // When the variable explorer wants to refresh state (say if it was expanded)
  private refreshVariables = () => {
- this.sendMessage(HistoryMessages.GetVariablesRequest);
+ this.sendMessage(HistoryMessages.GetVariablesRequest, this.currentExecutionCount);
  }
 
  // Find the display value for one specific variable
@@ -761,8 +771,11 @@ export class MainPanel extends React.Component<IMainPanelProps, IMainPanelState>
  if (payload) {
  const variable = payload as IJupyterVariable;
 
- if (this.variableExplorerRef.current) {
- this.variableExplorerRef.current.newVariableData(variable);
+ // Only send the updated variable data if we are on the same execution count as when we requsted it
+ if (variable && variable.executionCount !== undefined && variable.executionCount === this.currentExecutionCount) {
+ if (this.variableExplorerRef.current) {
+ this.variableExplorerRef.current.newVariableData(variable);
+ }
  }
  }
  }
@@ -773,12 +786,15 @@ export class MainPanel extends React.Component<IMainPanelProps, IMainPanelState>
  if (payload) {
  const variables = payload as IJupyterVariable[];
 
- if (this.variableExplorerRef.current) {
- this.variableExplorerRef.current.newVariablesData(variables);
- }
+ // Check to see if we have moved to a new execution count only send our update if we are on the same count as the request
+ if (variables.length > 0 && variables[0].executionCount !== undefined && variables[0].executionCount === this.currentExecutionCount) {
+ if (this.variableExplorerRef.current) {
+ this.variableExplorerRef.current.newVariablesData(variables);
+ }
 
- // Now put out a request for all of the sub values for the variables
- variables.forEach(this.refreshVariable);
+ // Now put out a request for all of the sub values for the variables
+ variables.forEach(this.refreshVariable);
+ }
  }
  }
 }

src/datascience-ui/history-react/variableExplorer.tsx

@@ -54,7 +54,7 @@ export class VariableExplorer extends React.Component<IVariableExplorerProps, IV
  {key: 'type', name: getLocString('DataScience.variableExplorerTypeColumn', 'Type'), type: 'string', width: 120},
  {key: 'size', name: getLocString('DataScience.variableExplorerSizeColumn', 'Size'), type: 'number', width: 120, formatter: <VariableExplorerCellFormatter cellStyle={CellStyle.numeric} />},
  {key: 'value', name: getLocString('DataScience.variableExplorerValueColumn', 'Value'), type: 'string', width: 300},
- {key: 'buttons', name: '', type: 'boolean', width: 32, formatter: <VariableExplorerButtonCellFormatter showDataExplorer={this.props.showDataExplorer} baseTheme={this.props.baseTheme} /> }
+ {key: 'buttons', name: '', type: 'boolean', width: 34, formatter: <VariableExplorerButtonCellFormatter showDataExplorer={this.props.showDataExplorer} baseTheme={this.props.baseTheme} /> }
  ];
  this.state = { open: false,
  gridColumns: columns,

src/datascience-ui/history-react/variableExplorerGrid.scss

@@ -36,9 +36,9 @@
 #variable-explorer-data-grid .react-grid-HeaderCell {
  background-color: var(--vscode-editor-lineHighlightBorder);
  color: var(--vscode-editor-foreground);
- border-style: none none solid none;
- border-bottom-color: var(--vscode-badge-background);
- padding: 4px;
+ border-style: solid;
+ border-color: var(--vscode-badge-background);
+ padding: 2px;
 }
 
 

src/test/datascience/jupyterVariables.unit.test.ts

@@ -75,7 +75,7 @@ suite('JupyterVariables', () => {
 
  fakeServer.setup(fs => fs.execute(typemoq.It.isAny(), typemoq.It.isAny(), typemoq.It.isAny(), typemoq.It.isAny(), undefined, typemoq.It.isAny()))
  .returns(() => Promise.resolve(generateCells(
- '[{"name": "big_dataframe", "type": "DataFrame", "size": 62, "expensive": true}, {"name": "big_dict", "type": "dict", "size": 57, "expensive": true}, {"name": "big_float", "type": "float", "size": 58, "expensive": true}, {"name": "big_int", "type": "int", "size": 56, "expensive": true}, {"name": "big_list", "type": "list", "size": 57, "expensive": true}, {"name": "big_nparray", "type": "ndarray", "size": 60, "expensive": true}, {"name": "big_series", "type": "Series", "size": 59, "expensive": true}, {"name": "big_string", "type": "str", "size": 59, "expensive": true}, {"name": "big_tuple", "type": "tuple", "size": 58, "expensive": true}]',
+ '[{"name": "big_dataframe", "type": "DataFrame", "size": 62}, {"name": "big_dict", "type": "dict", "size": 57}, {"name": "big_float", "type": "float", "size": 58}, {"name": "big_int", "type": "int", "size": 56}, {"name": "big_list", "type": "list", "size": 57}, {"name": "big_nparray", "type": "ndarray", "size": 60}, {"name": "big_series", "type": "Series", "size": 59}, {"name": "big_string", "type": "str", "size": 59}, {"name": "big_tuple", "type": "tuple", "size": 58]',
  'stream'
  )))
  .verifiable(typemoq.Times.never());
@@ -157,7 +157,7 @@ suite('JupyterVariables', () => {
 
  fakeServer.setup(fs => fs.execute(typemoq.It.isValue('test'), typemoq.It.isValue(Identifiers.EmptyFileName), typemoq.It.isValue(0), typemoq.It.isAnyString(), undefined, typemoq.It.isValue(true)))
  .returns(() => Promise.resolve(generateCells(
- '[{"name": "big_dataframe", "type": "DataFrame", "size": 62, "expensive": true}, {"name": "big_dict", "type": "dict", "size": 57, "expensive": true}, {"name": "big_int", "type": "int", "size": 56, "expensive": true}, {"name": "big_list", "type": "list", "size": 57, "expensive": true}, {"name": "big_nparray", "type": "ndarray", "size": 60, "expensive": true}, {"name": "big_string", "type": "str", "size": 59, "expensive": true}]',
+ '[{"name": "big_dataframe", "type": "DataFrame", "size": 62}, {"name": "big_dict", "type": "dict", "size": 57}, {"name": "big_int", "type": "int", "size": 56}, {"name": "big_list", "type": "list", "size": 57}, {"name": "big_nparray", "type": "ndarray", "size": 60}, {"name": "big_string", "type": "str", "size": 59}]',
  'stream'
  )))
  .verifiable(typemoq.Times.once());
@@ -168,12 +168,12 @@ suite('JupyterVariables', () => {
  assert.equal(results.length, 6);
 
  // Check our items (just the first few real items, no need to check all 19)
- assert.deepEqual(results[0], {name: 'big_dataframe', size: 62, type: 'DataFrame', expensive: true});
- assert.deepEqual(results[1], {name: 'big_dict', size: 57, type: 'dict', expensive: true});
- assert.deepEqual(results[2], {name: 'big_int', size: 56, type: 'int', expensive: true});
- assert.deepEqual(results[3], {name: 'big_list', size: 57, type: 'list', expensive: true});
- assert.deepEqual(results[4], {name: 'big_nparray', size: 60, type: 'ndarray', expensive: true});
- assert.deepEqual(results[5], {name: 'big_string', size: 59, type: 'str', expensive: true});
+ assert.deepEqual(results[0], {name: 'big_dataframe', size: 62, type: 'DataFrame'});
+ assert.deepEqual(results[1], {name: 'big_dict', size: 57, type: 'dict'});
+ assert.deepEqual(results[2], {name: 'big_int', size: 56, type: 'int'});
+ assert.deepEqual(results[3], {name: 'big_list', size: 57, type: 'list'});
+ assert.deepEqual(results[4], {name: 'big_nparray', size: 60, type: 'ndarray'});
+ assert.deepEqual(results[5], {name: 'big_string', size: 59, type: 'str'});
 
  fakeServer.verifyAll();
  });
@@ -186,17 +186,17 @@ suite('JupyterVariables', () => {
 
  fakeServer.setup(fs => fs.execute(typemoq.It.isValue('test'), typemoq.It.isValue(Identifiers.EmptyFileName), typemoq.It.isValue(0), typemoq.It.isAnyString(), undefined, typemoq.It.isValue(true)))
  .returns(() => Promise.resolve(generateCells(
- '{"name": "big_complex", "type": "complex", "size": 60, "expensive": true, "value": "(1+1j)"}',
+ '{"name": "big_complex", "type": "complex", "size": 60, "value": "(1+1j)"}',
  'stream'
  )))
  .verifiable(typemoq.Times.once());
 
- const testVariable: IJupyterVariable = { name: 'big_complex', type: 'complex', size: 60, expensive: true, truncated: false, count: 0, shape: '', value: '', supportsDataExplorer: false };
+ const testVariable: IJupyterVariable = { name: 'big_complex', type: 'complex', size: 60, truncated: false, count: 0, shape: '', value: '', supportsDataExplorer: false };
 
  const resultVariable = await jupyterVariables.getValue(testVariable);
 
  // Verify the result value should be filled out from fake server result
- assert.deepEqual(resultVariable, {name: 'big_complex', size: 60, type: 'complex', expensive: true, value: '(1+1j)'});
+ assert.deepEqual(resultVariable, {name: 'big_complex', size: 60, type: 'complex', value: '(1+1j)'});
  fakeServer.verifyAll();
  });
 });