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.

Sign up for GitHub

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

Add Boolean filters to the Data Explorer #2949

Merged
merged 8 commits into from
May 2, 2024
Merged

Add Boolean filters to the Data Explorer #2949

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
9c892fb
apply to boolean columns
jmcphers Apr 30, 2024
1f0399b
update row filter contract types
jmcphers Apr 30, 2024
d3cdb47
add row filter descriptors
jmcphers Apr 30, 2024
ba8a7e0
add to union type
jmcphers Apr 30, 2024
642f314
bind to filter UI
jmcphers Apr 30, 2024
188fecb
implement boolean filters in python
jmcphers Apr 30, 2024
f5bec73
python tests
jmcphers Apr 30, 2024
f444e43
update supported versions; satisfy ruff
jmcphers Apr 30, 2024
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
35 changes: 18 additions & 17 deletions extensions/positron-python/python_files/positron/positron_ipykernel/data_explorer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,31 +8,22 @@
import abc
import logging
import operator
from typing import (
TYPE_CHECKING,
Callable,
Dict,
List,
Optional,
Sequence,
Set,
Tuple,
)
from typing import TYPE_CHECKING, Callable, Dict, List, Optional, Sequence, Set, Tuple

import comm

from .access_keys import decode_access_key
from .data_explorer_comm import (
BackendState,
ColumnDisplayType,
ColumnFrequencyTable,
ColumnHistogram,
ColumnSummaryStats,
CompareFilterParamsOp,
ColumnProfileType,
ColumnProfileResult,
ColumnProfileType,
ColumnSchema,
ColumnDisplayType,
ColumnSortKey,
ColumnSummaryStats,
CompareFilterParamsOp,
DataExplorerBackendMessageContent,
DataExplorerFrontendEvent,
FilterResult,
Expand Down Expand Up @@ -63,7 +54,6 @@
from .third_party import pd_
from .utils import guid


if TYPE_CHECKING:
import pandas as pd

Expand Down Expand Up @@ -753,6 +743,11 @@ def _is_supported_filter(self, filt: RowFilter) -> bool:
RowFilterType.NotBetween,
]:
return display_type in _FILTER_RANGE_COMPARE_SUPPORTED
elif filt.filter_type in [
RowFilterType.IsTrue,
RowFilterType.IsFalse,
]:
return display_type == ColumnDisplayType.Boolean
else:
# Filters always supported
assert filt.filter_type in [
Expand Down Expand Up @@ -797,6 +792,10 @@ def _eval_filter(self, filt: RowFilter):
mask = col.str.len() != 0
elif filt.filter_type == RowFilterType.NotNull:
mask = col.notnull()
elif filt.filter_type == RowFilterType.IsTrue:
mask = col == True # noqa: E712
elif filt.filter_type == RowFilterType.IsFalse:
mask = col == False # noqa: E712
elif filt.filter_type == RowFilterType.SetMembership:
params = filt.set_membership_params
assert params is not None
Expand Down Expand Up @@ -955,10 +954,12 @@ def _prof_histogram(self, column_index: int):
RowFilterType.Between,
RowFilterType.Compare,
RowFilterType.IsEmpty,
RowFilterType.NotEmpty,
RowFilterType.IsFalse,
RowFilterType.IsNull,
RowFilterType.NotNull,
RowFilterType.IsTrue,
RowFilterType.NotBetween,
RowFilterType.NotEmpty,
RowFilterType.NotNull,
RowFilterType.Search,
RowFilterType.SetMembership,
}
Expand Down
4 changes: 4 additions & 0 deletions extensions/positron-python/python_files/positron/positron_ipykernel/data_explorer_comm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,12 @@ class RowFilterType(str, enum.Enum):

IsEmpty = "is_empty"

IsFalse = "is_false"

IsNull = "is_null"

IsTrue = "is_true"

NotBetween = "not_between"

