Point fix for duplicated data in data viewer (#5259)

* More data viewer style tweaks (#5055) * Add slice data functional tests (#5057) * Changelog and news entry * Fix smoke tests * Lint Co-authored-by: Don Jayamanne <[email protected]>
microsoft · Mar 24, 2021 · 8cff767 · 8cff767
1 parent 36f22e3
commit 8cff767
Show file tree

Hide file tree

Showing 16 changed files with 555 additions and 87 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,17 @@
 # Changelog
 
+## 2021.3.1 (24 March 2021)
+
+### Fixes
+
+1. Fix duplicate rows being fetched into data viewer for large data.
+   ([#5200](https://github.com/Microsoft/vscode-jupyter/issues/5200))
+
+### Code Health
+
+1. Add tests for data viewer slice data functionality.
+   ([#5066](https://github.com/Microsoft/vscode-jupyter/issues/5066))
+
 ## 2021.3.0 (3 March 2021)
 
 ### Enhancements

diff --git a/news/2 Fixes/5200.md b/news/2 Fixes/5200.md
@@ -0,0 +1 @@
+Fix duplicate rows being fetched into data viewer for large data.
diff --git a/news/3 Code Health/5066.md b/news/3 Code Health/5066.md
@@ -0,0 +1 @@
+Add tests for data viewer slice data functionality.
diff --git 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
@@ -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/cellFormatter.css b/src/datascience-ui/data-explorer/cellFormatter.css
@@ -11,3 +11,7 @@
     text-overflow: ellipsis;
     overflow: hidden;
 }
+
+.index-column-formatter {
+    text-align: right;
+}
diff --git a/src/datascience-ui/data-explorer/cellFormatter.tsx b/src/datascience-ui/data-explorer/cellFormatter.tsx
@@ -24,17 +24,14 @@ class CellFormatter extends React.Component<ICellFormatterProps> {
             // eslint-disable-next-line @typescript-eslint/no-explicit-any
             const columnType = (this.props.columnDef as any).type;
             switch (columnType) {
-                case ColumnType.Bool:
-                    return this.renderBool(this.props.value as boolean);
-
                 case ColumnType.Number:
                     return this.renderNumber(this.props.value as number);
 
                 default:
                     break;
             }
         }
-        // Otherwise an unknown type or a string
+        // Otherwise an unknown type, boolean, or a string
         const val = this.props.value?.toString() ?? '';
         return (
             <div className="cell-formatter" role="gridcell" title={val}>
@@ -43,18 +40,16 @@ class CellFormatter extends React.Component<ICellFormatterProps> {
         );
     }
 
-    private renderBool(value: boolean) {
-        return (
-            <div className="cell-formatter" role="gridcell" title={value.toString()}>
-                <span>{value.toString()}</span>
-            </div>
-        );
-    }
-
     private renderNumber(value: number) {
         let val = generateDisplayValue(value);
+        const isIndexColumn = this.props.columnDef.id === '0';
+
         return (
-            <div className="number-formatter cell-formatter" role="gridcell" title={val}>
+            <div
+                className={`number-formatter cell-formatter${isIndexColumn ? ' index-column-formatter' : ''}`}
+                role="gridcell"
+                title={val}
+            >
                 <span>{val}</span>
             </div>
         );

diff --git 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.css b/src/datascience-ui/data-explorer/mainPanel.css
@@ -16,6 +16,9 @@
     padding-left: 16px;
     overflow: auto;
     white-space: nowrap;
+    border-bottom-color: var(--vscode-editor-inactiveSelectionBackground);
+    border-bottom-style: solid;
+    border-bottom-width: 1px;
 }
 
 .breadcrumb {
@@ -39,3 +42,7 @@
     font-size: 18px;
     padding-right: 2px;
 }
+
+.breadcrumb-codicon {
+    color: var(--vscode-breadcrumb-foreground);
+}
diff --git a/src/datascience-ui/data-explorer/mainPanel.tsx b/src/datascience-ui/data-explorer/mainPanel.tsx
@@ -27,13 +27,15 @@ import { StyleInjector } from '../react-common/styleInjector';
 import { cellFormatterFunc } from './cellFormatter';
 import { ISlickGridAdd, ISlickGridSlice, ISlickRow, ReactSlickGrid } from './reactSlickGrid';
 import { generateTestData } from './testData';
-import { Image, ImageName } from '../react-common/image';
 
 import '../react-common/codicon/codicon.css';
 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<string> = new Set<string>(['ndarray', 'Tensor', 'EagerTensor']);
 
 // Our css has to come after in order to override body styles
@@ -192,12 +194,7 @@ export class MainPanel extends React.Component<IMainPanelProps, IMainPanelState>
                         <div className="icon-python breadcrumb-file-icon" />
                         <span>{this.state.fileName}</span>
                         {this.state.fileName ? (
-                            <Image
-                                baseTheme={this.props.baseTheme}
-                                class="image-button-image"
-                                codicon="chevron-right"
-                                image={ImageName.Cancel}
-                            />
+                            <div className="codicon codicon-chevron-right breadcrumb-codicon" />
                         ) : undefined}
                         <span>{breadcrumbText}</span>
                     </div>

diff --git a/src/datascience-ui/data-explorer/reactSlickGrid.css b/src/datascience-ui/data-explorer/reactSlickGrid.css
@@ -4,9 +4,7 @@
 }
 
 .react-grid-container {
-    border-color: var(--vscode-editor-inactiveSelectionBackground);
-    border-style: solid;
-    border-width: 1px;
+    border: none;
 }
 
 .react-grid-measure {
@@ -17,19 +15,17 @@
 .react-grid-header-cell {
     padding: 0px 4px;
     background-color: var(--vscode-menu-background);
-    color: var(--vscode-editor-foreground);
+    color: var(--vscode-menu-foreground);
     text-align: left;
     font-weight: bold;
-    border-right-color: var(--vscode-editor-inactiveSelectionBackground);
+    border-right: 1px solid transparent;
 }
 
 .react-grid-cell {
     padding: 4px;
     background-color: var(--vscode-editor-background);
     color: var(--vscode-editor-foreground);
-    border-bottom-color: var(--vscode-editor-inactiveSelectionBackground);
-    border-right-color: var(--vscode-editor-inactiveSelectionBackground);
-    border-right-style: solid;
+    border-right: 1px solid transparent;
     box-sizing: border-box;
 }
 
@@ -41,9 +37,8 @@
 /* Some overrides necessary to get the colors we want */
 .slick-headerrow-column {
     background-color: var(--vscode-menu-background);
-    border-right-color: var(--vscode-editor-inactiveSelectionBackground);
-    border-right-style: solid;
-    border-bottom-color: var(--vscode-menu-background);
+    color: var(--vscode-menu-foreground);
+    border-right: 1px solid transparent;
 }
 
 .slick-headerrow-column.ui-state-default {
@@ -55,7 +50,7 @@
 
 .slick-header-column.ui-state-default,
 .slick-group-header-column.ui-state-default {
-    border-right-color: var(--vscode-editor-inactiveSelectionBackground);
+    border-right: 1px solid transparent;
     display: flex;
 }
 
@@ -76,21 +71,31 @@
 }
 
 .react-grid-header-cell:hover {
-    background-color: var(--vscode-editor-inactiveSelectionBackground);
+    background-color: var(--override-selection-background, var(--vscode-list-hoverBackground));
 }
 
 .react-grid-header-cell > .slick-sort-indicator-asc::before {
     background: none;
-    font: normal normal normal 16px/1 codicon;
+    font: normal normal normal 10px/1 codicon;
     content: '\eaa1'; /* VS Code arrow-up codicon */
     align-items: center;
+    text-align: right;
+    display: flex;
+    padding: 3px;
+    color: var(--vscode-menu-foreground);
+    opacity: 0.4;
 }
 
 .react-grid-header-cell > .slick-sort-indicator-desc::before {
     background: none;
-    font: normal normal normal 16px/1 codicon;
+    font: normal normal normal 10px/1 codicon;
     content: '\ea9a'; /* VS Code arrow-down codicon */
     align-items: center;
+    text-align: right;
+    display: flex;
+    padding: 3px;
+    color: var(--vscode-menu-foreground);
+    opacity: 0.4;
 }
 
 .slick-row:hover > .react-grid-cell {
@@ -108,14 +113,18 @@ input.editor-text {
     outline: 0 none;
     border-style: solid;
     background-color: var(--vscode-list-activeSelectionBackground);
-    color: var(--vscode-button-foreground);
+    color: var(--vscode-list-activeSelectionForeground);
     text-align: left;
     /* input does not inherit font from body */
     font-family: var(--vscode-editor-font-family);
     font-weight: var(--vscode-editor-font-weight);
     font-size: var(--vscode-editor-font-size);
 }
 
+.slick-cell {
+    border: 1px solid var(--vscode-editor-lineHighlightBorder);
+}
+
 .slick-cell.editable {
     border-color: none;
     border-style: none;
@@ -125,9 +134,6 @@ input.editor-text {
 }
 
 .control-container {
-    border-bottom-color: var(--vscode-editor-inactiveSelectionBackground);
-    border-bottom-style: solid;
-    border-bottom-width: 1px;
     padding: 6px;
     display: flex;
     justify-content: start;
@@ -136,6 +142,9 @@ input.editor-text {
 
 .codicon-button {
     cursor: pointer;
+    padding-left: 3px;
+    padding-top: 3px;
+    color: var(--vscode-menu-foreground);
 }
 
 .header-cell-button {

diff --git a/src/datascience-ui/data-explorer/reactSlickGridFilterBox.tsx b/src/datascience-ui/data-explorer/reactSlickGridFilterBox.tsx
@@ -18,6 +18,21 @@ const filterIcon: IIconProps = {
     }
 };
 
+const styles = {
+    iconContainer: {
+        opacity: 0.4,
+        ':active': {
+            opacity: 0
+        }
+    },
+    root: {
+        '::after': {
+            borderRadius: '0px',
+            border: '1px solid var(--vscode-focusBorder)'
+        }
+    }
+};
+
 interface IFilterProps {
     column: Slick.Column<Slick.SlickData>;
     fontSize: number;
@@ -39,6 +54,7 @@ export class ReactSlickGridFilterBox extends React.Component<IFilterProps> {
                 tabIndex={0}
                 ariaLabel={this.props.column.name}
                 className="filter-box"
+                styles={styles}
                 value={this.props.filter}
             />
         );
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		Fix duplicate rows being fetched into data viewer for large data.
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		Add tests for data viewer slice data functionality.