Skip to content

Commit

Permalink
Wire up row filtering UI with backend RPCs in data explorer (#2728)
Browse files Browse the repository at this point in the history 
* Row filters are hooked up

* Take into account filters when returning table shape

* Add clearing filter support

---------

Co-authored-by: Wes McKinney <[email protected]>
  • Loading branch information
@softwarenerd @wesm
softwarenerd and wesm authored Apr 10, 2024
1 parent c5d799f commit c5a007f
Show file tree
Hide file tree
Showing 8 changed files with 251 additions and 94 deletions.
9 changes: 7 additions & 2 deletions extensions/positron-python/python_files/positron/positron_ipykernel/data_explorer.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ def get_data_values(self, request: GetDataValuesRequest):
).dict()

def set_row_filters(self, request: SetRowFiltersRequest):
return self._set_row_filters(request.params.filters)
return self._set_row_filters(request.params.filters).dict()

def set_sort_columns(self, request: SetSortColumnsRequest):
self.sort_keys = request.params.sort_keys
Expand Down Expand Up @@ -688,8 +688,13 @@ def _prof_histogram(self, column_index: int):
raise NotImplementedError

def _get_state(self) -> TableState:
if self.view_indices is not None:
num_rows = len(self.view_indices)
else:
num_rows = self.table.shape[0]

return TableState(
table_shape=TableShape(num_rows=self.table.shape[0], num_columns=self.table.shape[1]),
table_shape=TableShape(num_rows=num_rows, num_columns=self.table.shape[1]),
row_filters=self.filters,
sort_keys=self.sort_keys,
)
Expand Down
6 changes: 5 additions & 1 deletion ...ions/positron-python/python_files/positron/positron_ipykernel/tests/test_data_explorer.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,11 @@ def check_filter_case(self, table, filter_set, expected_table):
self.register_table(ex_id, expected_table)

response = self.set_row_filters(table_id, filters=filter_set)
assert response == FilterResult(selected_num_rows=len(expected_table))

ex_num_rows = len(expected_table)
assert response == FilterResult(selected_num_rows=ex_num_rows)

assert self.get_state(table_id)["table_shape"]["num_rows"] == ex_num_rows
self.compare_tables(table_id, ex_id, table.shape)

def check_sort_case(self, table, sort_keys, expected_table, filters=None):
Expand Down
37 changes: 23 additions & 14 deletions ...ts/dataExplorerPanel/components/addEditRowFilterModalPopup/addEditRowFilterModalPopup.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { DataExplorerClientInstance } from 'vs/workbench/services/languageRuntim
import { DropDownListBox, DropDownListBoxEntry } from 'vs/workbench/browser/positronComponents/dropDownListBox/dropDownListBox';
import { RowFilterParameter } from 'vs/workbench/browser/positronDataExplorer/components/dataExplorerPanel/components/addEditRowFilterModalPopup/components/rowFilterParameter';
import { DropDownColumnSelector } from 'vs/workbench/browser/positronDataExplorer/components/dataExplorerPanel/components/addEditRowFilterModalPopup/components/dropDownColumnSelector';
import { RangeRowFilter, RowFilter, RowFilterCondition, RowFilterIsBetween, RowFilterIsEmpty, RowFilterIsEqualTo, RowFilterIsGreaterThan, RowFilterIsLessThan, RowFilterIsNotBetween, RowFilterIsNotEmpty, SingleValueRowFilter } from 'vs/workbench/browser/positronDataExplorer/components/dataExplorerPanel/components/addEditRowFilterModalPopup/rowFilter';
import { RangeRowFilterDescriptor, RowFilterDescriptor, RowFilterCondition, RowFilterDescriptorIsBetween, RowFilterDescriptorIsEmpty, RowFilterDescriptorIsEqualTo, RowFilterDescriptorIsGreaterThan, RowFilterDescriptorIsLessThan, RowFilterDescriptorIsNotBetween, RowFilterDescriptorIsNotEmpty, SingleValueRowFilterDescriptor } from 'vs/workbench/browser/positronDataExplorer/components/dataExplorerPanel/components/addEditRowFilterModalPopup/rowFilterDescriptor';