NotEmpty = "not_empty"
Expand Down
25 changes: 20 additions & 5 deletions ...ions/positron-python/python_files/positron/positron_ipykernel/tests/test_data_explorer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,15 @@
from ..data_explorer import COMPARE_OPS, DataExplorerService
from ..data_explorer_comm import (
ColumnDisplayType,
RowFilter,
ColumnProfileResult,
ColumnSchema,
ColumnSortKey,
FilterResult,
RowFilter,
)
from ..utils import guid
from .conftest import DummyComm, PositronShell
from .test_variables import BIG_ARRAY_LENGTH
from ..utils import guid
from .utils import json_rpc_notification, json_rpc_request, json_rpc_response

TARGET_NAME = "positron.dataExplorer"
Expand Down Expand Up @@ -459,13 +459,15 @@ def test_pandas_supported_features(dxf: DataExplorerFixture):
assert row_filters["supported"]
assert row_filters["supports_conditions"]
assert set(row_filters["supported_types"]) == {
"between",
"compare",
"is_empty",
"is_false",
"is_null",
"is_true",
"not_between",
"not_empty",
"not_null",
"between",
"compare",
"not_between",
"search",
"set_membership",
}
Expand Down Expand Up @@ -850,6 +852,19 @@ def test_pandas_filter_empty(dxf: DataExplorerFixture):
dxf.check_filter_case(df, [_filter("not_empty", schema[1])], df[df["b"].str.len() != 0])


def test_pandas_filter_boolean(dxf: DataExplorerFixture):
df = pd.DataFrame(
{
"a": [True, True, None, False, False, False, True, True],
}
)

schema = dxf.get_schema_for(df)["columns"]

dxf.check_filter_case(df, [_filter("is_true", schema[0])], df[df["a"] == True]) # noqa: E712
dxf.check_filter_case(df, [_filter("is_false", schema[0])], df[df["a"] == False]) # noqa: E712


def test_pandas_filter_is_null_not_null(dxf: DataExplorerFixture):
df = SIMPLE_PANDAS_DF
schema = dxf.get_schema_for(df)["columns"]
Expand Down
2 changes: 2 additions & 0 deletions positron/comms/data_explorer-backend-openrpc.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,9 @@
"between",
"compare",
"is_empty",
"is_false",
"is_null",
"is_true",
"not_between",
"not_empty",
"not_null",
Expand Down
36 changes: 35 additions & 1 deletion ...ts/dataExplorerPanel/components/addEditRowFilterModalPopup/addEditRowFilterModalPopup.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ import {
RowFilterDescriptorIsNotNull,
RowFilterDescriptorIsNull,
RowFilterDescriptorSearch,
RowFilterDescriptorIsTrue,
RowFilterDescriptorIsFalse,
} from 'vs/workbench/browser/positronDataExplorer/components/dataExplorerPanel/components/addEditRowFilterModalPopup/rowFilterDescriptor';

