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

Fix display of non-numeric DataFrame index columns #5254

Merged
merged 5 commits into from
Mar 24, 2021
Merged
Show file tree
Hide file tree
Changes from 4 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
1 change: 1 addition & 0 deletions news/2 Fixes/5253.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix data viewer display of non-numeric index columns in DataFrames.
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,17 @@ def _VSCODE_getDataFrameInfo(df):

columnTypes = _VSCODE_builtins.list(df.dtypes)

# Compute the index column. It may have been renamed
try:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks a bit different from the code in getJupyterVariableDataFrameInfo.py that does something similar with index column. Looks like your version might be safer with the try / except. Should that maybe be updated as well?

Copy link
Contributor Author

@joyceerhl joyceerhl Mar 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually just restoring code that I removed several PRs ago. Rich pointed out at the time that he thought it was required, and now I see why 😊

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, maybe I didn't say that right. I was saying that this code here looked safer than the code in the other location. Just wondering if getJupyterVariableDataFrameInfo.py should be upgraded with the try / except that you have here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah good point, will update

indexColumn = df.index.name if df.index.name else "index"
except AttributeError:
indexColumn = "index"

# Make sure the index column exists
if indexColumn not in columnNames:
columnNames.insert(0, indexColumn)
columnTypes.insert(0, "int64")

# Then loop and generate our output json
columns = []
for n in _VSCODE_builtins.range(0, _VSCODE_builtins.len(columnNames)):
Expand All @@ -184,6 +195,7 @@ def _VSCODE_getDataFrameInfo(df):
# Save this in our target
target = {}
target["columns"] = columns
target["indexColumn"] = indexColumn
target["rowCount"] = rowCount

# return our json object as a string
Expand Down
51 changes: 32 additions & 19 deletions src/datascience-ui/data-explorer/mainPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,13 @@ import '../react-common/codicon/codicon.css';
import '../react-common/seti/seti.less';
import { SliceControl } from './sliceControl';
import { debounce } from 'lodash';
import * as uuid from 'uuid/v4';

import { initializeIcons } from '@fluentui/react';
initializeIcons(); // Register all FluentUI icons being used to prevent developer console errors

const SliceableTypes: Set<string> = new Set<string>(['ndarray', 'Tensor', 'EagerTensor']);
const RowNumberColumnName = uuid(); // Unique key for our column containing row numbers

// Our css has to come after in order to override body styles
export interface IMainPanelProps {
Expand Down Expand Up @@ -257,7 +259,7 @@ export class MainPanel extends React.Component<IMainPanelProps, IMainPanelState>
<ReactSlickGrid
ref={this.grid}
columns={this.state.gridColumns}
idProperty={this.state.indexColumn}
idProperty={RowNumberColumnName}
rowsAdded={this.gridAddEvent}
resetGridEvent={this.resetGridEvent}
columnsUpdated={this.gridColumnUpdateEvent}
Expand Down Expand Up @@ -368,7 +370,7 @@ export class MainPanel extends React.Component<IMainPanelProps, IMainPanelState>
gridRows: newActual
});

// Tell our grid about the new ros
// Tell our grid about the new rows
this.updateRows(normalized);

// Get the next chunk
Expand All @@ -384,23 +386,34 @@ export class MainPanel extends React.Component<IMainPanelProps, IMainPanelState>
}