/**
* Validates a row filter value.
Expand Down Expand Up @@ -81,8 +81,8 @@ interface AddEditRowFilterModalPopupProps {
dataExplorerClientInstance: DataExplorerClientInstance;
renderer: PositronModalReactRenderer;
anchor: HTMLElement;
editRowFilter?: RowFilter;
onApplyRowFilter: (rowFilter: RowFilter) => void;
editRowFilter?: RowFilterDescriptor;
onApplyRowFilter: (rowFilter: RowFilterDescriptor) => void;
}

/**
Expand All @@ -103,16 +103,16 @@ export const AddEditRowFilterModalPopup = (props: AddEditRowFilterModalPopupProp
props.editRowFilter?.rowFilterCondition
);
const [firstRowFilterValue, setFirstRowFilterValue] = useState<string>(() => {
if (props.editRowFilter instanceof SingleValueRowFilter) {
if (props.editRowFilter instanceof SingleValueRowFilterDescriptor) {
return props.editRowFilter.value;
} else if (props.editRowFilter instanceof RangeRowFilter) {
} else if (props.editRowFilter instanceof RangeRowFilterDescriptor) {
return props.editRowFilter.lowerLimit;
} else {
return '';
}
});
const [secondRowFilterValue, setSecondRowFilterValue] = useState<string>(() => {
if (props.editRowFilter instanceof RangeRowFilter) {
if (props.editRowFilter instanceof RangeRowFilterDescriptor) {
return props.editRowFilter.upperLimit;
} else {
return '';
Expand Down Expand Up @@ -449,7 +449,7 @@ export const AddEditRowFilterModalPopup = (props: AddEditRowFilterModalPopupProp
* Applies a row filter.
* @param rowFilter The row filter to add.
*/
const applyRowFilter = (rowFilter: RowFilter) => {
const applyRowFilter = (rowFilter: RowFilterDescriptor) => {
setErrorText(undefined);
props.renderer.dispose();
props.onApplyRowFilter(rowFilter);
Expand All @@ -459,13 +459,13 @@ export const AddEditRowFilterModalPopup = (props: AddEditRowFilterModalPopupProp
switch (selectedCondition) {
// Apply the is empty row filter.
case RowFilterCondition.CONDITION_IS_EMPTY: {
applyRowFilter(new RowFilterIsEmpty(selectedColumnSchema));
applyRowFilter(new RowFilterDescriptorIsEmpty(selectedColumnSchema));
break;
}

// Apply the is not empty row filter.
case RowFilterCondition.CONDITION_IS_NOT_EMPTY: {
applyRowFilter(new RowFilterIsNotEmpty(selectedColumnSchema));
applyRowFilter(new RowFilterDescriptorIsNotEmpty(selectedColumnSchema));
break;
}

Expand All @@ -474,7 +474,10 @@ export const AddEditRowFilterModalPopup = (props: AddEditRowFilterModalPopupProp
if (!validateFirstRowFilterValue()) {
return;
}
applyRowFilter(new RowFilterIsLessThan(selectedColumnSchema, firstRowFilterValue));
applyRowFilter(new RowFilterDescriptorIsLessThan(
selectedColumnSchema,
firstRowFilterValue
));
break;
}

Expand All @@ -483,7 +486,10 @@ export const AddEditRowFilterModalPopup = (props: AddEditRowFilterModalPopupProp
if (!validateFirstRowFilterValue()) {
return;
}
applyRowFilter(new RowFilterIsGreaterThan(selectedColumnSchema, firstRowFilterValue));
applyRowFilter(new RowFilterDescriptorIsGreaterThan(
selectedColumnSchema,
firstRowFilterValue
));
break;
}

Expand All @@ -492,7 +498,10 @@ export const AddEditRowFilterModalPopup = (props: AddEditRowFilterModalPopupProp
if (!validateFirstRowFilterValue()) {
return;
}
applyRowFilter(new RowFilterIsEqualTo(selectedColumnSchema, firstRowFilterValue));
applyRowFilter(new RowFilterDescriptorIsEqualTo(
selectedColumnSchema,
firstRowFilterValue
));
break;
}

Expand All @@ -504,7 +513,7 @@ export const AddEditRowFilterModalPopup = (props: AddEditRowFilterModalPopupProp
if (!validateSecondRowFilterValue()) {
return;
}
applyRowFilter(new RowFilterIsBetween(
applyRowFilter(new RowFilterDescriptorIsBetween(
selectedColumnSchema,
firstRowFilterValue,
secondRowFilterValue
Expand All @@ -520,7 +529,7 @@ export const AddEditRowFilterModalPopup = (props: AddEditRowFilterModalPopupProp
if (!validateSecondRowFilterValue()) {
return;
}
applyRowFilter(new RowFilterIsNotBetween(
applyRowFilter(new RowFilterDescriptorIsNotBetween(
selectedColumnSchema,
firstRowFilterValue,
secondRowFilterValue
Expand Down
57 changes: 29 additions & 28 deletions ...s/addEditRowFilterModalPopup/rowFilter.ts → ...owFilterModalPopup/rowFilterDescriptor.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ export enum RowFilterCondition {
}

/**
* BaseRowFilter class.
* BaseRowFilterDescriptor class.
*/
abstract class BaseRowFilter {
abstract class BaseRowFilterDescriptor {
/**
* Gets the identifier.
*/
Expand All @@ -47,9 +47,9 @@ abstract class BaseRowFilter {
}

/**
* RowFilterIsEmpty class.
* RowFilterDescriptorIsEmpty class.
*/
export class RowFilterIsEmpty extends BaseRowFilter {
export class RowFilterDescriptorIsEmpty extends BaseRowFilterDescriptor {
/**
* Constructor.
* @param columnSchema The column schema.
Expand All @@ -67,9 +67,9 @@ export class RowFilterIsEmpty extends BaseRowFilter {
}

/**
* RowFilterIsNotEmpty class.
* RowFilterDescriptorIsNotEmpty class.
*/
export class RowFilterIsNotEmpty extends BaseRowFilter {
export class RowFilterDescriptorIsNotEmpty extends BaseRowFilterDescriptor {
/**
* Constructor.
* @param columnSchema The column schema.
Expand All @@ -87,9 +87,9 @@ export class RowFilterIsNotEmpty extends BaseRowFilter {
}

/**
* SingleValueRowFilter class.
* SingleValueRowFilterDescriptor class.
*/
export abstract class SingleValueRowFilter extends BaseRowFilter {
export abstract class SingleValueRowFilterDescriptor extends BaseRowFilterDescriptor {
/**
* Constructor.
* @param columnSchema The column schema.
Expand All @@ -101,9 +101,9 @@ export abstract class SingleValueRowFilter extends BaseRowFilter {
}

/**
* RowFilterIsLessThan row filter.
* RowFilterDescriptorIsLessThan class.
*/
export class RowFilterIsLessThan extends SingleValueRowFilter {
export class RowFilterDescriptorIsLessThan extends SingleValueRowFilterDescriptor {
/**
* Constructor.
* @param columnSchema The column schema.
Expand All @@ -122,9 +122,9 @@ export class RowFilterIsLessThan extends SingleValueRowFilter {
}

/**
* RowFilterIsGreaterThan row filter.
* RowFilterDescriptorIsGreaterThan class.
*/
export class RowFilterIsGreaterThan extends SingleValueRowFilter {
export class RowFilterDescriptorIsGreaterThan extends SingleValueRowFilterDescriptor {
/**
* Constructor.
* @param columnSchema The column schema.
Expand All @@ -143,9 +143,9 @@ export class RowFilterIsGreaterThan extends SingleValueRowFilter {
}

/**
* RowFilterIsEqualTo row filter.
* RowFilterDescriptorIsEqualTo class.
*/
export class RowFilterIsEqualTo extends SingleValueRowFilter {
export class RowFilterDescriptorIsEqualTo extends SingleValueRowFilterDescriptor {
/**
* Constructor.
* @param columnSchema The column schema.
Expand All @@ -164,9 +164,9 @@ export class RowFilterIsEqualTo extends SingleValueRowFilter {
}

/**
* RangeRowFilter class.
* RangeRowFilterDescriptor class.
*/
export abstract class RangeRowFilter extends BaseRowFilter {
export abstract class RangeRowFilterDescriptor extends BaseRowFilterDescriptor {
/**
* Constructor.
* @param columnSchema The column schema.
Expand All @@ -183,9 +183,9 @@ export abstract class RangeRowFilter extends BaseRowFilter {
}

/**
* RowFilterIsBetween row filter.
* RowFilterDescriptorIsBetween class.
*/
export class RowFilterIsBetween extends RangeRowFilter {
export class RowFilterDescriptorIsBetween extends RangeRowFilterDescriptor {
/**
* Constructor.
* @param columnSchema The column schema.
Expand All @@ -205,9 +205,9 @@ export class RowFilterIsBetween extends RangeRowFilter {
}

/**
* RowFilterIsNotBetween row filter.
* RowFilterDescriptorIsNotBetween class.
*/
export class RowFilterIsNotBetween extends RangeRowFilter {
export class RowFilterDescriptorIsNotBetween extends RangeRowFilterDescriptor {
/**
* Constructor.
* @param columnSchema The column schema.
Expand All @@ -227,12 +227,13 @@ export class RowFilterIsNotBetween extends RangeRowFilter {
}

/**
* RowFilter type.
* RowFilterDescriptor type.
*/
export type RowFilter =
RowFilterIsEmpty |
RowFilterIsNotEmpty |
RowFilterIsLessThan |
RowFilterIsGreaterThan |
RowFilterIsBetween |
RowFilterIsNotBetween;
export type RowFilterDescriptor =
RowFilterDescriptorIsEmpty |
RowFilterDescriptorIsNotEmpty |
RowFilterDescriptorIsLessThan |
RowFilterDescriptorIsGreaterThan |
RowFilterDescriptorIsEqualTo |
RowFilterDescriptorIsBetween |
RowFilterDescriptorIsNotBetween;
18 changes: 9 additions & 9 deletions ...lorer/components/dataExplorerPanel/components/rowFilterBar/components/rowFilterWidget.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@ import { forwardRef } from 'react'; // eslint-disable-line no-duplicate-imports
// Other dependencies.
import { localize } from 'vs/nls';
import { Button } from 'vs/base/browser/ui/positronComponents/button/button';
import { RowFilter, RowFilterIsBetween, RowFilterIsEmpty, RowFilterIsEqualTo, RowFilterIsGreaterThan, RowFilterIsLessThan, RowFilterIsNotBetween, RowFilterIsNotEmpty } from 'vs/workbench/browser/positronDataExplorer/components/dataExplorerPanel/components/addEditRowFilterModalPopup/rowFilter';
import { RowFilterDescriptor, RowFilterDescriptorIsBetween, RowFilterDescriptorIsEmpty, RowFilterDescriptorIsEqualTo, RowFilterDescriptorIsGreaterThan, RowFilterDescriptorIsLessThan, RowFilterDescriptorIsNotBetween, RowFilterDescriptorIsNotEmpty } from 'vs/workbench/browser/positronDataExplorer/components/dataExplorerPanel/components/addEditRowFilterModalPopup/rowFilterDescriptor';

/**
* RowFilterWidgetProps interface.
*/
interface RowFilterWidgetProps {
rowFilter: RowFilter;
rowFilter: RowFilterDescriptor;
booleanOperator?: 'and';
onEdit: () => void;
onClear: () => void;
Expand All @@ -32,39 +32,39 @@ interface RowFilterWidgetProps {
export const RowFilterWidget = forwardRef<HTMLButtonElement, RowFilterWidgetProps>((props, ref) => {
// Compute the title.
const title = (() => {
if (props.rowFilter instanceof RowFilterIsEmpty) {
if (props.rowFilter instanceof RowFilterDescriptorIsEmpty) {
return <>
<span className='column-name'>{props.rowFilter.columnSchema.column_name}</span>
<span className='space-before'>
{localize('positron.dataExplorer.rowFilterWidget.isEmpty', "is empty")}
</span>
</>;
} else if (props.rowFilter instanceof RowFilterIsNotEmpty) {
} else if (props.rowFilter instanceof RowFilterDescriptorIsNotEmpty) {
return <>
<span className='column-name'>{props.rowFilter.columnSchema.column_name}</span>
<span className='space-before'>
{localize('positron.dataExplorer.rowFilterWidget.isNotEmpty', "is not empty")}
</span>
</>;
} else if (props.rowFilter instanceof RowFilterIsLessThan) {
} else if (props.rowFilter instanceof RowFilterDescriptorIsLessThan) {
return <>
<span className='column-name'>{props.rowFilter.columnSchema.column_name}</span>
<span className='space-before space-after'>&lt;</span>
<span>{props.rowFilter.value}</span>
</>;
} else if (props.rowFilter instanceof RowFilterIsGreaterThan) {
} else if (props.rowFilter instanceof RowFilterDescriptorIsGreaterThan) {
return <>
<span className='column-name'>{props.rowFilter.columnSchema.column_name}</span>
<span className='space-before space-after'>&gt;</span>
<span>{props.rowFilter.value}</span>
</>;
} else if (props.rowFilter instanceof RowFilterIsEqualTo) {
} else if (props.rowFilter instanceof RowFilterDescriptorIsEqualTo) {
return <>
<span className='column-name'>{props.rowFilter.columnSchema.column_name}</span>
<span className='space-before space-after'>=</span>
<span>{props.rowFilter.value}</span>
</>;
} else if (props.rowFilter instanceof RowFilterIsBetween) {
} else if (props.rowFilter instanceof RowFilterDescriptorIsBetween) {
return <>
<span className='column-name'>{props.rowFilter.columnSchema.column_name}</span>
<span className='space-before space-after'>&gt;=</span>
Expand All @@ -76,7 +76,7 @@ export const RowFilterWidget = forwardRef<HTMLButtonElement, RowFilterWidgetProp
<span className='space-before space-after'>&lt;=</span>
<span>{props.rowFilter.upperLimit}</span>
</>;
} else if (props.rowFilter instanceof RowFilterIsNotBetween) {
} else if (props.rowFilter instanceof RowFilterDescriptorIsNotBetween) {
return <>
<span className='column-name'>{props.rowFilter.columnSchema.column_name}</span>
<span className='space-before space-after'>&lt;</span>
Expand Down
Loading

0 comments on commit c5a007f

Please sign in to comment.