/**
Expand Down Expand Up @@ -94,6 +96,8 @@ const filterNumParams = (filterType: RowFilterDescrType | undefined) => {
case RowFilterDescrType.IS_NOT_EMPTY:
case RowFilterDescrType.IS_NULL:
case RowFilterDescrType.IS_NOT_NULL:
case RowFilterDescrType.IS_TRUE:
case RowFilterDescrType.IS_FALSE:
return 0;
case RowFilterDescrType.IS_EQUAL_TO:
case RowFilterDescrType.IS_NOT_EQUAL_TO:
Expand Down Expand Up @@ -310,7 +314,6 @@ export const AddEditRowFilterModalPopup = (props: AddEditRowFilterModalPopupProp
// Add is equal to, is not equal to conditions.
switch (selectedColumnSchema.type_display) {
case ColumnDisplayType.Number:
case ColumnDisplayType.Boolean:
case ColumnDisplayType.String:
case ColumnDisplayType.Date:
case ColumnDisplayType.Datetime:
Expand All @@ -334,6 +337,27 @@ export const AddEditRowFilterModalPopup = (props: AddEditRowFilterModalPopupProp
break;
}

// Add is true, is false conditions.
switch (selectedColumnSchema.type_display) {
case ColumnDisplayType.Boolean:
conditionEntries.push(new DropDownListBoxItem({
identifier: RowFilterDescrType.IS_TRUE,
title: localize(
'positron.addEditRowFilter.conditionIsTrue',
"is true"
),
value: RowFilterDescrType.IS_TRUE
}));
Comment on lines +343 to +350
Copy link
Contributor

Choose a reason for hiding this comment

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

The fact these are all the same makes me wonder if a new class could be made for each condition ala how ContextKeyExpression works.
E.g.

export class DropDownListBoxTrue extends DropDownListBoxItem<RowFilterDescrType.IS_TRUE, string> {
	constructor() {
		super(
			{
				identifier: RowFilterDescrType.IS_TRUE,
				title: localize(
					'positron.addEditRowFilter.conditionIsTrue',
					"is true"
				),
				value: RowFilterDescrType.IS_TRUE
			});
	}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Not super applicable to this PR though

Copy link
Contributor

Choose a reason for hiding this comment

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

This whole file is chock full of code duplication. I did a little refactoring a week or two ago to reduce some of it, but it would be good to modularize and make the list of "what filters are supported for each type" more declarative / easier to edit and less ad hoc. Same with the row filter descriptor classes which feel more verbose than is needed

conditionEntries.push(new DropDownListBoxItem({
identifier: RowFilterDescrType.IS_FALSE,
title: localize(
'positron.addEditRowFilter.conditionIsFalse',
"is false"
),
value: RowFilterDescrType.IS_FALSE
}));
}

// Add is between / is not between conditions.
switch (selectedColumnSchema.type_display) {
case ColumnDisplayType.Number:
Expand Down Expand Up @@ -608,6 +632,16 @@ export const AddEditRowFilterModalPopup = (props: AddEditRowFilterModalPopupProp
break;
}

// Apply boolean filters.
case RowFilterDescrType.IS_TRUE: {
applyRowFilter(new RowFilterDescriptorIsTrue({ columnSchema: selectedColumnSchema }));
break;
}
case RowFilterDescrType.IS_FALSE: {
applyRowFilter(new RowFilterDescriptorIsFalse({ columnSchema: selectedColumnSchema }));
break;
}

// Apply comparison row filter.
case RowFilterDescrType.SEARCH_CONTAINS:
case RowFilterDescrType.SEARCH_STARTS_WITH:
Expand Down
70 changes: 69 additions & 1 deletion ...components/dataExplorerPanel/components/addEditRowFilterModalPopup/rowFilterDescriptor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ export enum RowFilterDescrType {
IS_NOT_EMPTY = 'is-not-empty',
IS_NULL = 'is-null',
IS_NOT_NULL = 'is-not-null',
IS_TRUE = 'is-true',
IS_FALSE = 'is-false',

// Filters with one parameter.
IS_LESS_THAN = 'is-less-than',
Expand Down Expand Up @@ -172,6 +174,66 @@ export class RowFilterDescriptorIsNull extends BaseRowFilterDescriptor {
}
}

/**
* RowFilterDescriptorIsTrue class.
*/
export class RowFilterDescriptorIsTrue extends BaseRowFilterDescriptor {
/**
* Constructor.
* @param props The common row filter descriptor properties.
*/
constructor(props: RowFilterCommonProps) {
super(props);
}

/**
* Gets the row filter condition.
*/
get descrType() {
return RowFilterDescrType.IS_TRUE;
}

/**
* Get the backend OpenRPC type.
*/
get backendFilter() {
return {
filter_type: RowFilterType.IsTrue,
...this._sharedBackendParams()
};
}
}

/**
* RowFilterDescriptorIsFalse class.
*/
export class RowFilterDescriptorIsFalse extends BaseRowFilterDescriptor {
/**
* Constructor.
* @param props The common row filter descriptor properties.
*/
constructor(props: RowFilterCommonProps) {
super(props);
}

/**
* Gets the row filter condition.
*/
get descrType() {
return RowFilterDescrType.IS_FALSE;
}

/**
* Get the backend OpenRPC type.
*/
get backendFilter() {
return {
filter_type: RowFilterType.IsFalse,
...this._sharedBackendParams()
};
}
}

