diff --git a/build/lib/stylelint/vscode-known-variables.json b/build/lib/stylelint/vscode-known-variables.json index f2a98889716..a5aa8c84149 100644 --- a/build/lib/stylelint/vscode-known-variables.json +++ b/build/lib/stylelint/vscode-known-variables.json @@ -544,6 +544,7 @@ "--vscode-positronDataExplorer-columnNullPercentGraphBackgroundFill", "--vscode-positronDataExplorer-columnNullPercentGraphBackgroundStroke", "--vscode-positronDataExplorer-columnNullPercentGraphIndicatorFill", + "--vscode-positronDataExplorer-invalidFilterBackground", "--vscode-positronDataExplorer-selectionBackground", "--vscode-positronDataGrid-background", "--vscode-positronDataGrid-border", diff --git a/extensions/positron-python/python_files/positron/positron_ipykernel/data_explorer.py b/extensions/positron-python/python_files/positron/positron_ipykernel/data_explorer.py index 773c26ca0c5..99f8e3e975f 100644 --- a/extensions/positron-python/python_files/positron/positron_ipykernel/data_explorer.py +++ b/extensions/positron-python/python_files/positron/positron_ipykernel/data_explorer.py @@ -44,7 +44,6 @@ RowFilter, RowFilterCondition, RowFilterType, - SchemaUpdateParams, SearchFilterType, SearchSchemaFeatures, SearchSchemaRequest, @@ -76,6 +75,7 @@ PathKey = Tuple[str, ...] +StateUpdate = Tuple[bool, List[RowFilter], List[ColumnSortKey]] class DataExplorerTableView(abc.ABC): @@ -137,12 +137,7 @@ def set_row_filters(self, request: SetRowFiltersRequest): return self._set_row_filters(request.params.filters).dict() def set_sort_columns(self, request: SetSortColumnsRequest): - self.sort_keys = request.params.sort_keys - - if not self._recompute_if_needed(): - # If a re-filter is pending, then it will automatically - # trigger a sort - self._sort_data() + return self._set_sort_columns(request.params.sort_keys) def get_column_profiles(self, request: GetColumnProfilesRequest): self._recompute_if_needed() @@ -167,7 +162,8 @@ def get_column_profiles(self, request: GetColumnProfilesRequest): return results - def get_state(self, request: GetStateRequest): + def get_state(self, _: GetStateRequest): + self._recompute_if_needed() return self._get_state().dict() @abc.abstractmethod @@ -175,11 +171,7 @@ def invalidate_computations(self): pass @abc.abstractmethod - def ui_should_update_schema(self, new_table) -> Tuple[bool, bool]: - pass - - @abc.abstractmethod - def ui_should_update_data(self, new_table) -> bool: + def get_updated_state(self, new_table) -> StateUpdate: pass @abc.abstractmethod @@ -205,6 +197,10 @@ def _get_data_values( def _set_row_filters(self, filters: List[RowFilter]) -> FilterResult: pass + @abc.abstractmethod + def _set_sort_columns(self, sort_keys: List[ColumnSortKey]): + pass + @abc.abstractmethod def _sort_data(self): pass @@ -240,6 +236,14 @@ def _pandas_format_values(col): return col.astype(str).tolist() +_FILTER_RANGE_COMPARE_SUPPORTED = { + ColumnDisplayType.Number, + ColumnDisplayType.Date, + ColumnDisplayType.Datetime, + ColumnDisplayType.Time, +} + + class PandasView(DataExplorerTableView): TYPE_DISPLAY_MAPPING = { "integer": "number", @@ -280,13 +284,15 @@ def __init__( ): super().__init__(display_name, table, filters, sort_keys) - self._dtypes = None - # Maintain a mapping of column index to inferred dtype for any # object columns, to avoid recomputing. If the underlying # object is changed, this needs to be reset self._inferred_dtypes = {} + # We store the column schemas for each sort key to help with + # eviction later during updates + self._sort_key_schemas: List[ColumnSchema] = [] + # NumPy array of selected ("true") indices using filters. If # there are also sort keys, we first filter the unsorted data, # and then sort the filtered data only, for the optimistic @@ -319,38 +325,198 @@ def invalidate_computations(self): self.filtered_indices = self.view_indices = None self._need_recompute = True - def ui_should_update_schema(self, new_table) -> Tuple[bool, bool]: - # Add smarter logic here later, but for now always update the - # schema + def get_updated_state(self, new_table) -> StateUpdate: + from pandas.api.types import infer_dtype - if self.table.columns.equals(new_table.columns): - update_schema = False - for i in range(len(self.table.columns)): - if self.table.iloc[:, i].dtype != new_table.iloc[:, i].dtype: - update_schema = True - break + filtered_columns = { + filt.column_schema.column_index: filt.column_schema for filt in self.filters + } + + # self.table may have been modified in place, so we cannot + # assume that new_table is different than self.table + if new_table is self.table: + # For in-place updates, we have to assume the worst case + # scenario of a schema change + schema_updated = True else: - update_schema = True + # The table object has changed -- now we look for + # suspected schema changes + schema_updated = False - discard_state = update_schema - return update_schema, discard_state + # We go through the columns in the new table and see whether + # there is a type change or whether a column name moved. + # + # TODO: duplicate column names are a can of worms here, and we + # will need to return to make this logic robust to that + old_columns = self.table.columns + shifted_columns: Dict[int, int] = {} + schema_changes: Dict[int, ColumnSchema] = {} + + # First, we look for detectable deleted columns + deleted_columns: Set[int] = set() + if not self.table.columns.equals(new_table.columns): + for old_index, column in enumerate(self.table.columns): + if column not in new_table.columns: + deleted_columns.add(old_index) + schema_updated = True + + def _get_column_schema(column, column_name, column_index): + # We only use infer_dtype for columns that are involved in + # a filter + type_name, type_display = self._get_type_display( + column.dtype, lambda: infer_dtype(column) + ) + + return ColumnSchema( + column_name=column_name, + column_index=column_index, + type_name=type_name, + type_display=type_display, + ) + + # When computing the new display type of a column requires + # calling infer_dtype, we are careful to only do it for + # columns that are involved in a filter + for new_index, column_name in enumerate(new_table.columns): + # New table has more columns than the old table + out_of_bounds = new_index >= len(old_columns) + + if out_of_bounds or old_columns[new_index] != column_name: + if column_name not in old_columns: + # New column + schema_updated = True + continue + # Column was shifted + old_index = old_columns.get_loc(column_name) + shifted_columns[old_index] = new_index + else: + old_index = new_index + + new_column = new_table.iloc[:, new_index] + + # For object dtype columns, we refuse to make any + # assumptions about whether the data type has changed + # and will let re-filtering fail later if there is a + # problem + if new_column.dtype == object: + # The inferred type could be different + schema_updated = True + elif new_table is not self.table: + # While we must proceed under the conservative + # possibility that the table was modified in place, if + # the tables are indeed different we can check for + # schema changes more confidently + old_dtype = self.table.iloc[:, old_index].dtype + if new_column.dtype == old_dtype: + # Type is the same and not object dtype + continue + elif old_index in filtered_columns: + # If it was an in place modification, as a last ditch + # effort we check if we remember the data type because + # of a prior filter + if filtered_columns[old_index].type_name == str(new_column.dtype): + # Type is the same and not object dtype + continue + + # The type maybe changed + schema_updated = True + + if old_index not in filtered_columns: + # This column index did not have a row filter + # attached to it, so doing further analysis is + # unnecessary + continue + + schema_changes[old_index] = _get_column_schema(new_column, str(column_name), new_index) + + new_filters = [] + for filt in self.filters: + column_index = filt.column_schema.column_index + column_name = filt.column_schema.column_name + + filt = filt.copy(deep=True) + + is_deleted = False + if column_index in schema_changes: + # A schema change is only valid if the column name is + # the same, otherwise it's a deletion + change = schema_changes[column_index] + if column_name == change.column_name: + filt.column_schema = change.copy() + else: + # Column may be deleted. We need to distinguish + # between the case of a deleted column that was + # filtered and a filter that was invalid and is + # now valid again as a result of changes in the + # data (e.g. deleting a column and then re-adding + # it right away) + if str(new_table.columns[column_index]) == column_name: + # Probably a new column that allows the old + # filter to become valid again if the type is + # compatible + filt.column_schema = _get_column_schema( + new_table.iloc[:, column_index], + column_name, + column_index, + ) + else: + is_deleted = True + elif column_index in shifted_columns: + filt.column_schema.column_index = shifted_columns[column_index] + elif column_index in deleted_columns: + # Column deleted + is_deleted = True + + # If the column was not deleted, we always reset the + # validity state of the filter in case something we + # did to the data made a previous filter error (which + # set the is_valid flag to False) to go away. + if is_deleted: + filt.is_valid = False + filt.error_message = "Column was deleted" + else: + filt.is_valid = self._is_supported_filter(filt) + if filt.is_valid: + filt.error_message = None + else: + filt.error_message = "Unsupported column type for filter" + + new_filters.append(filt) + + new_sort_keys = [] + for i, key in enumerate(self.sort_keys): + column_index = key.column_index + prior_name = self._sort_key_schemas[i].column_name + + key = key.copy() + + # Evict any sort key that was deleted + if column_index in schema_changes: + # A schema change is only valid if the column name is + # the same, otherwise it's a deletion + change = schema_changes[column_index] + if prior_name == change.column_name: + key.column_schema = change.copy() + else: + # Column deleted + continue + elif column_index in shifted_columns: + key.column_index = shifted_columns[column_index] + elif column_index in deleted_columns or prior_name != str( + new_table.columns[column_index] + ): + # Column deleted + continue + + new_sort_keys.append(key) - def ui_should_update_data(self, new_table): - # If the variables service says the variable has been updated - # or is uncertain - return True + return schema_updated, new_filters, new_sort_keys def _recompute(self): - # Resetting the column filters will trigger filtering AND + # Re-setting the column filters will trigger filtering AND # sorting self._set_row_filters(self.filters) - @property - def dtypes(self): - if self._dtypes is None: - self._dtypes = self.table.dtypes - return self._dtypes - def _get_schema(self, column_start: int, num_columns: int) -> TableSchema: column_schemas = [] @@ -406,27 +572,38 @@ def _get_inferred_dtype(self, column_index: int): self._inferred_dtypes[column_index] = infer_dtype(self.table.iloc[:, column_index]) return self._inferred_dtypes[column_index] - def _get_single_column_schema(self, column_index: int): - column_raw_name = self.table.columns[column_index] - column_name = str(column_raw_name) + @classmethod + def _get_type_display(cls, dtype, get_inferred_dtype): + # A helper function for returning the backend type_name and + # the display type when returning schema results or analyzing + # schema changes # TODO: pandas MultiIndex columns # TODO: time zone for datetimetz datetime64[ns] types - dtype = self.dtypes.iloc[column_index] - if dtype == object: - type_name = self._get_inferred_dtype(column_index) + type_name = get_inferred_dtype() else: # TODO: more sophisticated type mapping type_name = str(dtype) - type_display = self.TYPE_DISPLAY_MAPPING.get(type_name, "unknown") + type_display = cls.TYPE_DISPLAY_MAPPING.get(type_name, "unknown") + + return type_name, ColumnDisplayType(type_display) + + def _get_single_column_schema(self, column_index: int): + column_raw_name = self.table.columns[column_index] + column_name = str(column_raw_name) + + type_name, type_display = self._get_type_display( + self.table.iloc[:, column_index].dtype, + lambda: self._get_inferred_dtype(column_index), + ) return ColumnSchema( column_name=column_name, column_index=column_index, type_name=type_name, - type_display=ColumnDisplayType(type_display), + type_display=type_display, ) def _get_data_values( @@ -480,24 +657,57 @@ def _update_view_indices(self): # reflect the filtered_indices that have just been updated self._sort_data() + def _set_sort_columns(self, sort_keys: List[ColumnSortKey]): + self.sort_keys = sort_keys + + self._sort_key_schemas = [ + self._get_single_column_schema(key.column_index) for key in sort_keys + ] + + if not self._recompute_if_needed(): + # If a re-filter is pending, then it will automatically + # trigger a sort + self._sort_data() + def _set_row_filters(self, filters: List[RowFilter]) -> FilterResult: self.filters = filters + for filt in self.filters: + # If is_valid isn't set, set it based on what is currently + # supported + if filt.is_valid is None: + filt.is_valid = self._is_supported_filter(filt) + if len(filters) == 0: # Simply reset if empty filter set passed self.filtered_indices = None self._update_view_indices() - return FilterResult(selected_num_rows=len(self.table)) + return FilterResult(selected_num_rows=len(self.table), had_errors=False) # Evaluate all the filters and combine them using the # indicated conditions combined_mask = None + had_errors = False for filt in filters: if filt.is_valid is False: # If filter is invalid, do not evaluate it continue - single_mask = self._eval_filter(filt) + try: + single_mask = self._eval_filter(filt) + except Exception as e: + had_errors = True + + # Filter fails: we capture the error message and mark + # the filter as invalid + filt.is_valid = False + filt.error_message = str(e) + + # Perhaps use a different log level, but to help with + # debugging for now. + logger.warning(e, exc_info=True) + continue + if combined_mask is None: combined_mask = single_mask elif filt.condition == RowFilterCondition.And: @@ -514,10 +724,48 @@ def _set_row_filters(self, filters: List[RowFilter]) -> FilterResult: # Update the view indices, re-sorting if needed self._update_view_indices() - return FilterResult(selected_num_rows=selected_num_rows) + return FilterResult(selected_num_rows=selected_num_rows, had_errors=had_errors) + + def _is_supported_filter(self, filt: RowFilter) -> bool: + if filt.filter_type not in self.SUPPORTED_FILTERS: + return False + + display_type = filt.column_schema.type_display + + if filt.filter_type in [ + RowFilterType.IsEmpty, + RowFilterType.NotEmpty, + RowFilterType.Search, + ]: + # String-only filter types + return display_type == ColumnDisplayType.String + elif filt.filter_type == RowFilterType.Compare: + compare_op = filt.compare_params.op + if compare_op in [ + CompareFilterParamsOp.Eq, + CompareFilterParamsOp.NotEq, + ]: + return True + else: + return display_type in _FILTER_RANGE_COMPARE_SUPPORTED + elif filt.filter_type in [ + RowFilterType.Between, + RowFilterType.NotBetween, + ]: + return display_type in _FILTER_RANGE_COMPARE_SUPPORTED + else: + # Filters always supported + assert filt.filter_type in [ + RowFilterType.IsNull, + RowFilterType.NotNull, + RowFilterType.SetMembership, + ] + return True def _eval_filter(self, filt: RowFilter): - col = self.table.iloc[:, filt.column_index] + column_index = filt.column_schema.column_index + + col = self.table.iloc[:, column_index] mask = None if filt.filter_type in ( RowFilterType.Between, @@ -562,7 +810,7 @@ def _eval_filter(self, filt: RowFilter): params = filt.search_params assert params is not None - col_inferred_type = self._get_inferred_dtype(filt.column_index) + col_inferred_type = self._get_inferred_dtype(column_index) if col_inferred_type != "string": col = col.astype(str) @@ -701,32 +949,32 @@ def _prof_freq_table(self, column_index: int): def _prof_histogram(self, column_index: int): raise NotImplementedError - _row_filter_features = SetRowFiltersFeatures( - supported=True, - supports_conditions=False, - supported_types=[ - RowFilterType.Between, - RowFilterType.Compare, - RowFilterType.IsNull, - RowFilterType.NotNull, - RowFilterType.NotBetween, - RowFilterType.Search, - RowFilterType.SetMembership, - ], - ) - - _column_profile_features = GetColumnProfilesFeatures( - supported=True, - supported_types=[ - ColumnProfileType.NullCount, - ColumnProfileType.SummaryStats, - ], - ) + SUPPORTED_FILTERS = { + RowFilterType.Between, + RowFilterType.Compare, + RowFilterType.IsEmpty, + RowFilterType.NotEmpty, + RowFilterType.IsNull, + RowFilterType.NotNull, + RowFilterType.NotBetween, + RowFilterType.Search, + RowFilterType.SetMembership, + } FEATURES = SupportedFeatures( search_schema=SearchSchemaFeatures(supported=True), - set_row_filters=_row_filter_features, - get_column_profiles=_column_profile_features, + set_row_filters=SetRowFiltersFeatures( + supported=True, + supports_conditions=True, + supported_types=list(SUPPORTED_FILTERS), + ), + get_column_profiles=GetColumnProfilesFeatures( + supported=True, + supported_types=[ + ColumnProfileType.NullCount, + ColumnProfileType.SummaryStats, + ], + ), ) def _get_state(self) -> BackendState: @@ -850,7 +1098,7 @@ def register_table( data={"title": title}, ) - def close_callback(msg): + def close_callback(_): # Notify via callback that the comm_id has closed if self._close_callback: self._close_callback(comm_id) @@ -981,56 +1229,28 @@ def _update_explorer_for_comm(self, comm_id: str, path: PathKey, new_variable): # looking at invalid. return self._close_explorer(comm_id) - def _fire_data_update(): - comm.send_event(DataExplorerFrontendEvent.DataUpdate.value, {}) - - def _fire_schema_update(discard_state=False): - msg = SchemaUpdateParams(discard_state=discard_state) - comm.send_event(DataExplorerFrontendEvent.SchemaUpdate.value, msg.dict()) - - if type(new_table) is not type(table_view.table): # noqa: E721 - # Data type has changed. For now, we will signal the UI to - # reset its entire state: sorting keys, filters, etc. and - # start over. At some point we can return here and - # selectively preserve state if we feel it is safe enough - # to do so. - self.table_views[comm_id] = _get_table_view(new_table, name=full_title) - return _fire_schema_update(discard_state=True) - - # New value for data explorer is the same. For now, we just - # invalidate the stored computatations and fire a data update, - # but we'll come back here and improve this for immutable / - # copy-on-write tables like Arrow and Polars - # - # TODO: address pathological pandas case where columns have - # been modified - if new_table is table_view.table: - # The object references are the same, but we were probably - # unsure about whether the data has been mutated, so we - # invalidate the view's cached computations - # (e.g. filter/sort indices) so they get recomputed - table_view.invalidate_computations() - return _fire_data_update() - - ( - should_update_schema, - should_discard_state, - ) = table_view.ui_should_update_schema(new_table) - - if should_discard_state: - self.table_views[comm_id] = _get_table_view(new_table, name=full_title) + if not isinstance(new_table, type(table_view.table)): + # Data structure type has changed. For now, we drop the + # entire state: sorting keys, filters, etc. and start + # over. At some point we can return here and selectively + # preserve state if we can confidently do so. + schema_updated = True + new_filters = [] + new_sort_keys = [] else: - self.table_views[comm_id] = _get_table_view( - new_table, - filters=table_view.filters, - sort_keys=table_view.sort_keys, - name=full_title, - ) + (schema_updated, new_filters, new_sort_keys) = table_view.get_updated_state(new_table) - if should_update_schema: - _fire_schema_update(discard_state=should_discard_state) + self.table_views[comm_id] = _get_table_view( + new_table, + filters=new_filters, + sort_keys=new_sort_keys, + name=full_title, + ) + + if schema_updated: + comm.send_event(DataExplorerFrontendEvent.SchemaUpdate.value, {}) else: - _fire_data_update() + comm.send_event(DataExplorerFrontendEvent.DataUpdate.value, {}) def handle_msg(self, msg: CommMessage[DataExplorerBackendMessageContent], raw_msg): """ diff --git a/extensions/positron-python/python_files/positron/positron_ipykernel/data_explorer_comm.py b/extensions/positron-python/python_files/positron/positron_ipykernel/data_explorer_comm.py index 1cc8132ee97..e79c7039d39 100644 --- a/extensions/positron-python/python_files/positron/positron_ipykernel/data_explorer_comm.py +++ b/extensions/positron-python/python_files/positron/positron_ipykernel/data_explorer_comm.py @@ -166,6 +166,11 @@ class FilterResult(BaseModel): description="Number of rows in table after applying filters", ) + had_errors: Optional[bool] = Field( + default=None, + description="Flag indicating if there were errors in evaluation", + ) + class BackendState(BaseModel): """ @@ -282,8 +287,8 @@ class RowFilter(BaseModel): description="Type of row filter to apply", ) - column_index: int = Field( - description="Column index to apply filter to", + column_schema: ColumnSchema = Field( + description="Column to apply filter to", ) condition: RowFilterCondition = Field( @@ -295,6 +300,11 @@ class RowFilter(BaseModel): description="Whether the filter is valid and supported by the backend, if undefined then true", ) + error_message: Optional[str] = Field( + default=None, + description="Optional error message when the filter is invalid", + ) + between_params: Optional[BetweenFilterParams] = Field( default=None, description="Parameters for the 'between' and 'not_between' filter types", @@ -888,23 +898,13 @@ class DataExplorerFrontendEvent(str, enum.Enum): An enumeration of all the possible events that can be sent to the frontend data_explorer comm. """ - # Reset after a schema change + # Request to sync after a schema change SchemaUpdate = "schema_update" # Clear cache and request fresh data DataUpdate = "data_update" -class SchemaUpdateParams(BaseModel): - """ - Reset after a schema change - """ - - discard_state: bool = Field( - description="If true, the UI should discard the filter/sort state.", - ) - - SearchSchemaResult.update_forward_refs() TableData.update_forward_refs() @@ -984,5 +984,3 @@ class SchemaUpdateParams(BaseModel): GetColumnProfilesRequest.update_forward_refs() GetStateRequest.update_forward_refs() - -SchemaUpdateParams.update_forward_refs() diff --git a/extensions/positron-python/python_files/positron/positron_ipykernel/tests/conftest.py b/extensions/positron-python/python_files/positron/positron_ipykernel/tests/conftest.py index 8b0d1752287..79915427237 100644 --- a/extensions/positron-python/python_files/positron/positron_ipykernel/tests/conftest.py +++ b/extensions/positron-python/python_files/positron/positron_ipykernel/tests/conftest.py @@ -10,7 +10,6 @@ from traitlets.config import Config from positron_ipykernel.connections import ConnectionsService -from positron_ipykernel.data_explorer import DataExplorerService from positron_ipykernel.positron_ipkernel import ( PositronIPKernelApp, PositronIPyKernel, @@ -156,15 +155,17 @@ def variables_comm(variables_service: VariablesService) -> DummyComm: return variables_comm -@pytest.fixture() -def de_service(kernel: PositronIPyKernel) -> DataExplorerService: +@pytest.fixture +def de_service(kernel: PositronIPyKernel): """ The Positron dataviewer service. """ - return kernel.data_explorer_service + fixture = kernel.data_explorer_service + yield fixture + fixture.shutdown() -@pytest.fixture() +@pytest.fixture def connections_service(kernel: PositronIPyKernel) -> ConnectionsService: """ The Positron connections service. diff --git a/extensions/positron-python/python_files/positron/positron_ipykernel/tests/test_data_explorer.py b/extensions/positron-python/python_files/positron/positron_ipykernel/tests/test_data_explorer.py index 53deba30512..a0ac83878a5 100644 --- a/extensions/positron-python/python_files/positron/positron_ipykernel/tests/test_data_explorer.py +++ b/extensions/positron-python/python_files/positron/positron_ipykernel/tests/test_data_explorer.py @@ -197,89 +197,23 @@ def _check_delete_variable(name): _check_delete_variable("y") -def _check_update_variable(de_service, name, update_type="schema", discard_state=True): +def _check_update_variable(de_service, name, update_type="schema"): paths = de_service.get_paths_for_variable(name) assert len(paths) > 0 comms = [de_service.comms[comm_id] for p in paths for comm_id in de_service.path_to_comm_ids[p]] if update_type == "schema": - expected_msg = json_rpc_notification("schema_update", {"discard_state": discard_state}) + expected_msg = json_rpc_notification("schema_update", {}) else: expected_msg = json_rpc_notification("data_update", {}) # Check that comms were all closed for comm in comms: - last_message = cast(DummyComm, comm.comm).messages[-1] + dummy_comm = cast(DummyComm, comm.comm) + last_message = dummy_comm.messages[-1] assert last_message == expected_msg - - -def test_explorer_variable_updates( - shell: PositronShell, - de_service: DataExplorerService, - variables_comm: DummyComm, -): - x = pd.DataFrame({"a": [1, 0, 3, 4]}) - big_x = pd.DataFrame({"a": np.arange(BIG_ARRAY_LENGTH)}) - - _assign_variables( - shell, - variables_comm, - x=x, - big_x=big_x, - y={"key1": SIMPLE_PANDAS_DF, "key2": SIMPLE_PANDAS_DF}, - ) - - # Check updates - - path_x = _open_viewer(variables_comm, ["x"]) - _open_viewer(variables_comm, ["big_x"]) - _open_viewer(variables_comm, ["y", "key1"]) - _open_viewer(variables_comm, ["y", "key2"]) - _open_viewer(variables_comm, ["y", "key2"]) - - # Do a simple update and make sure that sort keys are preserved - x_comm_id = list(de_service.path_to_comm_ids[path_x])[0] - x_sort_keys = [{"column_index": 0, "ascending": True}] - msg = json_rpc_request( - "set_sort_columns", - params={"sort_keys": [{"column_index": 0, "ascending": True}]}, - comm_id=x_comm_id, - ) - de_service.comms[x_comm_id].comm.handle_msg(msg) - shell.run_cell("import pandas as pd") - shell.run_cell("x = pd.DataFrame({'a': [1, 0, 3, 4, 5]})") - _check_update_variable(de_service, "x", update_type="data") - - tv = de_service.table_views[x_comm_id] - assert tv.sort_keys == [ColumnSortKey(**k) for k in x_sort_keys] - assert tv._need_recompute - - dxf = DataExplorerFixture(de_service) - new_state = dxf.get_state("x") - assert new_state["display_name"] == "x" - assert new_state["table_shape"]["num_rows"] == 5 - assert new_state["table_shape"]["num_columns"] == 1 - assert new_state["sort_keys"] == [ColumnSortKey(**k) for k in x_sort_keys] - - # Execute code that triggers an update event for big_x because it's large - shell.run_cell("print('hello world')") - _check_update_variable(de_service, "big_x", update_type="data") - - # Update nested values in y and check for schema updates - shell.run_cell( - """y = {'key1': y['key1'].iloc[:1]], - 'key2': y['key2'].copy()} - """ - ) - _check_update_variable(de_service, "y", update_type="update", discard_state=False) - - shell.run_cell( - """y = {'key1': y['key1'].iloc[:-1, :-1], - 'key2': y['key2'].copy().iloc[:, 1:]} - """ - ) - _check_update_variable(de_service, "y", update_type="schema", discard_state=True) + dummy_comm.messages.clear() def test_register_table(de_service: DataExplorerService): @@ -311,10 +245,27 @@ def test_shutdown(de_service: DataExplorerService): class DataExplorerFixture: - def __init__(self, de_service: DataExplorerService): + def __init__( + self, + shell: PositronShell, + de_service: DataExplorerService, + variables_comm: DummyComm, + ): + self.shell = shell self.de_service = de_service - + self.variables_comm = variables_comm self.register_table("simple", SIMPLE_PANDAS_DF) + self._table_views = {} + + def assign_and_open_viewer(self, table_name: str, table): + _assign_variables(self.shell, self.variables_comm, **{table_name: table}) + path_df = _open_viewer(self.variables_comm, [table_name]) + comm_id = list(self.de_service.path_to_comm_ids[path_df])[0] + + return comm_id + + def execute_code(self, code: str): + self.shell.run_cell(code) def register_table(self, table_name: str, table): comm_id = guid() @@ -331,6 +282,12 @@ def register_table(self, table_name: str, table): variable_path=[encode_access_key(table_name)], ) + def get_schema_for(self, df): + comm_id = guid() + self.register_table(comm_id, df) + result = self.get_schema(comm_id) + return result + def do_json_rpc(self, table_name, method, **params): paths = self.de_service.get_paths_for_variable(table_name) assert len(paths) == 1 @@ -350,7 +307,14 @@ def do_json_rpc(self, table_name, method, **params): else: return data["result"] - def get_schema(self, table_name, start_index, num_columns): + def get_schema(self, table_name, start_index=None, num_columns=None): + if start_index is None: + start_index = 0 + + if num_columns is None: + shape = self.get_state(table_name)["table_shape"] + num_columns = shape["num_columns"] + return self.do_json_rpc( table_name, "get_schema", @@ -391,7 +355,7 @@ def check_filter_case(self, table, filter_set, expected_table): response = self.set_row_filters(table_id, filters=filter_set) ex_num_rows = len(expected_table) - assert response == FilterResult(selected_num_rows=ex_num_rows) + assert response == FilterResult(selected_num_rows=ex_num_rows, had_errors=False) assert self.get_state(table_id)["table_shape"]["num_rows"] == ex_num_rows self.compare_tables(table_id, ex_id, table.shape) @@ -428,8 +392,12 @@ def compare_tables(self, table_id: str, expected_id: str, table_shape: tuple): @pytest.fixture() -def dxf(de_service: DataExplorerService): - return DataExplorerFixture(de_service) +def dxf( + shell: PositronShell, + de_service: DataExplorerService, + variables_comm: DummyComm, +): + return DataExplorerFixture(shell, de_service, variables_comm) def _wrap_json(model: Type[BaseModel], data: JsonRecords): @@ -442,16 +410,25 @@ def test_pandas_get_state(dxf: DataExplorerFixture): assert result["table_shape"]["num_rows"] == 5 assert result["table_shape"]["num_columns"] == 6 + schema = dxf.get_schema("simple")["columns"] + sort_keys = [ {"column_index": 0, "ascending": True}, {"column_index": 1, "ascending": False}, ] - filters = [_compare_filter(0, ">", 0), _compare_filter(0, "<", 5)] + filters = [ + _compare_filter(schema[0], ">", 0), + _compare_filter(schema[0], "<", 5), + ] dxf.set_sort_columns("simple", sort_keys=sort_keys) dxf.set_row_filters("simple", filters=filters) result = dxf.get_state("simple") assert result["sort_keys"] == sort_keys + + # Validity is checked in set_row_filters + for f in filters: + f["is_valid"] = True assert result["row_filters"] == [RowFilter(**f) for f in filters] @@ -466,9 +443,11 @@ def test_pandas_supported_features(dxf: DataExplorerFixture): assert search_schema["supported"] assert row_filters["supported"] - assert not row_filters["supports_conditions"] + assert row_filters["supports_conditions"] assert set(row_filters["supported_types"]) == { + "is_empty", "is_null", + "not_empty", "not_null", "between", "compare", @@ -667,12 +646,12 @@ def test_pandas_get_data_values(dxf: DataExplorerFixture): # ) -def _filter(filter_type, column_index, condition="and", is_valid=None, **kwargs): +def _filter(filter_type, column_schema, condition="and", is_valid=None, **kwargs): kwargs.update( { "filter_id": guid(), "filter_type": filter_type, - "column_index": column_index, + "column_schema": column_schema, "condition": condition, "is_valid": is_valid, } @@ -680,28 +659,28 @@ def _filter(filter_type, column_index, condition="and", is_valid=None, **kwargs) return kwargs -def _compare_filter(column_index, op, value, condition="and", is_valid=None): +def _compare_filter(column_schema, op, value, condition="and", is_valid=None): return _filter( "compare", - column_index, + column_schema, condition=condition, is_valid=is_valid, compare_params={"op": op, "value": value}, ) -def _between_filter(column_index, left_value, right_value, op="between", condition="and"): +def _between_filter(column_schema, left_value, right_value, op="between", condition="and"): return _filter( op, - column_index, + column_schema, condition=condition, between_params={"left_value": left_value, "right_value": right_value}, ) -def _not_between_filter(column_index, left_value, right_value, condition="and"): +def _not_between_filter(column_schema, left_value, right_value, condition="and"): return _between_filter( - column_index, + column_schema, left_value, right_value, op="not_between", @@ -710,7 +689,7 @@ def _not_between_filter(column_index, left_value, right_value, condition="and"): def _search_filter( - column_index, + column_schema, term, case_sensitive=False, search_type="contains", @@ -718,7 +697,7 @@ def _search_filter( ): return _filter( "search", - column_index, + column_schema, condition=condition, search_params={ "search_type": search_type, @@ -729,14 +708,14 @@ def _search_filter( def _set_member_filter( - column_index, + column_schema, values, inclusive=True, condition="and", ): return _filter( "set_membership", - column_index, + column_schema, condition=condition, set_membership_params={"values": values, "inclusive": inclusive}, ) @@ -744,28 +723,27 @@ def _set_member_filter( def test_pandas_filter_between(dxf: DataExplorerFixture): df = SIMPLE_PANDAS_DF - column = "a" - column_index = df.columns.get_loc(column) + schema = dxf.get_schema("simple")["columns"] cases = [ - (0, 2, 4), # a column - (3, 0, 2), # d column + (schema[0], 2, 4), # a column + (schema[0], 0, 2), # d column ] - for column_index, left_value, right_value in cases: - col = df.iloc[:, column_index] + for column_schema, left_value, right_value in cases: + col = df.iloc[:, column_schema["column_index"]] ex_between = df[(col >= left_value) & (col <= right_value)] ex_not_between = df[(col < left_value) | (col > right_value)] dxf.check_filter_case( df, - [_between_filter(column_index, str(left_value), str(right_value))], + [_between_filter(column_schema, str(left_value), str(right_value))], ex_between, ) dxf.check_filter_case( df, - [_not_between_filter(column_index, str(left_value), str(right_value))], + [_not_between_filter(column_schema, str(left_value), str(right_value))], ex_not_between, ) @@ -773,11 +751,13 @@ def test_pandas_filter_between(dxf: DataExplorerFixture): def test_pandas_filter_conditions(dxf: DataExplorerFixture): # Test AND/OR conditions when filtering df = SIMPLE_PANDAS_DF + schema = dxf.get_schema("simple")["columns"] + filters = [ - _compare_filter(0, ">=", 3, condition="or"), - _compare_filter(3, "<=", -4.5, condition="or"), + _compare_filter(schema[0], ">=", 3, condition="or"), + _compare_filter(schema[3], "<=", -4.5, condition="or"), # Delbierately duplicated - _compare_filter(3, "<=", -4.5, condition="or"), + _compare_filter(schema[3], "<=", -4.5, condition="or"), ] expected_df = df[(df["a"] >= 3) | (df["d"] <= -4.5)] @@ -785,7 +765,7 @@ def test_pandas_filter_conditions(dxf: DataExplorerFixture): # Test a single condition with or set filters = [ - _compare_filter(0, ">=", 3, condition="or"), + _compare_filter(schema[0], ">=", 3, condition="or"), ] dxf.check_filter_case(df, filters, df[df["a"] >= 3]) @@ -797,20 +777,20 @@ def test_pandas_filter_compare(dxf: DataExplorerFixture): df = SIMPLE_PANDAS_DF compare_value = 3 column = "a" - column_index = df.columns.get_loc(column) + schema = dxf.get_schema("simple")["columns"] for op, op_func in COMPARE_OPS.items(): - filt = _compare_filter(column_index, op, str(compare_value)) + filt = _compare_filter(schema[0], op, str(compare_value)) expected_df = df[op_func(df[column], compare_value)] dxf.check_filter_case(df, [filt], expected_df) # TODO(wesm): move these tests to their own test case # Test that passing empty filter set resets to unfiltered state - filt = _compare_filter(column_index, "<", str(compare_value)) + filt = _compare_filter(schema[0], "<", str(compare_value)) _ = dxf.set_row_filters(table_name, filters=[filt]) response = dxf.set_row_filters(table_name, filters=[]) - assert response == FilterResult(selected_num_rows=len(df)) + assert response == FilterResult(selected_num_rows=len(df), had_errors=False) # register the whole table to make sure the filters are really cleared ex_id = guid() @@ -821,9 +801,11 @@ def test_pandas_filter_compare(dxf: DataExplorerFixture): def test_pandas_filter_is_valid(dxf: DataExplorerFixture): # Test AND/OR conditions when filtering df = SIMPLE_PANDAS_DF + schema = dxf.get_schema("simple")["columns"] + filters = [ - _compare_filter(0, ">=", 3), - _compare_filter(0, "<", 3, is_valid=False), + _compare_filter(schema[0], ">=", 3), + _compare_filter(schema[0], "<", 3, is_valid=False), ] expected_df = df[df["a"] >= 3] @@ -831,8 +813,8 @@ def test_pandas_filter_is_valid(dxf: DataExplorerFixture): # No filter is valid filters = [ - _compare_filter(0, ">=", 3, is_valid=False), - _compare_filter(0, "<", 3, is_valid=False), + _compare_filter(schema[0], ">=", 3, is_valid=False), + _compare_filter(schema[0], "<", 3, is_valid=False), ] dxf.check_filter_case(df, filters, df) @@ -846,17 +828,20 @@ def test_pandas_filter_empty(dxf: DataExplorerFixture): } ) - dxf.check_filter_case(df, [_filter("is_empty", 0)], df[df["a"].str.len() == 0]) - dxf.check_filter_case(df, [_filter("not_empty", 0)], df[df["a"].str.len() != 0]) - dxf.check_filter_case(df, [_filter("is_empty", 1)], df[df["b"].str.len() == 0]) - dxf.check_filter_case(df, [_filter("not_empty", 1)], df[df["b"].str.len() != 0]) + schema = dxf.get_schema_for(df)["columns"] + + dxf.check_filter_case(df, [_filter("is_empty", schema[0])], df[df["a"].str.len() == 0]) + dxf.check_filter_case(df, [_filter("not_empty", schema[0])], df[df["a"].str.len() != 0]) + dxf.check_filter_case(df, [_filter("is_empty", schema[1])], df[df["b"].str.len() == 0]) + dxf.check_filter_case(df, [_filter("not_empty", schema[1])], df[df["b"].str.len() != 0]) def test_pandas_filter_is_null_not_null(dxf: DataExplorerFixture): df = SIMPLE_PANDAS_DF - b_is_null = _filter("is_null", 1) - b_not_null = _filter("not_null", 1) - c_not_null = _filter("not_null", 2) + schema = dxf.get_schema_for(df)["columns"] + b_is_null = _filter("is_null", schema[1]) + b_not_null = _filter("not_null", schema[1]) + c_not_null = _filter("not_null", schema[2]) cases = [ [[b_is_null], df[df["b"].isnull()]], @@ -870,16 +855,20 @@ def test_pandas_filter_is_null_not_null(dxf: DataExplorerFixture): def test_pandas_filter_set_membership(dxf: DataExplorerFixture): df = SIMPLE_PANDAS_DF + schema = dxf.get_schema_for(df)["columns"] cases = [ - [[_set_member_filter(0, [2, 4])], df[df["a"].isin([2, 4])]], + [[_set_member_filter(schema[0], [2, 4])], df[df["a"].isin([2, 4])]], [ - [_set_member_filter(2, ["bar", "foo"])], + [_set_member_filter(schema[2], ["bar", "foo"])], df[df["c"].isin(["bar", "foo"])], ], - [[_set_member_filter(2, [])], df[df["c"].isin([])]], - [[_set_member_filter(2, ["bar"], False)], df[~df["c"].isin(["bar"])]], - [[_set_member_filter(2, [], False)], df], + [[_set_member_filter(schema[2], [])], df[df["c"].isin([])]], + [ + [_set_member_filter(schema[2], ["bar"], False)], + df[~df["c"].isin(["bar"])], + ], + [[_set_member_filter(schema[2], [], False)], df], ] for filter_set, expected_df in cases: @@ -895,63 +884,70 @@ def test_pandas_filter_search(dxf: DataExplorerFixture): ) dxf.register_table("df", df) + schema = dxf.get_schema("df")["columns"] - # (search_type, column_index, term, case_sensitive, boolean mask) + # (search_type, column_schema, term, case_sensitive, boolean mask) cases = [ - ("contains", 0, "foo", False, df["a"].str.lower().str.contains("foo")), - ("contains", 0, "foo", True, df["a"].str.contains("foo")), + ( + "contains", + schema[0], + "foo", + False, + df["a"].str.lower().str.contains("foo"), + ), + ("contains", schema[0], "foo", True, df["a"].str.contains("foo")), ( "starts_with", - 0, + schema[0], "foo", False, df["a"].str.lower().str.startswith("foo"), ), ( "starts_with", - 0, + schema[0], "foo", True, df["a"].str.startswith("foo"), ), ( "ends_with", - 0, + schema[0], "foo", False, df["a"].str.lower().str.endswith("foo"), ), ( "ends_with", - 0, + schema[0], "foo", True, df["a"].str.endswith("foo"), ), ( "regex_match", - 0, + schema[0], "f[o]+", False, df["a"].str.match("f[o]+", case=False), ), ( "regex_match", - 0, + schema[0], "f[o]+[^o]*", True, df["a"].str.match("f[o]+[^o]*", case=True), ), ] - for search_type, column_index, term, cs, mask in cases: + for search_type, column_schema, term, cs, mask in cases: mask[mask.isna()] = False ex_table = df[mask.astype(bool)] dxf.check_filter_case( df, [ _search_filter( - column_index, + column_schema, term, case_sensitive=cs, search_type=search_type, @@ -961,6 +957,238 @@ def test_pandas_filter_search(dxf: DataExplorerFixture): ) +def test_pandas_variable_updates( + shell: PositronShell, + de_service: DataExplorerService, + variables_comm: DummyComm, + dxf: DataExplorerFixture, +): + x = pd.DataFrame({"a": [1, 0, 3, 4]}) + big_array = np.arange(BIG_ARRAY_LENGTH) + big_x = pd.DataFrame({"a": big_array}) + + _assign_variables( + shell, + variables_comm, + x=x, + big_x=big_x, + y={"key1": SIMPLE_PANDAS_DF, "key2": SIMPLE_PANDAS_DF}, + ) + + # Check updates + + path_x = _open_viewer(variables_comm, ["x"]) + _open_viewer(variables_comm, ["big_x"]) + _open_viewer(variables_comm, ["y", "key1"]) + _open_viewer(variables_comm, ["y", "key2"]) + _open_viewer(variables_comm, ["y", "key2"]) + + # Do a simple update and make sure that sort keys are preserved + x_comm_id = list(de_service.path_to_comm_ids[path_x])[0] + x_sort_keys = [{"column_index": 0, "ascending": True}] + msg = json_rpc_request( + "set_sort_columns", + params={"sort_keys": x_sort_keys}, # type: ignore + comm_id=x_comm_id, + ) + de_service.comms[x_comm_id].comm.handle_msg(msg) + shell.run_cell("import pandas as pd") + shell.run_cell("x = pd.DataFrame({'a': [1, 0, 3, 4, 5]})") + _check_update_variable(de_service, "x", update_type="data") + + tv = de_service.table_views[x_comm_id] + assert tv.sort_keys == [ColumnSortKey(**k) for k in x_sort_keys] + assert tv._need_recompute + + new_state = dxf.get_state("x") + assert new_state["display_name"] == "x" + assert new_state["table_shape"]["num_rows"] == 5 + assert new_state["table_shape"]["num_columns"] == 1 + assert new_state["sort_keys"] == [ColumnSortKey(**k) for k in x_sort_keys] + + # Execute code that triggers an update event for big_x because it's large + shell.run_cell("None") + _check_update_variable(de_service, "big_x", update_type="schema") + + # Update nested values in y and check for schema updates + shell.run_cell( + """y = {'key1': y['key1'].iloc[:1]], + 'key2': y['key2'].copy()} + """ + ) + _check_update_variable(de_service, "y", update_type="schema") + + shell.run_cell( + """y = {'key1': y['key1'].iloc[:-1, :-1], + 'key2': y['key2'].copy().iloc[:, 1:]} + """ + ) + _check_update_variable(de_service, "y", update_type="schema") + + +def test_pandas_schema_change_state_updates(dxf: DataExplorerFixture): + df = pd.DataFrame( + { + "a": [1, 2, 3, 4, 5], + "b": ["foo", "bar", None, "baz", "qux"], + "c": [False, True, False, True, False], + } + ) + + dxf.assign_and_open_viewer("df", df.copy()) + schema = dxf.get_schema("df")["columns"] + + def _check_scenario(var, scenario, code: str): + filter_spec = scenario.get("filters", []) + + if "sort_keys" in scenario: + dxf.set_sort_columns(var, sort_keys=scenario["sort_keys"]) + + if len(filter_spec) > 0: + dxf.set_row_filters(var, filters=list(zip(*filter_spec))[0]) + + dxf.execute_code(code) + + # Get state and confirm that the right filters were made + # invalid + state = dxf.get_state(var) + updated_filters = state["row_filters"] + new_schema = dxf.get_schema(var)["columns"] + + if "sort_keys" in scenario: + assert state["sort_keys"] == scenario["updated_sort_keys"] + + for i, (spec, is_still_valid) in enumerate(filter_spec): + assert updated_filters[i]["is_valid"] == is_still_valid + + orig_col_schema = spec["column_schema"] + new_col_schema = None + for c in new_schema: + if c["column_name"] == orig_col_schema["column_name"]: + new_col_schema = c + break + + if new_col_schema is None: + # Column deleted, check that filter is invalid + assert not updated_filters[i]["is_valid"] + else: + # Check that schema was updated + assert updated_filters[i]["column_schema"] == new_col_schema + + # Scenario 1: convert "a" from integer to string + # (filter, is_valid_after_change) + dxf.assign_and_open_viewer("df1", df.copy()) + scenario1 = { + "filters": [ + # is null, not null, set membership remain valid + (_filter("is_null", schema[0]), True), + (_filter("not_null", schema[0]), True), + (_set_member_filter(schema[0], ["1", "2"]), True), + # range comparison becomes invalid + (_compare_filter(schema[0], "<", "4"), False), + (_compare_filter(schema[0], "<=", "4"), False), + (_compare_filter(schema[0], ">=", "4"), False), + (_compare_filter(schema[0], ">", "4"), False), + # equals, not-equals remain valid + (_compare_filter(schema[0], "=", "4"), True), + (_compare_filter(schema[0], "!=", "4"), True), + # between, not between becomes invalid + (_between_filter(schema[0], "1", "3"), False), + (_not_between_filter(schema[0], "1", "3"), False), + ] + } + + _check_scenario("df1", scenario1, "df1['a'] = df1['a'].astype(str)") + + # Scenario 2: convert "a" from int64 to int16 + dxf.assign_and_open_viewer("df2", df.copy()) + schema = dxf.get_schema("df2")["columns"] + scenario2 = { + "filters": [ + (_filter("is_null", schema[0]), True), + (_compare_filter(schema[0], "<", "4"), True), + (_between_filter(schema[0], "1", "3"), True), + ] + } + _check_scenario("df2", scenario2, "df2['a'] = df2['a'].astype('int16')") + + # Scenario 3: delete "a" in place + dxf.assign_and_open_viewer("df3", df.copy()) + schema = dxf.get_schema("df3")["columns"] + scenario3 = { + "filters": [ + (_filter("is_null", schema[0]), False), + (_compare_filter(schema[0], "<", "4"), False), + ], + "sort_keys": [{"column_index": 0, "ascending": True}], + "updated_sort_keys": [], + } + _check_scenario("df3", scenario3, "del df3['a']") + + # Scenario 4: delete "a" in a new DataFrame + dxf.assign_and_open_viewer("df4", df.copy()) + schema = dxf.get_schema("df4")["columns"] + scenario4 = { + "filters": [ + (_filter("is_null", schema[0]), False), + (_compare_filter(schema[0], "<", "4"), False), + ] + } + _check_scenario("df4", scenario4, "df4 = df4[['b']]") + + # Scenario 5: replace a column in place with a new name + dxf.assign_and_open_viewer("df5", df.copy()) + schema = dxf.get_schema("df5")["columns"] + scenario5 = { + "filters": [ + (_compare_filter(schema[1], "=", "foo"), False), + ] + } + _check_scenario("df5", scenario5, "df5.insert(1, 'c', df5.pop('b'))") + + # Scenario 6: add some columns, but do not disturb filters + dxf.assign_and_open_viewer("df6", df.copy()) + schema = dxf.get_schema("df6")["columns"] + scenario6 = { + "filters": [ + (_compare_filter(schema[0], "=", "1"), True), + (_compare_filter(schema[1], "=", "foo"), True), + ] + } + _check_scenario("df6", scenario6, "df6['c'] = df6['b']") + + # Scenario 7: delete column, then restore it and check that the + # filter was made invalid and then valid again + dxf.assign_and_open_viewer("df7", df.copy()) + schema = dxf.get_schema("df7")["columns"] + scenario7 = { + "filters": [ + (_compare_filter(schema[0], "<", "4"), False), + ] + } + # Scenario 7 -- Validate the setup, so the filter will be invalid + # after this + _check_scenario("df7", scenario7, "del df7['a']") + + # Scenario 7 -- Now restore df7 to its prior state + dxf.execute_code("df7 = df.copy()") + state = dxf.get_state("df7") + + # Filter is made valid again because the column reappeared where + # it was before and with a compatible type + filt = state["row_filters"][0] + assert filt["is_valid"] + assert filt["error_message"] is None + + # Scenario 8: Delete sorted column in middle of table + dxf.assign_and_open_viewer("df8", df.copy()) + scenario8 = { + "sort_keys": [{"column_index": 1, "ascending": False}], + "updated_sort_keys": [], + } + _check_scenario("df8", scenario8, "del df8['b']") + + def test_pandas_set_sort_columns(dxf: DataExplorerFixture): tables = { "df1": SIMPLE_PANDAS_DF, @@ -974,6 +1202,7 @@ def test_pandas_set_sort_columns(dxf: DataExplorerFixture): } ), } + df2_schema = dxf.get_schema_for(tables["df2"])["columns"] cases = [ ("df1", [(2, True)], {"by": "c"}), @@ -995,13 +1224,13 @@ def test_pandas_set_sort_columns(dxf: DataExplorerFixture): ] # Test sort AND filter - filter_cases = {"df2": [(lambda x: x[x["a"] > 0], [_compare_filter(0, ">", 0)])]} + filter_cases = {"df2": [(lambda x: x[x["a"] > 0], [_compare_filter(df2_schema[0], ">", 0)])]} - for df_name, keys, expected_params in cases: + for df_name, sort_keys, expected_params in cases: + df = tables[df_name] wrapped_keys = [ - {"column_index": index, "ascending": ascending} for index, ascending in keys + {"column_index": index, "ascending": ascending} for index, ascending in sort_keys ] - df = tables[df_name] expected_params["kind"] = "mergesort" @@ -1023,10 +1252,10 @@ def test_pandas_change_schema_after_sort( df = pd.DataFrame( { "a": np.arange(10), - "b": np.arange(10), - "c": np.arange(10), - "d": np.arange(10), - "e": np.arange(10), + "b": np.arange(10) + 1, + "c": np.arange(10) + 2, + "d": np.arange(10) + 3, + "e": np.arange(10) + 4, } ) _assign_variables(shell, variables_comm, df=df) @@ -1034,18 +1263,28 @@ def test_pandas_change_schema_after_sort( # Sort a column that is out of bounds for the table after the # schema change below - dxf.set_sort_columns("df", [{"column_index": 4, "ascending": True}]) + dxf.set_sort_columns( + "df", + [ + {"column_index": 4, "ascending": True}, + {"column_index": 0, "ascending": False}, + ], + ) - expected_df = df[["a", "b"]] - dxf.register_table("expected_df", df) + expected_df = df[["b", "a"]].sort_values("a", ascending=False) # type: ignore + dxf.register_table("expected_df", expected_df) # Sort last column, and we will then change the schema - shell.run_cell("df = df[['a', 'b']]") - _check_update_variable(de_service, "df", update_type="schema", discard_state=True) + shell.run_cell("df = df[['b', 'a']]") + _check_update_variable(de_service, "df", update_type="schema") # Call get_data_values and make sure it works dxf.compare_tables("df", "expected_df", expected_df.shape) + # Check that the out of bounds column index was evicted, and the + # shift was correct + dxf.get_state("df")["sort_keys"] = [{"column_index": 1, "ascending": False}] + def _profile_request(column_index, profile_type): return {"column_index": column_index, "profile_type": profile_type} @@ -1106,9 +1345,18 @@ def test_pandas_profile_null_counts(dxf: DataExplorerFixture): assert results == ex_results + df1_schema = dxf.get_schema_for(df1)["columns"] + # Test profiling with filter # format: (table, filters, filtered_table, profiles) - filter_cases = [(df1, [_filter("not_null", 0)], df1[df1["a"].notnull()], all_profiles)] + filter_cases = [ + ( + df1, + [_filter("not_null", df1_schema[0])], + df1[df1["a"].notnull()], + all_profiles, + ) + ] for table, filters, filtered_table, profiles in filter_cases: table_id = guid() dxf.register_table(table_id, table) @@ -1220,11 +1468,3 @@ def test_pandas_profile_summary_stats(dxf: DataExplorerFixture): _assert_string_stats_equal(ex_result, stats["string_stats"]) elif ui_type == ColumnDisplayType.Boolean: _assert_boolean_stats_equal(ex_result, stats["boolean_stats"]) - - -# ---------------------------------------------------------------------- -# Test RPCs for polars DataFrame - - -# ---------------------------------------------------------------------- -# Test RPCs for pyarrow Table diff --git a/positron/comms/data_explorer-backend-openrpc.json b/positron/comms/data_explorer-backend-openrpc.json index 8449a63ba5b..1b8a6c92a21 100644 --- a/positron/comms/data_explorer-backend-openrpc.json +++ b/positron/comms/data_explorer-backend-openrpc.json @@ -181,6 +181,10 @@ "selected_num_rows": { "type": "integer", "description": "Number of rows in table after applying filters" + }, + "had_errors": { + "type": "boolean", + "description": "Flag indicating if there were errors in evaluation" } } } @@ -392,7 +396,7 @@ "required": [ "filter_id", "filter_type", - "column_index", + "column_schema", "condition" ], "properties": { @@ -404,9 +408,9 @@ "description": "Type of row filter to apply", "$ref": "#/components/schemas/row_filter_type" }, - "column_index": { - "type": "integer", - "description": "Column index to apply filter to" + "column_schema": { + "description": "Column to apply filter to", + "$ref": "#/components/schemas/column_schema" }, "condition": { "type": "string", @@ -420,6 +424,10 @@ "type": "boolean", "description": "Whether the filter is valid and supported by the backend, if undefined then true" }, + "error_message": { + "type": "string", + "description": "Optional error message when the filter is invalid" + }, "between_params": { "description": "Parameters for the 'between' and 'not_between' filter types", "$ref": "#/components/schemas/between_filter_params" diff --git a/positron/comms/data_explorer-frontend-openrpc.json b/positron/comms/data_explorer-frontend-openrpc.json index a73a97241bf..a9cb165222f 100644 --- a/positron/comms/data_explorer-frontend-openrpc.json +++ b/positron/comms/data_explorer-frontend-openrpc.json @@ -7,17 +7,9 @@ "methods": [ { "name": "schema_update", - "summary": "Reset after a schema change", - "description": "Fully reset and redraw the data explorer after a schema change.", - "params": [ - { - "name": "discard_state", - "description": "If true, the UI should discard the filter/sort state.", - "schema": { - "type": "boolean" - } - } - ] + "summary": "Request to sync after a schema change", + "description": "Notify the data explorer to do a state sync after a schema change.", + "params": [] }, { "name": "data_update", diff --git a/positron/comms/generate-comms.ts b/positron/comms/generate-comms.ts index 28f2c5e698f..c6b5f214888 100644 --- a/positron/comms/generate-comms.ts +++ b/positron/comms/generate-comms.ts @@ -1334,7 +1334,7 @@ async function createCommInterface() { // Use black to format the Python file; the lint tests for the // Python extension require that the Python files have exactly the // format that black produces. - execSync(`python3 -m black ${pythonOutputFile}`, { stdio: 'ignore' }); + execSync(`python3 -m ruff format ${pythonOutputFile}`, { stdio: 'ignore' }); } } catch (e: any) { if (e.message) { diff --git a/src/vs/workbench/browser/positronDataExplorer/components/dataExplorerPanel/components/addEditRowFilterModalPopup/addEditRowFilterModalPopup.tsx b/src/vs/workbench/browser/positronDataExplorer/components/dataExplorerPanel/components/addEditRowFilterModalPopup/addEditRowFilterModalPopup.tsx index 8ee1c0a31a0..8a88cb76fbb 100644 --- a/src/vs/workbench/browser/positronDataExplorer/components/dataExplorerPanel/components/addEditRowFilterModalPopup/addEditRowFilterModalPopup.tsx +++ b/src/vs/workbench/browser/positronDataExplorer/components/dataExplorerPanel/components/addEditRowFilterModalPopup/addEditRowFilterModalPopup.tsx @@ -24,7 +24,7 @@ import { DropDownColumnSelector } from 'vs/workbench/browser/positronDataExplore import { RangeRowFilterDescriptor, RowFilterDescriptor, - RowFilterCondition, + RowFilterDescrType, RowFilterDescriptorComparison, RowFilterDescriptorIsBetween, RowFilterDescriptorIsEmpty, @@ -87,41 +87,41 @@ const validateRowFilterValue = (columnSchema: ColumnSchema, value: string) => { } }; -const conditionNumParams = (cond: RowFilterCondition | undefined) => { - switch (cond) { +const filterNumParams = (filterType: RowFilterDescrType | undefined) => { + switch (filterType) { case undefined: - case RowFilterCondition.CONDITION_IS_EMPTY: - case RowFilterCondition.CONDITION_IS_NOT_EMPTY: - case RowFilterCondition.CONDITION_IS_NULL: - case RowFilterCondition.CONDITION_IS_NOT_NULL: + case RowFilterDescrType.IS_EMPTY: + case RowFilterDescrType.IS_NOT_EMPTY: + case RowFilterDescrType.IS_NULL: + case RowFilterDescrType.IS_NOT_NULL: return 0; - case RowFilterCondition.CONDITION_IS_EQUAL_TO: - case RowFilterCondition.CONDITION_IS_NOT_EQUAL_TO: - case RowFilterCondition.CONDITION_IS_GREATER_OR_EQUAL: - case RowFilterCondition.CONDITION_IS_GREATER_THAN: - case RowFilterCondition.CONDITION_IS_LESS_OR_EQUAL: - case RowFilterCondition.CONDITION_IS_LESS_THAN: - case RowFilterCondition.CONDITION_SEARCH_CONTAINS: - case RowFilterCondition.CONDITION_SEARCH_STARTS_WITH: - case RowFilterCondition.CONDITION_SEARCH_ENDS_WITH: - case RowFilterCondition.CONDITION_SEARCH_REGEX_MATCHES: + case RowFilterDescrType.IS_EQUAL_TO: + case RowFilterDescrType.IS_NOT_EQUAL_TO: + case RowFilterDescrType.IS_GREATER_OR_EQUAL: + case RowFilterDescrType.IS_GREATER_THAN: + case RowFilterDescrType.IS_LESS_OR_EQUAL: + case RowFilterDescrType.IS_LESS_THAN: + case RowFilterDescrType.SEARCH_CONTAINS: + case RowFilterDescrType.SEARCH_STARTS_WITH: + case RowFilterDescrType.SEARCH_ENDS_WITH: + case RowFilterDescrType.SEARCH_REGEX_MATCHES: return 1; - case RowFilterCondition.CONDITION_IS_BETWEEN: - case RowFilterCondition.CONDITION_IS_NOT_BETWEEN: + case RowFilterDescrType.IS_BETWEEN: + case RowFilterDescrType.IS_NOT_BETWEEN: return 2; } }; /** - * Checks whether a RowFilterCondition is a comparison or not. - * @param cond A row filter condition. - * @returns Whether the condition is a comparison. + * Checks whether a RowFilterDescrType is a comparison or not. + * @param filterType A row filter descr type. + * @returns Whether the filter is a comparison. */ -const isSingleParam = (cond: RowFilterCondition | undefined) => { - if (cond === undefined) { +const isSingleParam = (filterType: RowFilterDescrType | undefined) => { + if (filterType === undefined) { return false; } - return conditionNumParams(cond) === 1; + return filterNumParams(filterType) === 1; }; /** @@ -147,10 +147,10 @@ export const AddEditRowFilterModalPopup = (props: AddEditRowFilterModalPopupProp // State hooks. const [selectedColumnSchema, setSelectedColumnSchema] = useState( - props.editRowFilter?.columnSchema + props.editRowFilter?.schema ); - const [selectedCondition, setSelectedCondition] = useState( - props.editRowFilter?.rowFilterCondition + const [selectedFilterType, setSelectedFilterType] = useState( + props.editRowFilter?.descrType ); const [firstRowFilterValue, setFirstRowFilterValue] = useState(() => { if (props.editRowFilter instanceof SingleValueRowFilterDescriptor) { @@ -168,19 +168,21 @@ export const AddEditRowFilterModalPopup = (props: AddEditRowFilterModalPopupProp return ''; } }); - const [errorText, setErrorText] = useState(undefined); + const [errorText, setErrorText] = useState( + props.editRowFilter?.props.errorMessage + ); - // useEffect for when the selectedCondition changes. + // useEffect for when the selectedFilterType changes. useEffect(() => { - // When there is a selected condition, drive focus into the first row filter parameter. - if (selectedCondition) { + // When there is a selected type, drive focus into the first row filter parameter. + if (selectedFilterType) { firstRowFilterParameterRef.current?.focus(); } - }, [selectedCondition]); + }, [selectedFilterType]); /** - * Returns the condition entries for the condition drop down list box. - * @returns The condition entries for the condition drop down list box. + * Returns the entries for the filter type drop down list box. + * @returns The entries for the filter type drop down list box. */ const conditionEntries = () => { // If there isn't a column schema, return an empty set of condition entries. @@ -189,77 +191,77 @@ export const AddEditRowFilterModalPopup = (props: AddEditRowFilterModalPopupProp } // Build the condition entries. - const conditionEntries: DropDownListBoxEntry[] = []; + const conditionEntries: DropDownListBoxEntry[] = []; // Every type allows is null and is not null conditions. conditionEntries.push(new DropDownListBoxItem({ - identifier: RowFilterCondition.CONDITION_IS_NULL, + identifier: RowFilterDescrType.IS_NULL, title: localize( 'positron.addEditRowFilter.conditionIsNull', "is null" ), - value: RowFilterCondition.CONDITION_IS_NULL + value: RowFilterDescrType.IS_NULL })); conditionEntries.push(new DropDownListBoxItem({ - identifier: RowFilterCondition.CONDITION_IS_NOT_NULL, + identifier: RowFilterDescrType.IS_NOT_NULL, title: localize( 'positron.addEditRowFilter.conditionIsNotNull', "is not null" ), - value: RowFilterCondition.CONDITION_IS_NOT_NULL + value: RowFilterDescrType.IS_NOT_NULL })); conditionEntries.push(new DropDownListBoxSeparator()); if (selectedColumnSchema.type_display === ColumnDisplayType.String) { conditionEntries.push(new DropDownListBoxItem({ - identifier: RowFilterCondition.CONDITION_SEARCH_CONTAINS, + identifier: RowFilterDescrType.SEARCH_CONTAINS, title: localize( 'positron.addEditRowFilter.conditionSearchContains', "contains" ), - value: RowFilterCondition.CONDITION_SEARCH_CONTAINS + value: RowFilterDescrType.SEARCH_CONTAINS })); conditionEntries.push(new DropDownListBoxItem({ - identifier: RowFilterCondition.CONDITION_SEARCH_STARTS_WITH, + identifier: RowFilterDescrType.SEARCH_STARTS_WITH, title: localize( 'positron.addEditRowFilter.conditionSearchStartsWith', "starts with" ), - value: RowFilterCondition.CONDITION_SEARCH_STARTS_WITH + value: RowFilterDescrType.SEARCH_STARTS_WITH })); conditionEntries.push(new DropDownListBoxItem({ - identifier: RowFilterCondition.CONDITION_SEARCH_ENDS_WITH, + identifier: RowFilterDescrType.SEARCH_ENDS_WITH, title: localize( 'positron.addEditRowFilter.conditionSearchEndsWith', "ends with" ), - value: RowFilterCondition.CONDITION_SEARCH_ENDS_WITH + value: RowFilterDescrType.SEARCH_ENDS_WITH })); conditionEntries.push(new DropDownListBoxItem({ - identifier: RowFilterCondition.CONDITION_SEARCH_REGEX_MATCHES, + identifier: RowFilterDescrType.SEARCH_REGEX_MATCHES, title: localize( 'positron.addEditRowFilter.conditionSearchRegexMatches', "regex matches" ), - value: RowFilterCondition.CONDITION_SEARCH_REGEX_MATCHES + value: RowFilterDescrType.SEARCH_REGEX_MATCHES })); // String types support is empty, is not empty filter types conditionEntries.push(new DropDownListBoxItem({ - identifier: RowFilterCondition.CONDITION_IS_EMPTY, + identifier: RowFilterDescrType.IS_EMPTY, title: localize( 'positron.addEditRowFilter.conditionIsEmpty', "is empty" ), - value: RowFilterCondition.CONDITION_IS_EMPTY + value: RowFilterDescrType.IS_EMPTY })); conditionEntries.push(new DropDownListBoxItem({ - identifier: RowFilterCondition.CONDITION_IS_NOT_EMPTY, + identifier: RowFilterDescrType.IS_NOT_EMPTY, title: localize( 'positron.addEditRowFilter.conditionIsNotEmpty', "is not empty" ), - value: RowFilterCondition.CONDITION_IS_NOT_EMPTY + value: RowFilterDescrType.IS_NOT_EMPTY })); conditionEntries.push(new DropDownListBoxSeparator()); } @@ -271,36 +273,36 @@ export const AddEditRowFilterModalPopup = (props: AddEditRowFilterModalPopupProp case ColumnDisplayType.Datetime: case ColumnDisplayType.Time: conditionEntries.push(new DropDownListBoxItem({ - identifier: RowFilterCondition.CONDITION_IS_LESS_THAN, + identifier: RowFilterDescrType.IS_LESS_THAN, title: localize( 'positron.addEditRowFilter.conditionIsLessThan', "is less than" ), - value: RowFilterCondition.CONDITION_IS_LESS_THAN + value: RowFilterDescrType.IS_LESS_THAN })); conditionEntries.push(new DropDownListBoxItem({ - identifier: RowFilterCondition.CONDITION_IS_LESS_OR_EQUAL, + identifier: RowFilterDescrType.IS_LESS_OR_EQUAL, title: localize( 'positron.addEditRowFilter.conditionIsLessThanOrEqual', "is less than or equal to" ), - value: RowFilterCondition.CONDITION_IS_LESS_OR_EQUAL + value: RowFilterDescrType.IS_LESS_OR_EQUAL })); conditionEntries.push(new DropDownListBoxItem({ - identifier: RowFilterCondition.CONDITION_IS_GREATER_THAN, + identifier: RowFilterDescrType.IS_GREATER_THAN, title: localize( 'positron.addEditRowFilter.conditionIsGreaterThan', "is greater than" ), - value: RowFilterCondition.CONDITION_IS_GREATER_THAN + value: RowFilterDescrType.IS_GREATER_THAN })); conditionEntries.push(new DropDownListBoxItem({ - identifier: RowFilterCondition.CONDITION_IS_GREATER_OR_EQUAL, + identifier: RowFilterDescrType.IS_GREATER_OR_EQUAL, title: localize( 'positron.addEditRowFilter.conditionIsGreaterThanOrEqual', "is greater than or equal to" ), - value: RowFilterCondition.CONDITION_IS_GREATER_OR_EQUAL + value: RowFilterDescrType.IS_GREATER_OR_EQUAL })); break; } @@ -314,20 +316,20 @@ export const AddEditRowFilterModalPopup = (props: AddEditRowFilterModalPopupProp case ColumnDisplayType.Datetime: case ColumnDisplayType.Time: conditionEntries.push(new DropDownListBoxItem({ - identifier: RowFilterCondition.CONDITION_IS_EQUAL_TO, + identifier: RowFilterDescrType.IS_EQUAL_TO, title: localize( 'positron.addEditRowFilter.conditionIsEqualTo', "is equal to" ), - value: RowFilterCondition.CONDITION_IS_EQUAL_TO + value: RowFilterDescrType.IS_EQUAL_TO })); conditionEntries.push(new DropDownListBoxItem({ - identifier: RowFilterCondition.CONDITION_IS_NOT_EQUAL_TO, + identifier: RowFilterDescrType.IS_NOT_EQUAL_TO, title: localize( 'positron.addEditRowFilter.conditionIsNotEqualTo', "is not equal to" ), - value: RowFilterCondition.CONDITION_IS_NOT_EQUAL_TO + value: RowFilterDescrType.IS_NOT_EQUAL_TO })); break; } @@ -340,20 +342,20 @@ export const AddEditRowFilterModalPopup = (props: AddEditRowFilterModalPopupProp case ColumnDisplayType.Time: conditionEntries.push(new DropDownListBoxSeparator()); conditionEntries.push(new DropDownListBoxItem({ - identifier: RowFilterCondition.CONDITION_IS_BETWEEN, + identifier: RowFilterDescrType.IS_BETWEEN, title: localize( 'positron.addEditRowFilter.conditionIsBetween', "is between" ), - value: RowFilterCondition.CONDITION_IS_BETWEEN + value: RowFilterDescrType.IS_BETWEEN })); conditionEntries.push(new DropDownListBoxItem({ - identifier: RowFilterCondition.CONDITION_IS_NOT_BETWEEN, + identifier: RowFilterDescrType.IS_NOT_BETWEEN, title: localize( 'positron.addEditRowFilter.conditionIsNotBetween', "is not between" ), - value: RowFilterCondition.CONDITION_IS_NOT_BETWEEN + value: RowFilterDescrType.IS_NOT_BETWEEN })); break; } @@ -362,7 +364,7 @@ export const AddEditRowFilterModalPopup = (props: AddEditRowFilterModalPopupProp return conditionEntries; }; - const numParams = conditionNumParams(selectedCondition); + const numParams = filterNumParams(selectedFilterType); // Set the first row filter parameter component. const firstRowFilterParameterComponent = (() => { @@ -458,7 +460,7 @@ export const AddEditRowFilterModalPopup = (props: AddEditRowFilterModalPopupProp } // Ensure that the user has selected a condition. - if (!selectedCondition) { + if (!selectedFilterType) { setErrorText(localize( 'positron.addEditRowFilter.pleaseSelectCondition', "Please select the condition." @@ -478,9 +480,9 @@ export const AddEditRowFilterModalPopup = (props: AddEditRowFilterModalPopupProp // Validate that the first row filter value is not empty. if (value.length === 0) { // Set the error text. - switch (selectedCondition) { - case RowFilterCondition.CONDITION_IS_BETWEEN: - case RowFilterCondition.CONDITION_IS_NOT_BETWEEN: + switch (selectedFilterType) { + case RowFilterDescrType.IS_BETWEEN: + case RowFilterDescrType.IS_NOT_BETWEEN: setErrorText(localize( 'positron.addEditRowFilter.pleaseSupplyLowerLimit', "Please supply the lower limit." @@ -503,9 +505,9 @@ export const AddEditRowFilterModalPopup = (props: AddEditRowFilterModalPopupProp // Validate the first row filter value. if (!validateRowFilterValue(selectedColumnSchema, value)) { // Set the error text. - switch (selectedCondition) { - case RowFilterCondition.CONDITION_IS_BETWEEN: - case RowFilterCondition.CONDITION_IS_NOT_BETWEEN: + switch (selectedFilterType) { + case RowFilterDescrType.IS_BETWEEN: + case RowFilterDescrType.IS_NOT_BETWEEN: setErrorText(localize( 'positron.addEditRowFilter.pleaseSupplyValidLowerLimit', "Please supply a valid lower limit." @@ -579,67 +581,69 @@ export const AddEditRowFilterModalPopup = (props: AddEditRowFilterModalPopupProp }; // Validate the condition and row filter values. If things are valid, add the row filter. - switch (selectedCondition) { + switch (selectedFilterType) { // Apply the is empty row filter. - case RowFilterCondition.CONDITION_IS_EMPTY: { - applyRowFilter(new RowFilterDescriptorIsEmpty(selectedColumnSchema)); + case RowFilterDescrType.IS_EMPTY: { + applyRowFilter(new RowFilterDescriptorIsEmpty({ columnSchema: selectedColumnSchema })); break; } // Apply the is not empty row filter. - case RowFilterCondition.CONDITION_IS_NOT_EMPTY: { - applyRowFilter(new RowFilterDescriptorIsNotEmpty(selectedColumnSchema)); + case RowFilterDescrType.IS_NOT_EMPTY: { + applyRowFilter(new RowFilterDescriptorIsNotEmpty({ columnSchema: selectedColumnSchema })); break; } // Apply the is null row filter. - case RowFilterCondition.CONDITION_IS_NULL: { - applyRowFilter(new RowFilterDescriptorIsNull(selectedColumnSchema)); + case RowFilterDescrType.IS_NULL: { + applyRowFilter(new RowFilterDescriptorIsNull( + { columnSchema: selectedColumnSchema })); break; } // Apply the is not null row filter. - case RowFilterCondition.CONDITION_IS_NOT_NULL: { - applyRowFilter(new RowFilterDescriptorIsNotNull(selectedColumnSchema)); + case RowFilterDescrType.IS_NOT_NULL: { + applyRowFilter(new RowFilterDescriptorIsNotNull( + { columnSchema: selectedColumnSchema })); break; } // Apply comparison row filter. - case RowFilterCondition.CONDITION_SEARCH_CONTAINS: - case RowFilterCondition.CONDITION_SEARCH_STARTS_WITH: - case RowFilterCondition.CONDITION_SEARCH_ENDS_WITH: - case RowFilterCondition.CONDITION_SEARCH_REGEX_MATCHES: { + case RowFilterDescrType.SEARCH_CONTAINS: + case RowFilterDescrType.SEARCH_STARTS_WITH: + case RowFilterDescrType.SEARCH_ENDS_WITH: + case RowFilterDescrType.SEARCH_REGEX_MATCHES: { if (!validateFirstRowFilterValue()) { return; } applyRowFilter(new RowFilterDescriptorSearch( - selectedColumnSchema, + { columnSchema: selectedColumnSchema }, firstRowFilterValue, - selectedCondition + selectedFilterType )); break; } // Apply comparison row filter. - case RowFilterCondition.CONDITION_IS_LESS_THAN: - case RowFilterCondition.CONDITION_IS_LESS_OR_EQUAL: - case RowFilterCondition.CONDITION_IS_GREATER_THAN: - case RowFilterCondition.CONDITION_IS_GREATER_OR_EQUAL: - case RowFilterCondition.CONDITION_IS_EQUAL_TO: - case RowFilterCondition.CONDITION_IS_NOT_EQUAL_TO: { + case RowFilterDescrType.IS_LESS_THAN: + case RowFilterDescrType.IS_LESS_OR_EQUAL: + case RowFilterDescrType.IS_GREATER_THAN: + case RowFilterDescrType.IS_GREATER_OR_EQUAL: + case RowFilterDescrType.IS_EQUAL_TO: + case RowFilterDescrType.IS_NOT_EQUAL_TO: { if (!validateFirstRowFilterValue()) { return; } applyRowFilter(new RowFilterDescriptorComparison( - selectedColumnSchema, + { columnSchema: selectedColumnSchema }, firstRowFilterValue, - selectedCondition + selectedFilterType )); break; } // Apply the is between row filter. - case RowFilterCondition.CONDITION_IS_BETWEEN: { + case RowFilterDescrType.IS_BETWEEN: { if (!validateFirstRowFilterValue()) { return; } @@ -647,7 +651,7 @@ export const AddEditRowFilterModalPopup = (props: AddEditRowFilterModalPopupProp return; } applyRowFilter(new RowFilterDescriptorIsBetween( - selectedColumnSchema, + { columnSchema: selectedColumnSchema }, firstRowFilterValue, secondRowFilterValue )); @@ -655,7 +659,7 @@ export const AddEditRowFilterModalPopup = (props: AddEditRowFilterModalPopupProp } // Apply the is not between row filter. - case RowFilterCondition.CONDITION_IS_NOT_BETWEEN: { + case RowFilterDescrType.IS_NOT_BETWEEN: { if (!validateFirstRowFilterValue()) { return; } @@ -663,7 +667,7 @@ export const AddEditRowFilterModalPopup = (props: AddEditRowFilterModalPopupProp return; } applyRowFilter(new RowFilterDescriptorIsNotBetween( - selectedColumnSchema, + { columnSchema: selectedColumnSchema }, firstRowFilterValue, secondRowFilterValue )); @@ -708,7 +712,7 @@ export const AddEditRowFilterModalPopup = (props: AddEditRowFilterModalPopupProp setSelectedColumnSchema(columnSchema); // Reset the selected condition. - setSelectedCondition(undefined); + setSelectedFilterType(undefined); // Clear the filter values and error text. clearFilterValuesAndErrorText(); @@ -723,12 +727,12 @@ export const AddEditRowFilterModalPopup = (props: AddEditRowFilterModalPopupProp "Select Condition" ))()} entries={conditionEntries()} - selectedIdentifier={selectedCondition} + selectedIdentifier={selectedFilterType} onSelectionChanged={dropDownListBoxItem => { - const prevSelected = selectedCondition; + const prevSelected = selectedFilterType; const nextSelected = dropDownListBoxItem.options.identifier; // Set the selected condition. - setSelectedCondition(nextSelected); + setSelectedFilterType(nextSelected); // Clear the filter values and error text. if (!(isSingleParam(prevSelected) && isSingleParam(nextSelected))) { diff --git a/src/vs/workbench/browser/positronDataExplorer/components/dataExplorerPanel/components/addEditRowFilterModalPopup/rowFilterDescriptor.ts b/src/vs/workbench/browser/positronDataExplorer/components/dataExplorerPanel/components/addEditRowFilterModalPopup/rowFilterDescriptor.ts index 7a12c317f3e..b40a5734eec 100644 --- a/src/vs/workbench/browser/positronDataExplorer/components/dataExplorerPanel/components/addEditRowFilterModalPopup/rowFilterDescriptor.ts +++ b/src/vs/workbench/browser/positronDataExplorer/components/dataExplorerPanel/components/addEditRowFilterModalPopup/rowFilterDescriptor.ts @@ -3,33 +3,46 @@ *--------------------------------------------------------------------------------------------*/ import { generateUuid } from 'vs/base/common/uuid'; -import { ColumnSchema, CompareFilterParamsOp, SearchFilterType } from 'vs/workbench/services/languageRuntime/common/positronDataExplorerComm'; +import { + ColumnSchema, + CompareFilterParamsOp, + RowFilter, + RowFilterCondition, + RowFilterType, + SearchFilterType +} from 'vs/workbench/services/languageRuntime/common/positronDataExplorerComm'; /** - * RowFilterCondition enumeration. + * RowFilterDescrType enumeration. */ -export enum RowFilterCondition { - // Conditions with no parameters. - CONDITION_IS_EMPTY = 'is-empty', - CONDITION_IS_NOT_EMPTY = 'is-not-empty', - CONDITION_IS_NULL = 'is-null', - CONDITION_IS_NOT_NULL = 'is-not-null', - - // Conditions with one parameter. - CONDITION_IS_LESS_THAN = 'is-less-than', - CONDITION_IS_LESS_OR_EQUAL = 'is-less-than-or-equal-to', - CONDITION_IS_GREATER_THAN = 'is-greater-than', - CONDITION_IS_GREATER_OR_EQUAL = 'is-greater-than-or-equal-to', - CONDITION_IS_EQUAL_TO = 'is-equal-to', - CONDITION_IS_NOT_EQUAL_TO = 'is-not-equal-to', - CONDITION_SEARCH_CONTAINS = 'search-contains', - CONDITION_SEARCH_STARTS_WITH = 'search-starts-with', - CONDITION_SEARCH_ENDS_WITH = 'search-ends-with', - CONDITION_SEARCH_REGEX_MATCHES = 'search-regex', - - // Conditions with two parameters. - CONDITION_IS_BETWEEN = 'is-between', - CONDITION_IS_NOT_BETWEEN = 'is-not-between' +export enum RowFilterDescrType { + // Filters with no parameters. + IS_EMPTY = 'is-empty', + IS_NOT_EMPTY = 'is-not-empty', + IS_NULL = 'is-null', + IS_NOT_NULL = 'is-not-null', + + // Filters with one parameter. + IS_LESS_THAN = 'is-less-than', + IS_LESS_OR_EQUAL = 'is-less-than-or-equal-to', + IS_GREATER_THAN = 'is-greater-than', + IS_GREATER_OR_EQUAL = 'is-greater-than-or-equal-to', + IS_EQUAL_TO = 'is-equal-to', + IS_NOT_EQUAL_TO = 'is-not-equal-to', + SEARCH_CONTAINS = 'search-contains', + SEARCH_STARTS_WITH = 'search-starts-with', + SEARCH_ENDS_WITH = 'search-ends-with', + SEARCH_REGEX_MATCHES = 'search-regex', + + // Filters with two parameters. + IS_BETWEEN = 'is-between', + IS_NOT_BETWEEN = 'is-not-between' +} + +interface RowFilterCommonProps { + readonly columnSchema: ColumnSchema; + readonly isValid?: boolean; + readonly errorMessage?: string; } /** @@ -43,16 +56,30 @@ abstract class BaseRowFilterDescriptor { /** * Constructor. - * @param columnSchema The column schema. + * @param props The common row filter descriptor properties. */ - constructor(public readonly columnSchema: ColumnSchema) { + constructor(public readonly props: RowFilterCommonProps) { this.identifier = generateUuid(); } /** - * Gets the row filter condition. + * Gets the row filter UI type. */ - abstract get rowFilterCondition(): RowFilterCondition; + abstract get descrType(): RowFilterDescrType; + + abstract get backendFilter(): RowFilter; + + get schema() { + return this.props.columnSchema; + } + + protected _sharedBackendParams() { + return { + filter_id: this.identifier, + column_schema: this.props.columnSchema, + condition: RowFilterCondition.And + }; + } } /** @@ -61,17 +88,27 @@ abstract class BaseRowFilterDescriptor { export class RowFilterDescriptorIsEmpty extends BaseRowFilterDescriptor { /** * Constructor. - * @param columnSchema The column schema. + * @param props The common row filter descriptor properties. */ - constructor(columnSchema: ColumnSchema) { - super(columnSchema); + constructor(props: RowFilterCommonProps) { + super(props); } /** * Gets the row filter condition. */ - get rowFilterCondition() { - return RowFilterCondition.CONDITION_IS_EMPTY; + get descrType() { + return RowFilterDescrType.IS_EMPTY; + } + + /** + * Get the backend OpenRPC type. + */ + get backendFilter() { + return { + filter_type: RowFilterType.IsEmpty, + ...this._sharedBackendParams() + }; } } @@ -81,17 +118,27 @@ export class RowFilterDescriptorIsEmpty extends BaseRowFilterDescriptor { export class RowFilterDescriptorIsNotEmpty extends BaseRowFilterDescriptor { /** * Constructor. - * @param columnSchema The column schema. + * @param props The common row filter descriptor properties. */ - constructor(columnSchema: ColumnSchema) { - super(columnSchema); + constructor(props: RowFilterCommonProps) { + super(props); } /** * Gets the row filter condition. */ - get rowFilterCondition() { - return RowFilterCondition.CONDITION_IS_NOT_EMPTY; + get descrType() { + return RowFilterDescrType.IS_NOT_EMPTY; + } + + /** + * Get the backend OpenRPC type. + */ + get backendFilter() { + return { + filter_type: RowFilterType.NotEmpty, + ...this._sharedBackendParams() + }; } } @@ -101,17 +148,27 @@ export class RowFilterDescriptorIsNotEmpty extends BaseRowFilterDescriptor { export class RowFilterDescriptorIsNull extends BaseRowFilterDescriptor { /** * Constructor. - * @param columnSchema The column schema. + * @param props The common row filter descriptor properties. */ - constructor(columnSchema: ColumnSchema) { - super(columnSchema); + constructor(props: RowFilterCommonProps) { + super(props); } /** * Gets the row filter condition. */ - get rowFilterCondition() { - return RowFilterCondition.CONDITION_IS_NULL; + get descrType() { + return RowFilterDescrType.IS_NULL; + } + + /** + * Get the backend OpenRPC type. + */ + get backendFilter() { + return { + filter_type: RowFilterType.IsNull, + ...this._sharedBackendParams() + }; } } @@ -121,17 +178,27 @@ export class RowFilterDescriptorIsNull extends BaseRowFilterDescriptor { export class RowFilterDescriptorIsNotNull extends BaseRowFilterDescriptor { /** * Constructor. - * @param columnSchema The column schema. + * @param props The common row filter descriptor properties. */ - constructor(columnSchema: ColumnSchema) { - super(columnSchema); + constructor(props: RowFilterCommonProps) { + super(props); } /** * Gets the row filter condition. */ - get rowFilterCondition() { - return RowFilterCondition.CONDITION_IS_NOT_NULL; + get descrType() { + return RowFilterDescrType.IS_NOT_NULL; + } + + /** + * Get the backend OpenRPC type. + */ + get backendFilter() { + return { + filter_type: RowFilterType.NotNull, + ...this._sharedBackendParams() + }; } } @@ -141,11 +208,12 @@ export class RowFilterDescriptorIsNotNull extends BaseRowFilterDescriptor { export abstract class SingleValueRowFilterDescriptor extends BaseRowFilterDescriptor { /** * Constructor. - * @param columnSchema The column schema. + * @param props The common row filter descriptor properties. * @param value The value. */ - constructor(columnSchema: ColumnSchema, public readonly value: string) { - super(columnSchema); + constructor(props: RowFilterCommonProps, + public readonly value: string) { + super(props); } } @@ -155,59 +223,72 @@ export abstract class SingleValueRowFilterDescriptor extends BaseRowFilterDescri export class RowFilterDescriptorComparison extends SingleValueRowFilterDescriptor { /** * Constructor. - * @param columnSchema The column schema. + * @param props The common row filter descriptor properties. * @param value The value. - * @param condition The filter condition. + * @param descrType The filter condition. */ - condition: RowFilterCondition; + _descrType: RowFilterDescrType; - constructor(columnSchema: ColumnSchema, value: string, condition: RowFilterCondition) { - super(columnSchema, value); - this.condition = condition; + constructor(props: RowFilterCommonProps, value: string, descrType: RowFilterDescrType) { + super(props, value); + this._descrType = descrType; } get operatorText() { - switch (this.condition) { - case RowFilterCondition.CONDITION_IS_EQUAL_TO: + switch (this.descrType) { + case RowFilterDescrType.IS_EQUAL_TO: return '='; - case RowFilterCondition.CONDITION_IS_GREATER_OR_EQUAL: + case RowFilterDescrType.IS_GREATER_OR_EQUAL: return '>='; - case RowFilterCondition.CONDITION_IS_GREATER_THAN: + case RowFilterDescrType.IS_GREATER_THAN: return '>'; - case RowFilterCondition.CONDITION_IS_LESS_OR_EQUAL: + case RowFilterDescrType.IS_LESS_OR_EQUAL: return '<='; - case RowFilterCondition.CONDITION_IS_LESS_THAN: + case RowFilterDescrType.IS_LESS_THAN: return '<'; - case RowFilterCondition.CONDITION_IS_NOT_EQUAL_TO: + case RowFilterDescrType.IS_NOT_EQUAL_TO: return '!='; default: return ''; } } - get compareFilterOp() { - switch (this.condition) { - case RowFilterCondition.CONDITION_IS_EQUAL_TO: - return CompareFilterParamsOp.Eq; - case RowFilterCondition.CONDITION_IS_GREATER_OR_EQUAL: - return CompareFilterParamsOp.GtEq; - case RowFilterCondition.CONDITION_IS_GREATER_THAN: - return CompareFilterParamsOp.Gt; - case RowFilterCondition.CONDITION_IS_LESS_OR_EQUAL: - return CompareFilterParamsOp.LtEq; - case RowFilterCondition.CONDITION_IS_LESS_THAN: - return CompareFilterParamsOp.Lt; - default: - // CONDITION_IS_NOT_EQUAL_TO - return CompareFilterParamsOp.NotEq; - } + /** + * Get the backend OpenRPC type. + */ + get backendFilter() { + const getCompareOp = () => { + switch (this.descrType) { + case RowFilterDescrType.IS_EQUAL_TO: + return CompareFilterParamsOp.Eq; + case RowFilterDescrType.IS_GREATER_OR_EQUAL: + return CompareFilterParamsOp.GtEq; + case RowFilterDescrType.IS_GREATER_THAN: + return CompareFilterParamsOp.Gt; + case RowFilterDescrType.IS_LESS_OR_EQUAL: + return CompareFilterParamsOp.LtEq; + case RowFilterDescrType.IS_LESS_THAN: + return CompareFilterParamsOp.Lt; + default: + // IS_NOT_EQUAL_TO + return CompareFilterParamsOp.NotEq; + } + }; + return { + filter_type: RowFilterType.Compare, + compare_params: { + op: getCompareOp(), + value: this.value + }, + ...this._sharedBackendParams() + }; } /** * Gets the row filter condition. */ - get rowFilterCondition() { - return this.condition; + get descrType() { + return this._descrType; } } @@ -217,50 +298,109 @@ export class RowFilterDescriptorComparison extends SingleValueRowFilterDescripto export class RowFilterDescriptorSearch extends SingleValueRowFilterDescriptor { /** * Constructor. - * @param columnSchema The column schema. + * @param props The common row filter descriptor properties. * @param value The value. - * @param condition The filter condition. + * @param descrType The filter condition. */ - condition: RowFilterCondition; + _descrType: RowFilterDescrType; - constructor(columnSchema: ColumnSchema, value: string, condition: RowFilterCondition) { - super(columnSchema, value); - this.condition = condition; + constructor(props: RowFilterCommonProps, value: string, descrType: RowFilterDescrType) { + super(props, value); + this._descrType = descrType; } get operatorText() { - switch (this.condition) { - case RowFilterCondition.CONDITION_SEARCH_CONTAINS: + switch (this._descrType) { + case RowFilterDescrType.SEARCH_CONTAINS: return 'contains'; - case RowFilterCondition.CONDITION_SEARCH_STARTS_WITH: + case RowFilterDescrType.SEARCH_STARTS_WITH: return 'starts with'; - case RowFilterCondition.CONDITION_SEARCH_ENDS_WITH: + case RowFilterDescrType.SEARCH_ENDS_WITH: return 'ends with'; default: - // CONDITION_SEARCH_REGEX_MATCHES + // SEARCH_REGEX_MATCHES return 'matches regex'; } } - get searchOp() { - switch (this.condition) { - case RowFilterCondition.CONDITION_SEARCH_CONTAINS: - return SearchFilterType.Contains; - case RowFilterCondition.CONDITION_SEARCH_STARTS_WITH: - return SearchFilterType.StartsWith; - case RowFilterCondition.CONDITION_SEARCH_ENDS_WITH: - return SearchFilterType.EndsWith; - default: - // CONDITION_SEARCH_REGEX_MATCHES - return SearchFilterType.RegexMatch; - } + + /** + * Gets the row filter condition. + */ + get descrType() { + return this._descrType; + } + + /** + * Get the backend OpenRPC type. + */ + get backendFilter() { + const getSearchOp = () => { + switch (this._descrType) { + case RowFilterDescrType.SEARCH_CONTAINS: + return SearchFilterType.Contains; + case RowFilterDescrType.SEARCH_STARTS_WITH: + return SearchFilterType.StartsWith; + case RowFilterDescrType.SEARCH_ENDS_WITH: + return SearchFilterType.EndsWith; + default: + // SEARCH_REGEX_MATCHES + return SearchFilterType.RegexMatch; + } + }; + + return { + filter_type: RowFilterType.Search, + search_params: { + search_type: getSearchOp(), + term: this.value, + case_sensitive: false + }, + ...this._sharedBackendParams() + }; + } +} + +/** + * RowFilterDescriptorSetMembership class. + */ +export class RowFilterDescriptorSetMembership extends BaseRowFilterDescriptor { + /** + * Constructor. + * @param props The common row filter descriptor properties. + * @param values The values to include. + */ + values: Array; + + constructor(props: RowFilterCommonProps, values: Array) { + super(props); + this.values = values; } /** * Gets the row filter condition. */ - get rowFilterCondition() { - return this.condition; + get descrType() { + // TODO: Add case and implement this + return RowFilterDescrType.IS_NULL; + } + + get operatorText() { + return 'includes'; + } + + /** + * Get the backend OpenRPC type. + */ + get backendFilter() { + return { + filter_type: RowFilterType.SetMembership, + set_membership_filter_params: { + values: this.values, + inclusive: true + }, + ...this._sharedBackendParams() + }; } } @@ -270,16 +410,16 @@ export class RowFilterDescriptorSearch extends SingleValueRowFilterDescriptor { export abstract class RangeRowFilterDescriptor extends BaseRowFilterDescriptor { /** * Constructor. - * @param columnSchema The column schema. + * @param props The common row filter descriptor properties. * @param lowerLimit The lower limit. * @param upperLimit The lower limit. */ constructor( - columnSchema: ColumnSchema, + props: RowFilterCommonProps, public readonly lowerLimit: string, - public readonly upperLimit: string + public readonly upperLimit: string, ) { - super(columnSchema); + super(props); } } @@ -289,19 +429,33 @@ export abstract class RangeRowFilterDescriptor extends BaseRowFilterDescriptor { export class RowFilterDescriptorIsBetween extends RangeRowFilterDescriptor { /** * Constructor. - * @param columnSchema The column schema. + * @param props The common row filter descriptor properties. * @param lowerLimit The lower limit. * @param upperLimit The lower limit. */ - constructor(columnSchema: ColumnSchema, lowerLimit: string, upperLimit: string) { - super(columnSchema, lowerLimit, upperLimit); + constructor(props: RowFilterCommonProps, lowerLimit: string, upperLimit: string) { + super(props, lowerLimit, upperLimit); } /** * Gets the row filter condition. */ - get rowFilterCondition() { - return RowFilterCondition.CONDITION_IS_BETWEEN; + get descrType() { + return RowFilterDescrType.IS_BETWEEN; + } + + /** + * Get the backend OpenRPC type. + */ + get backendFilter() { + return { + filter_type: RowFilterType.Between, + between_params: { + left_value: this.lowerLimit, + right_value: this.upperLimit + }, + ...this._sharedBackendParams() + }; } } @@ -311,19 +465,110 @@ export class RowFilterDescriptorIsBetween extends RangeRowFilterDescriptor { export class RowFilterDescriptorIsNotBetween extends RangeRowFilterDescriptor { /** * Constructor. - * @param columnSchema The column schema. + * @param props The common row filter descriptor properties. * @param lowerLimit The lower limit. * @param upperLimit The lower limit. */ - constructor(columnSchema: ColumnSchema, lowerLimit: string, upperLimit: string) { - super(columnSchema, lowerLimit, upperLimit); + constructor(props: RowFilterCommonProps, lowerLimit: string, upperLimit: string) { + super(props, lowerLimit, upperLimit); } /** * Gets the row filter condition. */ - get rowFilterCondition() { - return RowFilterCondition.CONDITION_IS_NOT_BETWEEN; + get descrType() { + return RowFilterDescrType.IS_NOT_BETWEEN; + } + + /** + * Get the backend OpenRPC type. + */ + get backendFilter() { + return { + filter_type: RowFilterType.NotBetween, + between_params: { + left_value: this.lowerLimit, + right_value: this.upperLimit + }, + ...this._sharedBackendParams() + }; + } +} + +function getCompareDescrType(op: CompareFilterParamsOp) { + switch (op) { + case CompareFilterParamsOp.Eq: + return RowFilterDescrType.IS_EQUAL_TO; + case CompareFilterParamsOp.NotEq: + return RowFilterDescrType.IS_NOT_EQUAL_TO; + case CompareFilterParamsOp.Lt: + return RowFilterDescrType.IS_LESS_THAN; + case CompareFilterParamsOp.LtEq: + return RowFilterDescrType.IS_LESS_OR_EQUAL; + case CompareFilterParamsOp.Gt: + return RowFilterDescrType.IS_GREATER_THAN; + case CompareFilterParamsOp.GtEq: + return RowFilterDescrType.IS_GREATER_OR_EQUAL; + } +} + +function getSearchDescrType(searchType: SearchFilterType) { + switch (searchType) { + case SearchFilterType.Contains: + return RowFilterDescrType.SEARCH_CONTAINS; + case SearchFilterType.EndsWith: + return RowFilterDescrType.SEARCH_ENDS_WITH; + case SearchFilterType.StartsWith: + return RowFilterDescrType.SEARCH_STARTS_WITH; + case SearchFilterType.RegexMatch: + return RowFilterDescrType.SEARCH_REGEX_MATCHES; + } +} + +export function getRowFilterDescriptor(backendFilter: RowFilter) { + const commonProps = { + columnSchema: backendFilter.column_schema, + isValid: backendFilter.is_valid, + errorMessage: backendFilter.error_message + }; + switch (backendFilter.filter_type) { + case RowFilterType.Compare: { + const params = backendFilter.compare_params!; + return new RowFilterDescriptorComparison(commonProps, + params.value, getCompareDescrType(params.op), + ); + } + case RowFilterType.Between: { + const params = backendFilter.between_params!; + return new RowFilterDescriptorIsBetween(commonProps, + params.left_value, params.right_value + ); + } + case RowFilterType.NotBetween: { + const params = backendFilter.between_params!; + return new RowFilterDescriptorIsNotBetween(commonProps, + params.left_value, params.right_value + ); + } + case RowFilterType.IsEmpty: + return new RowFilterDescriptorIsEmpty(commonProps); + case RowFilterType.NotEmpty: + return new RowFilterDescriptorIsNotEmpty(commonProps); + case RowFilterType.IsNull: + return new RowFilterDescriptorIsNull(commonProps); + case RowFilterType.NotNull: + return new RowFilterDescriptorIsNotNull(commonProps); + case RowFilterType.Search: { + const params = backendFilter.search_params!; + return new RowFilterDescriptorSearch(commonProps, + params.term, getSearchDescrType(params.search_type)); + } + case RowFilterType.SetMembership: { + const params = backendFilter.set_membership_params!; + return new RowFilterDescriptorSetMembership(commonProps, + params.values + ); + } } } diff --git a/src/vs/workbench/browser/positronDataExplorer/components/dataExplorerPanel/components/rowFilterBar/components/rowFilterWidget.css b/src/vs/workbench/browser/positronDataExplorer/components/dataExplorerPanel/components/rowFilterBar/components/rowFilterWidget.css index d2ed6f9b90e..8e90374ea59 100644 --- a/src/vs/workbench/browser/positronDataExplorer/components/dataExplorerPanel/components/rowFilterBar/components/rowFilterWidget.css +++ b/src/vs/workbench/browser/positronDataExplorer/components/dataExplorerPanel/components/rowFilterBar/components/rowFilterWidget.css @@ -16,12 +16,16 @@ background-color: var(--vscode-positronDataExplorer-background); } +.invalid-row-filter-widget { + color: var(--vscode-positronDataExplorer-background) !important; + background-color: var(--vscode-positronDataExplorer-invalidFilterBackground) !important; +} + .row-filter-widget:hover { background-color: var(--vscode-positronDataExplorer-contrastBackground); } -.row-filter-widget -.boolean-operator { +.row-filter-widget .boolean-operator { height: 100%; padding: 0 6px; display: flex; @@ -30,33 +34,25 @@ border-right: 1px solid var(--vscode-positronDataExplorer-border); } -.row-filter-widget -.title { +.row-filter-widget .title { margin: 0 2px 0 6px; } -.row-filter-widget -.title -.column-name { +.row-filter-widget .title .column-name { font-weight: 600; } -.row-filter-widget -.title -.space-before::before { +.row-filter-widget .title .space-before::before { content: " "; white-space: pre; } -.row-filter-widget -.title -.space-after::after { +.row-filter-widget .title .space-after::after { content: " "; white-space: pre; } -.row-filter-widget -.clear-filter-button { +.row-filter-widget .clear-filter-button { width: 18px; height: 18px; display: flex; @@ -69,20 +65,17 @@ justify-content: center; } -.row-filter-widget -.clear-filter-button:hover { +.row-filter-widget .clear-filter-button:hover { border: 1px solid var(--vscode-positronDataExplorer-border); /* outline: 1px solid var(--vscode-focusBorder); */ background-color: var(--vscode-positronDataExplorer-contrastBackground); } -.row-filter-widget -.clear-button:focus { +.row-filter-widget .clear-button:focus { outline: none !important; } -.row-filter-widget -.clear-button:focus-visible { +.row-filter-widget .clear-button:focus-visible { outline: 1px solid var(--vscode-focusBorder); } diff --git a/src/vs/workbench/browser/positronDataExplorer/components/dataExplorerPanel/components/rowFilterBar/components/rowFilterWidget.tsx b/src/vs/workbench/browser/positronDataExplorer/components/dataExplorerPanel/components/rowFilterBar/components/rowFilterWidget.tsx index d7e2e683189..7f0ac7a89f6 100644 --- a/src/vs/workbench/browser/positronDataExplorer/components/dataExplorerPanel/components/rowFilterBar/components/rowFilterWidget.tsx +++ b/src/vs/workbench/browser/positronDataExplorer/components/dataExplorerPanel/components/rowFilterBar/components/rowFilterWidget.tsx @@ -44,28 +44,28 @@ export const RowFilterWidget = forwardRef { if (props.rowFilter instanceof RowFilterDescriptorIsEmpty) { return <> - {props.rowFilter.columnSchema.column_name} + {props.rowFilter.schema.column_name} {localize('positron.dataExplorer.rowFilterWidget.isEmpty', "is empty")} ; } else if (props.rowFilter instanceof RowFilterDescriptorIsNotEmpty) { return <> - {props.rowFilter.columnSchema.column_name} + {props.rowFilter.schema.column_name} {localize('positron.dataExplorer.rowFilterWidget.isNotEmpty', "is not empty")} ; } else if (props.rowFilter instanceof RowFilterDescriptorIsNull) { return <> - {props.rowFilter.columnSchema.column_name} + {props.rowFilter.schema.column_name} {localize('positron.dataExplorer.rowFilterWidget.isNull', "is null")} ; } else if (props.rowFilter instanceof RowFilterDescriptorIsNotNull) { return <> - {props.rowFilter.columnSchema.column_name} + {props.rowFilter.schema.column_name} {localize('positron.dataExplorer.rowFilterWidget.isNotNull', "is not null")} @@ -73,37 +73,37 @@ export const RowFilterWidget = forwardRef - {props.rowFilter.columnSchema.column_name} + {props.rowFilter.schema.column_name} {props.rowFilter.operatorText} {props.rowFilter.value} ; } else if (props.rowFilter instanceof RowFilterDescriptorSearch) { return <> - {props.rowFilter.columnSchema.column_name} + {props.rowFilter.schema.column_name} {props.rowFilter.operatorText} {props.rowFilter.value} ; } else if (props.rowFilter instanceof RowFilterDescriptorIsBetween) { return <> - {props.rowFilter.columnSchema.column_name} + {props.rowFilter.schema.column_name} >= {props.rowFilter.lowerLimit} {localize('positron.dataExplorer.rowFilterWidget.and', "and")} - {props.rowFilter.columnSchema.column_name} + {props.rowFilter.schema.column_name} <= {props.rowFilter.upperLimit} ; } else if (props.rowFilter instanceof RowFilterDescriptorIsNotBetween) { return <> - {props.rowFilter.columnSchema.column_name} + {props.rowFilter.schema.column_name} < {props.rowFilter.lowerLimit} {localize('positron.dataExplorer.rowFilterWidget.and', "and")} - {props.rowFilter.columnSchema.column_name} + {props.rowFilter.schema.column_name} > {props.rowFilter.upperLimit} ; @@ -113,11 +113,16 @@ export const RowFilterWidget = forwardRef props.onEdit()} > {props.booleanOperator && diff --git a/src/vs/workbench/browser/positronDataExplorer/components/dataExplorerPanel/components/rowFilterBar/rowFilterBar.tsx b/src/vs/workbench/browser/positronDataExplorer/components/dataExplorerPanel/components/rowFilterBar/rowFilterBar.tsx index 92c98e22475..122b3de0201 100644 --- a/src/vs/workbench/browser/positronDataExplorer/components/dataExplorerPanel/components/rowFilterBar/rowFilterBar.tsx +++ b/src/vs/workbench/browser/positronDataExplorer/components/dataExplorerPanel/components/rowFilterBar/rowFilterBar.tsx @@ -7,9 +7,10 @@ import 'vs/css!./rowFilterBar'; // React. import * as React from 'react'; -import { useRef, useState } from 'react'; // eslint-disable-line no-duplicate-imports +import { useEffect, useRef, useState } from 'react'; // eslint-disable-line no-duplicate-imports // Other dependencies. +import { DisposableStore } from 'vs/base/common/lifecycle'; import { localize } from 'vs/nls'; import * as DOM from 'vs/base/browser/dom'; import { Button } from 'vs/base/browser/ui/positronComponents/button/button'; @@ -18,104 +19,13 @@ import { ContextMenuItem } from 'vs/workbench/browser/positronComponents/context import { ContextMenuSeparator } from 'vs/workbench/browser/positronComponents/contextMenu/contextMenuSeparator'; import { usePositronDataExplorerContext } from 'vs/workbench/browser/positronDataExplorer/positronDataExplorerContext'; import { PositronModalReactRenderer } from 'vs/workbench/browser/positronModalReactRenderer/positronModalReactRenderer'; -import { RowFilter, RowFilterCondition, RowFilterType } from 'vs/workbench/services/languageRuntime/common/positronDataExplorerComm'; import { RowFilterWidget } from 'vs/workbench/browser/positronDataExplorer/components/dataExplorerPanel/components/rowFilterBar/components/rowFilterWidget'; import { AddEditRowFilterModalPopup } from 'vs/workbench/browser/positronDataExplorer/components/dataExplorerPanel/components/addEditRowFilterModalPopup/addEditRowFilterModalPopup'; import { + getRowFilterDescriptor, RowFilterDescriptor, - RowFilterDescriptorComparison, - RowFilterDescriptorIsEmpty, - RowFilterDescriptorIsNotEmpty, - RowFilterDescriptorIsNull, - RowFilterDescriptorIsNotNull, - RowFilterDescriptorIsBetween, - RowFilterDescriptorIsNotBetween, - RowFilterDescriptorSearch } from 'vs/workbench/browser/positronDataExplorer/components/dataExplorerPanel/components/addEditRowFilterModalPopup/rowFilterDescriptor'; -/** - * Creates row filters from row filter descriptors. - * @param rowFilterDescriptors The row filter descriptors. - * @returns The row filters. - */ -const createRowFilters = (rowFilterDescriptors: RowFilterDescriptor[]) => { - // Create the set of row filters. - return rowFilterDescriptors.reduce(( - rowFilters, - rowFilterDescriptor - ) => { - // - const sharedParams = { - filter_id: rowFilterDescriptor.identifier, - column_index: rowFilterDescriptor.columnSchema.column_index, - condition: RowFilterCondition.And - }; - - if (rowFilterDescriptor instanceof RowFilterDescriptorIsEmpty) { - rowFilters.push({ - filter_type: RowFilterType.IsEmpty, - ...sharedParams - }); - } else if (rowFilterDescriptor instanceof RowFilterDescriptorIsNotEmpty) { - rowFilters.push({ - filter_type: RowFilterType.NotEmpty, - ...sharedParams - }); - } else if (rowFilterDescriptor instanceof RowFilterDescriptorIsNull) { - rowFilters.push({ - filter_type: RowFilterType.IsNull, - ...sharedParams - }); - } else if (rowFilterDescriptor instanceof RowFilterDescriptorIsNotNull) { - rowFilters.push({ - filter_type: RowFilterType.NotNull, - ...sharedParams - }); - } else if (rowFilterDescriptor instanceof RowFilterDescriptorComparison) { - rowFilters.push({ - filter_type: RowFilterType.Compare, - compare_params: { - op: rowFilterDescriptor.compareFilterOp, - value: rowFilterDescriptor.value - }, - ...sharedParams - }); - } else if (rowFilterDescriptor instanceof RowFilterDescriptorSearch) { - rowFilters.push({ - filter_type: RowFilterType.Search, - search_params: { - search_type: rowFilterDescriptor.searchOp, - term: rowFilterDescriptor.value, - case_sensitive: false - }, - ...sharedParams - }); - } else if (rowFilterDescriptor instanceof RowFilterDescriptorIsBetween) { - rowFilters.push({ - filter_type: RowFilterType.Between, - between_params: { - left_value: rowFilterDescriptor.lowerLimit, - right_value: rowFilterDescriptor.upperLimit - }, - ...sharedParams - }); - } else if (rowFilterDescriptor instanceof RowFilterDescriptorIsNotBetween) { - rowFilters.push({ - filter_type: RowFilterType.NotBetween, - between_params: { - left_value: rowFilterDescriptor.lowerLimit, - right_value: rowFilterDescriptor.upperLimit - }, - ...sharedParams - }); - } - - // Return the row filters. - return rowFilters; - }, []); -}; - - /** * RowFilterBar component. * @returns The rendered component. @@ -123,6 +33,7 @@ const createRowFilters = (rowFilterDescriptors: RowFilterDescriptor[]) => { export const RowFilterBar = () => { // Context hooks. const context = usePositronDataExplorerContext(); + const backendClient = context.instance.dataExplorerClientInstance; // Reference hooks. const ref = useRef(undefined!); @@ -131,9 +42,29 @@ export const RowFilterBar = () => { const addFilterButtonRef = useRef(undefined!); // State hooks. - const [rowFilterDescriptors, setRowFilterDescriptors] = useState([]); + const [rowFilterDescriptors, setRowFilterDescriptors] = useState( + backendClient.cachedBackendState === undefined ? [] : + backendClient.cachedBackendState.row_filters.map(getRowFilterDescriptor) + ); const [rowFiltersHidden, setRowFiltersHidden] = useState(false); + // Main useEffect. This is where we set up event handlers. + useEffect(() => { + // Create the disposable store for cleanup. + const disposableStore = new DisposableStore(); + + // Set up event handler for backend state sync updating the filter bar + disposableStore.add(context.instance.dataExplorerClientInstance.onDidUpdateBackendState( + (state) => { + const newDescriptors = state.row_filters.map(getRowFilterDescriptor); + setRowFilterDescriptors(newDescriptors); + }) + ); + + // Return the cleanup function that will dispose of the event handlers. + return () => disposableStore.dispose(); + }, [context.instance]); + /** * Shows the add / edit row filter modal popup. * @param editRowFilterDescriptor The row filter to edit, or undefined, to add a row filter. @@ -178,9 +109,9 @@ export const RowFilterBar = () => { setRowFilterDescriptors(newRowFilterDescriptors); // Set the new row filters. - await context.instance.tableDataDataGridInstance.setRowFilters(createRowFilters( - newRowFilterDescriptors - )); + await context.instance.tableDataDataGridInstance.setRowFilters( + newRowFilterDescriptors.map(descr => descr.backendFilter) + ); }; // Show the add /edit row filter modal popup. @@ -260,9 +191,9 @@ export const RowFilterBar = () => { setRowFilterDescriptors(newRowFilterDescriptors); // Set the new row filters. - await context.instance.tableDataDataGridInstance.setRowFilters(createRowFilters( - newRowFilterDescriptors - )); + await context.instance.tableDataDataGridInstance.setRowFilters( + newRowFilterDescriptors.map(descr => descr.backendFilter) + ); }; // Render. diff --git a/src/vs/workbench/browser/positronDataGrid/classes/dataGridInstance.ts b/src/vs/workbench/browser/positronDataGrid/classes/dataGridInstance.ts index e101f06a3cb..9bb8d38032c 100644 --- a/src/vs/workbench/browser/positronDataGrid/classes/dataGridInstance.ts +++ b/src/vs/workbench/browser/positronDataGrid/classes/dataGridInstance.ts @@ -218,9 +218,11 @@ interface CellSelectionRange { } /** - * ColumnSortKey class. + * ColumnSortKeyDescriptor class. + * + * "Descriptor" added to disambiguate from ColumnSortKey in generated comm. */ -class ColumnSortKey implements IColumnSortKey { +export class ColumnSortKeyDescriptor implements IColumnSortKey { //#region Private Properties /** @@ -476,12 +478,16 @@ export abstract class DataGridInstance extends Disposable { */ private readonly _rowHeights = new Map(); + //#endregion Private Properties + + //#region Protected Properties + /** * Gets the column sort keys. */ - private readonly _columnSortKeys = new Map(); + protected readonly _columnSortKeys = new Map(); - //#endregion Private Properties + //#endregion Protected Properties //#region Protected Events @@ -994,7 +1000,7 @@ export abstract class DataGridInstance extends Disposable { // Add the column sort key. this._columnSortKeys.set( columnIndex, - new ColumnSortKey(this._columnSortKeys.size, columnIndex, ascending) + new ColumnSortKeyDescriptor(this._columnSortKeys.size, columnIndex, ascending) ); // Fire the onDidUpdate event. diff --git a/src/vs/workbench/common/theme.ts b/src/vs/workbench/common/theme.ts index 96af49ecd4d..b3b282a1b3d 100644 --- a/src/vs/workbench/common/theme.ts +++ b/src/vs/workbench/common/theme.ts @@ -2094,6 +2094,14 @@ export const POSITRON_DATA_EXPLORER_CONTRAST_BACKGROUND_COLOR = registerColor('p hcLight: editorBackground }, localize('positronDataExplorer.contrastBackground', "Positron data explorer contrast background color.")); +// Positron data explorer invalid filter background color. +export const POSITRON_DATA_GRID_INVALID_FILTER_BACKGROUND_COLOR = registerColor('positronDataExplorer.invalidFilterBackground', { + dark: '#e95b5b', + light: '#e95b5b', + hcDark: '#e95b5b', + hcLight: '#e95b5b' +}, localize('positronDataExplorer.invalidFilterBackground', "Positron data grid invalid background color.")); + // Positron data explorer selection background color. export const POSITRON_DATA_EXPLORER_SELECTION_BACKGROUND_COLOR = registerColor('positronDataExplorer.selectionBackground', { dark: lighten(editorBackground, 0.2), diff --git a/src/vs/workbench/services/languageRuntime/common/languageRuntimeDataExplorerClient.ts b/src/vs/workbench/services/languageRuntime/common/languageRuntimeDataExplorerClient.ts index 2f7b25ec6f0..e06c1633269 100644 --- a/src/vs/workbench/services/languageRuntime/common/languageRuntimeDataExplorerClient.ts +++ b/src/vs/workbench/services/languageRuntime/common/languageRuntimeDataExplorerClient.ts @@ -47,6 +47,16 @@ export class DataExplorerClientInstance extends Disposable { */ private readonly _identifier = generateUuid(); + /** + * The current cached backend state. + */ + public cachedBackendState: BackendState | undefined = undefined; + + /** + * A promise resolving to an active request for the backend state. + */ + private _backendPromise: Promise | undefined = undefined; + /** * Gets the PositronDataExplorerComm. */ @@ -57,6 +67,13 @@ export class DataExplorerClientInstance extends Disposable { */ private readonly _onDidSchemaUpdateEmitter = this._register(new Emitter()); + /** + * The onDidUpdateBackendState event emitter. + */ + private readonly _onDidUpdateBackendStateEmitter = this._register( + new Emitter + ); + /** * The onDidDataUpdate event emitter. */ @@ -71,7 +88,7 @@ export class DataExplorerClientInstance extends Disposable { * @param client The runtime client instance. */ constructor(client: IRuntimeClientInstance) { - // Call the disposable constrcutor. + // Call the disposable constructor. super(); // Create and register the PositronDataExplorerComm on the client. @@ -82,6 +99,7 @@ export class DataExplorerClientInstance extends Disposable { this.onDidClose = this._positronDataExplorerComm.onDidClose; this._positronDataExplorerComm.onDidSchemaUpdate(async (e: SchemaUpdateEvent) => { + this.updateBackendState(); this._onDidSchemaUpdateEmitter.fire(e); }); @@ -107,10 +125,39 @@ export class DataExplorerClientInstance extends Disposable { /** * Get the current active state of the data explorer backend. - * @returns A promose that resolves to the current table state. + * @returns A promose that resolves to the current backend state. */ - async getState(): Promise { - return this._positronDataExplorerComm.getState(); + async getBackendState(): Promise { + if (this._backendPromise) { + // If there is a request for the state pending + return this._backendPromise; + } else if (this.cachedBackendState === undefined) { + // The state is being requested for the first time + return this.updateBackendState(); + } else { + // The state was previously computed + return this.cachedBackendState; + } + } + + /** + * Requests a fresh update of the backend state and fires event to notify state listeners. + * @returns A promise that resolves to the latest table state. + */ + async updateBackendState(): Promise { + if (this._backendPromise) { + return this._backendPromise; + } + + this._backendPromise = this._positronDataExplorerComm.getState(); + this.cachedBackendState = await this._backendPromise; + this._backendPromise = undefined; + + // Notify listeners + this._onDidUpdateBackendStateEmitter.fire(this.cachedBackendState); + + // Fulfill to anyone waiting on the backend state. + return this.cachedBackendState; } /** @@ -140,7 +187,7 @@ export class DataExplorerClientInstance extends Disposable { */ // Get the table state so we know now many columns there are. - const tableState = await this._positronDataExplorerComm.getState(); + const tableState = await this.getBackendState(); // Load the entire schema of the table so it can be searched. const tableSchema = await this._positronDataExplorerComm.getSchema( @@ -220,6 +267,11 @@ export class DataExplorerClientInstance extends Disposable { */ onDidSchemaUpdate = this._onDidSchemaUpdateEmitter.event; + /** + * Event that fires when the backend state has been updated. + */ + onDidUpdateBackendState = this._onDidUpdateBackendStateEmitter.event; + /** * Event that fires when the data has been updated. */ diff --git a/src/vs/workbench/services/languageRuntime/common/positronDataExplorerComm.ts b/src/vs/workbench/services/languageRuntime/common/positronDataExplorerComm.ts index 545b3da086c..b73b7647788 100644 --- a/src/vs/workbench/services/languageRuntime/common/positronDataExplorerComm.ts +++ b/src/vs/workbench/services/languageRuntime/common/positronDataExplorerComm.ts @@ -51,6 +51,11 @@ export interface FilterResult { */ selected_num_rows: number; + /** + * Flag indicating if there were errors in evaluation + */ + had_errors?: boolean; + } /** @@ -182,9 +187,9 @@ export interface RowFilter { filter_type: RowFilterType; /** - * Column index to apply filter to + * Column to apply filter to */ - column_index: number; + column_schema: ColumnSchema; /** * The binary condition to use to combine with preceding row filters @@ -197,6 +202,11 @@ export interface RowFilter { */ is_valid?: boolean; + /** + * Optional error message when the filter is invalid + */ + error_message?: string; + /** * Parameters for the 'between' and 'not_between' filter types */ @@ -646,14 +656,9 @@ export enum ColumnProfileType { } /** - * Event: Reset after a schema change + * Event: Request to sync after a schema change */ export interface SchemaUpdateEvent { - /** - * If true, the UI should discard the filter/sort state. - */ - discard_state: boolean; - } /** @@ -670,7 +675,7 @@ export enum DataExplorerFrontendEvent { export class PositronDataExplorerComm extends PositronBaseComm { constructor(instance: IRuntimeClientInstance) { super(instance); - this.onDidSchemaUpdate = super.createEventEmitter('schema_update', ['discard_state']); + this.onDidSchemaUpdate = super.createEventEmitter('schema_update', []); this.onDidDataUpdate = super.createEventEmitter('data_update', []); } @@ -776,9 +781,9 @@ export class PositronDataExplorerComm extends PositronBaseComm { /** - * Reset after a schema change + * Request to sync after a schema change * - * Fully reset and redraw the data explorer after a schema change. + * Notify the data explorer to do a state sync after a schema change. */ onDidSchemaUpdate: Event; /** diff --git a/src/vs/workbench/services/positronDataExplorer/browser/positronDataExplorerService.ts b/src/vs/workbench/services/positronDataExplorer/browser/positronDataExplorerService.ts index 4c0be3f3a80..81d41703726 100644 --- a/src/vs/workbench/services/positronDataExplorer/browser/positronDataExplorerService.ts +++ b/src/vs/workbench/services/positronDataExplorer/browser/positronDataExplorerService.ts @@ -196,17 +196,19 @@ class PositronDataExplorerService extends Disposable implements IPositronDataExp resource: PositronDataExplorerUri.generate(dataExplorerClientInstance.identifier) }); - dataExplorerClientInstance.getState().then((state) => { + dataExplorerClientInstance.onDidUpdateBackendState((state) => { // Hack to be able to call PositronDataExplorerEditorInput.setName without // eslint errors; const dxInput = editor?.input as any; - if (state.display_name !== undefined) { + if (dxInput !== undefined && state.display_name !== undefined) { dxInput.setName?.(`Data: ${state.display_name}`); } }); - const end = new Date(); + // Trigger the initial state update. + dataExplorerClientInstance.updateBackendState(); + const end = new Date(); console.log(`this._editorService.openEditor took ${end.getTime() - start.getTime()}ms`); } diff --git a/src/vs/workbench/services/positronDataExplorer/browser/tableDataDataGridInstance.tsx b/src/vs/workbench/services/positronDataExplorer/browser/tableDataDataGridInstance.tsx index d175f8d4020..887ea11b6bf 100644 --- a/src/vs/workbench/services/positronDataExplorer/browser/tableDataDataGridInstance.tsx +++ b/src/vs/workbench/services/positronDataExplorer/browser/tableDataDataGridInstance.tsx @@ -7,13 +7,13 @@ import * as React from 'react'; // Other dependencies. import { IColumnSortKey } from 'vs/workbench/browser/positronDataGrid/interfaces/columnSortKey'; -import { DataGridInstance } from 'vs/workbench/browser/positronDataGrid/classes/dataGridInstance'; +import { ColumnSortKeyDescriptor, DataGridInstance } from 'vs/workbench/browser/positronDataGrid/classes/dataGridInstance'; import { DataExplorerCache } from 'vs/workbench/services/positronDataExplorer/common/dataExplorerCache'; import { TableDataCell } from 'vs/workbench/services/positronDataExplorer/browser/components/tableDataCell'; import { TableDataRowHeader } from 'vs/workbench/services/positronDataExplorer/browser/components/tableDataRowHeader'; import { PositronDataExplorerColumn } from 'vs/workbench/services/positronDataExplorer/browser/positronDataExplorerColumn'; import { DataExplorerClientInstance } from 'vs/workbench/services/languageRuntime/common/languageRuntimeDataExplorerClient'; -import { ColumnSortKey, RowFilter, SchemaUpdateEvent } from 'vs/workbench/services/languageRuntime/common/positronDataExplorerComm'; +import { BackendState, RowFilter, SchemaUpdateEvent } from 'vs/workbench/services/languageRuntime/common/positronDataExplorerComm'; /** * TableDataDataGridInstance class. @@ -75,15 +75,29 @@ export class TableDataDataGridInstance extends DataGridInstance { )); // Add the onDidSchemaUpdate event handler. - this._dataExplorerClientInstance.onDidSchemaUpdate(async (e: SchemaUpdateEvent) => { + this._register(this._dataExplorerClientInstance.onDidSchemaUpdate(async (e: SchemaUpdateEvent) => { this.softReset(); this.fetchData(); - }); + })); // Add the onDidDataUpdate event handler. - this._dataExplorerClientInstance.onDidDataUpdate(async () => { + this._register(this._dataExplorerClientInstance.onDidDataUpdate(async () => { this.fetchData(); - }); + })); + + // Add the onDidUpdateBackendState event handler. + this._register(this._dataExplorerClientInstance.onDidUpdateBackendState( + async (state: BackendState) => { + // Clear column sort keys. + this._columnSortKeys.clear(); + state.sort_keys.forEach((key, sortIndex) => { + this._columnSortKeys.set(key.column_index, + new ColumnSortKeyDescriptor(sortIndex, key.column_index, key.ascending) + ); + }); + this._onDidUpdateEmitter.fire(); + } + )); } //#endregion Constructor @@ -118,7 +132,7 @@ export class TableDataDataGridInstance extends DataGridInstance { { column_index: columnSort.columnIndex, ascending: columnSort.ascending - } satisfies ColumnSortKey + } ))); // Clear the data cache and fetch new data. @@ -204,6 +218,9 @@ export class TableDataDataGridInstance extends DataGridInstance { // Set the row filters. await this._dataExplorerClientInstance.setRowFilters(filters); + // Synchronize the backend state. + await this._dataExplorerClientInstance.updateBackendState(); + // Reload the data grid. this._dataExplorerCache.invalidateDataCache(); this.softReset(); diff --git a/src/vs/workbench/services/positronDataExplorer/common/columnSchemaCache.ts b/src/vs/workbench/services/positronDataExplorer/common/columnSchemaCache.ts index 1ef52d0fe2a..5db8f156b62 100644 --- a/src/vs/workbench/services/positronDataExplorer/common/columnSchemaCache.ts +++ b/src/vs/workbench/services/positronDataExplorer/common/columnSchemaCache.ts @@ -173,7 +173,7 @@ export class ColumnSchemaCache extends Disposable { this._searchText = searchText; // // Get the size of the data. - // const tableState = await this._dataExplorerClientInstance.getState(); + // const tableState = await this._dataExplorerClientInstance.getBackendState(); // this._columns = tableState.table_shape.num_columns; // Set the start column index and the end column index of the columns to cache. diff --git a/src/vs/workbench/services/positronDataExplorer/common/dataExplorerCache.ts b/src/vs/workbench/services/positronDataExplorer/common/dataExplorerCache.ts index 2ccfe3da221..18de36157c5 100644 --- a/src/vs/workbench/services/positronDataExplorer/common/dataExplorerCache.ts +++ b/src/vs/workbench/services/positronDataExplorer/common/dataExplorerCache.ts @@ -274,7 +274,7 @@ export class DataExplorerCache extends Disposable { } = cacheUpdateDescriptor; // Get the size of the data. - const tableState = await this._dataExplorerClientInstance.getState(); + const tableState = await this._dataExplorerClientInstance.getBackendState(); this._columns = tableState.table_shape.num_columns; this._rows = tableState.table_shape.num_rows; diff --git a/src/vs/workbench/services/positronDataExplorer/common/positronDataExplorerMocks.ts b/src/vs/workbench/services/positronDataExplorer/common/positronDataExplorerMocks.ts index 91b03069d14..5773fe3bee9 100644 --- a/src/vs/workbench/services/positronDataExplorer/common/positronDataExplorerMocks.ts +++ b/src/vs/workbench/services/positronDataExplorer/common/positronDataExplorerMocks.ts @@ -11,22 +11,24 @@ import { RowFilter, RowFilterType, TableData, - TableSchema + TableSchema, + RowFilterCondition, + ColumnDisplayType } from 'vs/workbench/services/languageRuntime/common/positronDataExplorerComm'; -const exampleTypes: [string, string, object][] = [ - ['int64', 'number', {}], - ['string', 'string', {}], - ['boolean', 'boolean', {}], - ['double', 'number', {}], - ['timestamp', 'datetime', { timezone: 'America/New_York' }] +const exampleTypes: [string, ColumnDisplayType, object][] = [ + ['int64', ColumnDisplayType.Number, {}], + ['string', ColumnDisplayType.String, {}], + ['boolean', ColumnDisplayType.Boolean, {}], + ['double', ColumnDisplayType.Number, {}], + ['timestamp', ColumnDisplayType.Datetime, { timezone: 'America/New_York' }] ]; export function getTableSchema(numRows: number = 100, numColumns: number = 10): TableSchema { const columns = []; for (let i = 0; i < numColumns; i++) { const typeProto = exampleTypes[i % exampleTypes.length]; - columns.push(getColumnSchema('column_' + i, typeProto[0], typeProto[1], typeProto[2])); + columns.push(getColumnSchema('column_' + i, i, typeProto[0], typeProto[1], typeProto[2])); } return { columns: columns, @@ -77,14 +79,16 @@ export function getExampleTableData(shape: [number, number], schema: TableSchema }; } -export function getColumnSchema(colName: string, typeName: string, typeDisplay: string, +export function getColumnSchema(column_name: string, column_index: number, + type_name: string, type_display: ColumnDisplayType, extraProps: object = {}): ColumnSchema { return { - column_name: colName, - type_name: typeName, - type_display: typeDisplay, + column_name, + column_index, + type_name, + type_display, ...extraProps - } as ColumnSchema; + }; } export function getExampleHistogram(): ColumnProfileResult { @@ -119,39 +123,53 @@ export function getExampleFreqtable(): ColumnProfileResult { // For filtering -function _getFilterWithProps(columnIndex: number, filterType: RowFilterType, - props: Partial = {}): RowFilter { +function _getCommonFilterProps(column_schema: ColumnSchema, filter_type: RowFilterType) { return { filter_id: generateUuid(), - filter_type: filterType, - column_index: columnIndex, - ...props - } as RowFilter; + filter_type, + column_schema, + condition: RowFilterCondition.And + }; } -export function getCompareFilter(columnIndex: number, op: CompareFilterParamsOp, - value: string) { - return _getFilterWithProps(columnIndex, RowFilterType.Compare, - { compare_params: { op, value } }); +export function getCompareFilter(columnSchema: ColumnSchema, op: CompareFilterParamsOp, + value: string): RowFilter { + return { + ..._getCommonFilterProps(columnSchema, RowFilterType.Compare), + compare_params: { + op, value + } + }; } -export function getIsNullFilter(columnIndex: number) { - return _getFilterWithProps(columnIndex, RowFilterType.IsNull); +export function getIsNullFilter(columnSchema: ColumnSchema): RowFilter { + return { + ..._getCommonFilterProps(columnSchema, RowFilterType.IsNull) + }; } -export function getNotNullFilter(columnIndex: number) { - return _getFilterWithProps(columnIndex, RowFilterType.NotNull); +export function getNotNullFilter(columnSchema: ColumnSchema): RowFilter { + return { + ..._getCommonFilterProps(columnSchema, RowFilterType.NotNull) + }; } -export function getSetMemberFilter(columnIndex: number, values: string[], inclusive: boolean) { - return _getFilterWithProps(columnIndex, RowFilterType.SetMembership, { - set_membership_params: { values, inclusive } - }); +export function getSetMemberFilter(columnSchema: ColumnSchema, values: string[], + inclusive: boolean): RowFilter { + return { + ..._getCommonFilterProps(columnSchema, RowFilterType.SetMembership), + set_membership_params: { + values, inclusive + } + }; } -export function getTextSearchFilter(columnIndex: number, searchTerm: string, - searchType: SearchFilterType, caseSensitive: boolean) { - return _getFilterWithProps(columnIndex, RowFilterType.Search, { - search_params: { term: searchTerm, search_type: searchType, case_sensitive: caseSensitive } - }); +export function getTextSearchFilter(columnSchema: ColumnSchema, searchTerm: string, + searchType: SearchFilterType, caseSensitive: boolean): RowFilter { + return { + ..._getCommonFilterProps(columnSchema, RowFilterType.Search), + search_params: { + term: searchTerm, search_type: searchType, case_sensitive: caseSensitive + } + }; } diff --git a/src/vs/workbench/services/positronDataExplorer/test/common/positronDataExplorerMocks.test.ts b/src/vs/workbench/services/positronDataExplorer/test/common/positronDataExplorerMocks.test.ts index 83f530f36c8..8a03b3ed663 100644 --- a/src/vs/workbench/services/positronDataExplorer/test/common/positronDataExplorerMocks.test.ts +++ b/src/vs/workbench/services/positronDataExplorer/test/common/positronDataExplorerMocks.test.ts @@ -39,9 +39,11 @@ suite('DataExplorerMocks', () => { }); test('Test getCompareFilter', () => { - const filter = mocks.getCompareFilter(2, CompareFilterParamsOp.Gt, '1234'); + const schema = mocks.getTableSchema(1000, 10); + + const filter = mocks.getCompareFilter(schema.columns[2], CompareFilterParamsOp.Gt, '1234'); assert.equal(filter.filter_type, RowFilterType.Compare); - assert.equal(filter.column_index, 2); + assert.equal(filter.column_schema.column_index, 2); const params = filter.compare_params!; @@ -50,18 +52,20 @@ suite('DataExplorerMocks', () => { }); test('Test getIsNullFilter', () => { - let filter = mocks.getIsNullFilter(3); - assert.equal(filter.column_index, 3); + const schema = mocks.getTableSchema(1000, 10); + let filter = mocks.getIsNullFilter(schema.columns[3]); + assert.equal(filter.column_schema.column_index, 3); assert.equal(filter.filter_type, RowFilterType.IsNull); - filter = mocks.getNotNullFilter(3); + filter = mocks.getNotNullFilter(schema.columns[3]); assert.equal(filter.filter_type, RowFilterType.NotNull); }); test('Test getTextSearchFilter', () => { - const filter = mocks.getTextSearchFilter(5, 'needle', + const schema = mocks.getTableSchema(1000, 10); + const filter = mocks.getTextSearchFilter(schema.columns[5], 'needle', SearchFilterType.Contains, false); - assert.equal(filter.column_index, 5); + assert.equal(filter.column_schema.column_index, 5); assert.equal(filter.filter_type, RowFilterType.Search); const params = filter.search_params!; @@ -72,9 +76,10 @@ suite('DataExplorerMocks', () => { }); test('Test getSetMemberFilter', () => { + const schema = mocks.getTableSchema(1000, 10); const set_values = ['need1', 'need2']; - const filter = mocks.getSetMemberFilter(6, set_values, true); - assert.equal(filter.column_index, 6); + const filter = mocks.getSetMemberFilter(schema.columns[6], set_values, true); + assert.equal(filter.column_schema.column_index, 6); assert.equal(filter.filter_type, RowFilterType.SetMembership); const params = filter.set_membership_params!;