private generateColumns(variable: IDataFrameInfo): Slick.Column<Slick.SlickData>[] {
// Generate an index column
const indexColumn = {
key: this.state.indexColumn,
type: ColumnType.Number
};
if (variable.columns) {
const columns = [indexColumn].concat(variable.columns);
return columns.map((c: { key: string; type: ColumnType }, i: number) => {
return {
type: c.type,
field: c.key.toString(),
id: `${i}`,
name: c.key.toString(),
sortable: true,
formatter: cellFormatterFunc
};
});
// Generate a column for row numbers
const rowNumberColumn = {
key: RowNumberColumnName,
type: ColumnType.Number
};
const columns = [rowNumberColumn].concat(variable.columns);
return columns.reduce(
(accum: Slick.Column<Slick.SlickData>[], c: { key: string; type: ColumnType }, i: number) => {
// Only show index column for pandas DataFrame and Series
if (
variable?.type === 'DataFrame' ||
variable?.type === 'Series' ||
c.key !== this.state.indexColumn
) {
accum.push({
type: c.type,
field: c.key.toString(),
id: `${i}`,
name: c.key.toString(),
sortable: true,
formatter: cellFormatterFunc
} as Slick.Column<Slick.SlickData>);
}
return accum;
},
[]
);
}
return [];
}
Expand All @@ -416,7 +429,7 @@ export class MainPanel extends React.Component<IMainPanelProps, IMainPanelState>
if (!r) {
r = {};
}
r[this.state.indexColumn] = this.state.fetchedRowCount + idx;
r[RowNumberColumnName] = this.state.fetchedRowCount + idx;
for (let [key, value] of Object.entries(r)) {
switch (value) {
case 'nan':
Expand Down
6 changes: 3 additions & 3 deletions src/datascience-ui/data-explorer/reactSlickGrid.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,7 @@ export class ReactSlickGrid extends React.Component<ISlickGridProps, ISlickGridS
const placeholder = '99999999999';
const maxFieldWidth = measureText(placeholder, fontString);
columns.forEach((c) => {
if (c.id !== '0') {
if (c.field !== this.props.idProperty) {
c.width = maxFieldWidth;
} else {
c.width = maxFieldWidth / 2;
Expand Down Expand Up @@ -520,7 +520,6 @@ export class ReactSlickGrid extends React.Component<ISlickGridProps, ISlickGridS
this.dataView.setItems([]);
const styledColumns = this.styleColumns(data.columns);
this.setColumns(styledColumns);
this.autoResizeColumns();
};

private updateColumns = (_e: Slick.EventData, newColumns: Slick.Column<Slick.SlickData>[]) => {
Expand All @@ -535,6 +534,7 @@ export class ReactSlickGrid extends React.Component<ISlickGridProps, ISlickGridS
// The solution is to force the header row to become visible just before sending our slice request.
this.state.grid?.setHeaderRowVisibility(true);
this.state.grid?.setColumns(newColumns);
this.autoResizeColumns();
};

private addedRows = (_e: Slick.EventData, data: ISlickGridAdd) => {
Expand Down Expand Up @@ -573,7 +573,7 @@ export class ReactSlickGrid extends React.Component<ISlickGridProps, ISlickGridS
}

private renderFilterCell = (_e: Slick.EventData, args: Slick.OnHeaderRowCellRenderedEventArgs<Slick.SlickData>) => {
if (args.column.id === '0') {
if (args.column.field === this.props.idProperty) {
const tooltipText = getLocString('DataScience.clearFilters', 'Clear all filters');
ReactDOM.render(
<div
Expand Down
16 changes: 14 additions & 2 deletions src/test/datascience/dataviewer.functional.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,19 @@ suite('DataScience DataViewer tests', () => {
assert.ok(dv, 'DataViewer not created');
await gotAllRows;

verifyRows(wrapper.wrapper, [0, 0, 1, 1, 2, 2, 3, 3]);
verifyRows(wrapper.wrapper, [0, 0, 0, 1, 1, 1, 2, 2, 2, 3, 3, 3]);
});

runMountedTest('Transposed Data Frame', async (wrapper) => {
await injectCode(
'import pandas as pd\r\ndata = [["tom", 10], ["nick", 15], ["juli", 14]]\r\ndf = pd.DataFrame(data, columns=["Name", "Age"])\r\ndf = df.transpose()'
);
const gotAllRows = getCompletedPromise(wrapper);
const dv = await createJupyterVariableDataViewer('df', 'DataFrame');
assert.ok(dv, 'DataViewer not created');
await gotAllRows;

verifyRows(wrapper.wrapper, [0, 'Name', 'tom', 'nick', 'juli', 1, 'Age', '10', '15', '14']);
});

runMountedTest('List', async (wrapper) => {
Expand All @@ -304,7 +316,7 @@ suite('DataScience DataViewer tests', () => {
assert.ok(dv, 'DataViewer not created');
await gotAllRows;

verifyRows(wrapper.wrapper, [0, 0, 1, 1, 2, 2, 3, 3]);
verifyRows(wrapper.wrapper, [0, 0, 0, 1, 1, 1, 2, 2, 2, 3, 3, 3]);
});

runMountedTest('np.array', async (wrapper) => {
Expand Down