/**
* RowFilterDescriptorIsNotEmpty class.
*/
Expand Down Expand Up @@ -525,7 +587,7 @@ function getSearchDescrType(searchType: SearchFilterType) {
}
}

export function getRowFilterDescriptor(backendFilter: RowFilter) {
export function getRowFilterDescriptor(backendFilter: RowFilter): RowFilterDescriptor {
const commonProps = {
columnSchema: backendFilter.column_schema,
isValid: backendFilter.is_valid,
Expand Down Expand Up @@ -558,6 +620,10 @@ export function getRowFilterDescriptor(backendFilter: RowFilter) {
return new RowFilterDescriptorIsNull(commonProps);
case RowFilterType.NotNull:
return new RowFilterDescriptorIsNotNull(commonProps);
case RowFilterType.IsTrue:
return new RowFilterDescriptorIsTrue(commonProps);
case RowFilterType.IsFalse:
return new RowFilterDescriptorIsFalse(commonProps);
case RowFilterType.Search: {
const params = backendFilter.search_params!;
return new RowFilterDescriptorSearch(commonProps,
Expand All @@ -581,6 +647,8 @@ export type RowFilterDescriptor =
RowFilterDescriptorIsNotEmpty |
RowFilterDescriptorIsNull |
RowFilterDescriptorIsNotNull |
RowFilterDescriptorIsTrue |
RowFilterDescriptorIsFalse |
RowFilterDescriptorIsBetween |
RowFilterDescriptorIsNotBetween |
RowFilterDescriptorSearch;
17 changes: 16 additions & 1 deletion ...lorer/components/dataExplorerPanel/components/rowFilterBar/components/rowFilterWidget.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,12 @@ import {
RowFilterDescriptorComparison,
RowFilterDescriptorIsBetween,
RowFilterDescriptorIsEmpty,
RowFilterDescriptorIsFalse,
RowFilterDescriptorIsNotBetween,
RowFilterDescriptorIsNotEmpty,
RowFilterDescriptorIsNotNull,
RowFilterDescriptorIsNull,
RowFilterDescriptorIsTrue,
RowFilterDescriptorSearch
} from 'vs/workbench/browser/positronDataExplorer/components/dataExplorerPanel/components/addEditRowFilterModalPopup/rowFilterDescriptor';

Expand Down Expand Up @@ -70,7 +72,20 @@ export const RowFilterWidget = forwardRef<HTMLButtonElement, RowFilterWidgetProp
{localize('positron.dataExplorer.rowFilterWidget.isNotNull', "is not null")}
</span>
</>;

} else if (props.rowFilter instanceof RowFilterDescriptorIsTrue) {
return <>
<span className='column-name'>{props.rowFilter.schema.column_name}</span>
<span className='space-before'>
{localize('positron.dataExplorer.rowFilterWidget.isTrue', "is true")}
</span>
</>;
} else if (props.rowFilter instanceof RowFilterDescriptorIsFalse) {
return <>
<span className='column-name'>{props.rowFilter.schema.column_name}</span>
<span className='space-before'>
{localize('positron.dataExplorer.rowFilterWidget.isFalse', "is false")}
</span>
</>;
} else if (props.rowFilter instanceof RowFilterDescriptorComparison) {
return <>
<span className='column-name'>{props.rowFilter.schema.column_name}</span>
Expand Down
2 changes: 2 additions & 0 deletions src/vs/workbench/services/languageRuntime/common/positronDataExplorerComm.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -620,7 +620,9 @@ export enum RowFilterType {
Between = 'between',
Compare = 'compare',
IsEmpty = 'is_empty',
IsFalse = 'is_false',
IsNull = 'is_null',
IsTrue = 'is_true',
NotBetween = 'not_between',
NotEmpty = 'not_empty',
NotNull = 'not_null',
Expand Down
Loading