Skip to content

Commit

Permalink
Data Explorer: Update data explorer comm and adapt to RPC protocol ch…
Browse files Browse the repository at this point in the history
…anges related to filtering (#424)

posit-dev/positron#3904 proposes some refactoring to the set_row_filters RPC that makes the filter-specific parameters a union and renames things a bit to enable some code reuse with to-be-implemented column filters.
  • Loading branch information
wesm authored Jul 8, 2024
1 parent 4ea2c07 commit d93dc2a
Show file tree
Hide file tree
Showing 4 changed files with 136 additions and 120 deletions.
119 changes: 89 additions & 30 deletions crates/amalthea/src/comm/data_explorer_comm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ pub struct SearchSchemaResult {
/// A schema containing matching columns up to the max_results limit
pub matches: Option<TableSchema>,

/// The total number of columns matching the search term
/// The total number of columns matching the filter
pub total_num_matches: i64
}

Expand Down Expand Up @@ -165,17 +165,8 @@ pub struct RowFilter {
/// Optional error message when the filter is invalid
pub error_message: Option<String>,

/// Parameters for the 'between' and 'not_between' filter types
pub between_params: Option<BetweenFilterParams>,

/// Parameters for the 'compare' filter type
pub compare_params: Option<CompareFilterParams>,

/// Parameters for the 'search' filter type
pub search_params: Option<SearchFilterParams>,

/// Parameters for the 'set_membership' filter type
pub set_membership_params: Option<SetMembershipFilterParams>
/// The row filter type-specific parameters
pub params: Option<RowFilterParams>
}

/// Support status for a row filter type
Expand All @@ -190,7 +181,7 @@ pub struct RowFilterTypeSupportStatus {

/// Parameters for the 'between' and 'not_between' filter types
#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
pub struct BetweenFilterParams {
pub struct FilterBetween {
/// The lower limit for filtering
pub left_value: String,

Expand All @@ -200,18 +191,18 @@ pub struct BetweenFilterParams {

/// Parameters for the 'compare' filter type
#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
pub struct CompareFilterParams {
pub struct FilterComparison {
/// String representation of a binary comparison
pub op: CompareFilterParamsOp,
pub op: FilterComparisonOp,

/// A stringified column value for a comparison filter
pub value: String
}

/// Parameters for the 'set_membership' filter type
#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
pub struct SetMembershipFilterParams {
/// Array of column values for a set membership filter
pub struct FilterSetMembership {
/// Array of values for a set membership filter
pub values: Vec<String>,

/// Filter by including only values passed (true) or excluding (false)
Expand All @@ -220,17 +211,45 @@ pub struct SetMembershipFilterParams {

/// Parameters for the 'search' filter type
#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
pub struct SearchFilterParams {
pub struct FilterTextSearch {
/// Type of search to perform
pub search_type: SearchFilterType,
pub search_type: TextSearchType,

/// String value/regex to search for in stringified data
/// String value/regex to search for
pub term: String,

/// If true, do a case-sensitive search, otherwise case-insensitive
pub case_sensitive: bool
}

/// Parameters for the 'match_data_types' filter type
#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
pub struct FilterMatchDataTypes {
/// Column display types to match
pub display_types: Vec<ColumnDisplayType>
}

/// A filter that selects a subset of columns by name, type, or other
/// criteria
#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
pub struct ColumnFilter {
/// Type of column filter to apply
pub filter_type: ColumnFilterType,

/// Parameters for column filter
pub params: ColumnFilterParams
}

/// Support status for a column filter type
#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
pub struct ColumnFilterTypeSupportStatus {
/// Type of column filter
pub column_filter_type: ColumnFilterType,

/// The support status for this column filter type
pub support_status: SupportStatus
}

/// A single column profile request
#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
pub struct ColumnProfileRequest {
Expand Down Expand Up @@ -447,7 +466,10 @@ pub struct SupportedFeatures {
#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
pub struct SearchSchemaFeatures {
/// The support status for this RPC method
pub support_status: SupportStatus
pub support_status: SupportStatus,

/// A list of supported types
pub supported_types: Vec<ColumnFilterTypeSupportStatus>
}

/// Feature flags for 'set_row_filters' RPC
Expand Down Expand Up @@ -477,7 +499,10 @@ pub struct GetColumnProfilesFeatures {
#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
pub struct ExportDataSelectionFeatures {
/// The support status for this RPC method
pub support_status: SupportStatus
pub support_status: SupportStatus,

/// Export formats supported
pub supported_formats: Vec<ExportFormat>
}

/// Feature flags for 'set_sort_columns' RPC
Expand Down Expand Up @@ -622,9 +647,9 @@ pub enum RowFilterType {
SetMembership
}

/// Possible values for Op in CompareFilterParams
/// Possible values for Op in FilterComparison
#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
pub enum CompareFilterParamsOp {
pub enum FilterComparisonOp {
#[serde(rename = "=")]
Eq,

Expand All @@ -644,9 +669,9 @@ pub enum CompareFilterParamsOp {
GtEq
}

/// Possible values for SearchFilterType
/// Possible values for TextSearchType
#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
pub enum SearchFilterType {
pub enum TextSearchType {
#[serde(rename = "contains")]
Contains,

Expand All @@ -660,6 +685,16 @@ pub enum SearchFilterType {
RegexMatch
}

/// Possible values for ColumnFilterType
#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
pub enum ColumnFilterType {
#[serde(rename = "text_search")]
TextSearch,

#[serde(rename = "match_data_types")]
MatchDataTypes
}

/// Possible values for ColumnProfileType
#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
pub enum ColumnProfileType {
Expand Down Expand Up @@ -733,6 +768,30 @@ pub enum ColumnValue {
FormattedValue(String)
}

/// Union type RowFilterParams
/// Union of row filter parameters
#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
#[serde(untagged)]
pub enum RowFilterParams {
Between(FilterBetween),

Comparison(FilterComparison),

TextSearch(FilterTextSearch),

SetMembership(FilterSetMembership)
}

/// Union type ColumnFilterParams
/// Union of column filter type-specific parameters
#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
#[serde(untagged)]
pub enum ColumnFilterParams {
TextSearch(FilterTextSearch),

MatchDataTypes(FilterMatchDataTypes)
}

/// Union type Selection in Properties
#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
#[serde(untagged)]
Expand Down Expand Up @@ -760,10 +819,10 @@ pub struct GetSchemaParams {
/// Parameters for the SearchSchema method.
#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
pub struct SearchSchemaParams {
/// Substring to match for (currently case insensitive)
pub search_term: String,
/// Column filters to apply when searching
pub filters: Vec<ColumnFilter>,

/// Index (starting from zero) of first result to fetch
/// Index (starting from zero) of first result to fetch (for paging)
pub start_index: i64,

/// Maximum number of resulting column schemas to fetch from the start
Expand Down Expand Up @@ -835,7 +894,7 @@ pub enum DataExplorerBackendRequest {
#[serde(rename = "get_schema")]
GetSchema(GetSchemaParams),

/// Search schema by column name
/// Search schema with column filters
///
/// Search schema for column names matching a passed substring
#[serde(rename = "search_schema")]
Expand Down
21 changes: 15 additions & 6 deletions crates/ark/src/data_explorer/r_data_explorer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ use amalthea::comm::data_explorer_comm::ColumnSchema;
use amalthea::comm::data_explorer_comm::ColumnSortKey;
use amalthea::comm::data_explorer_comm::ColumnSummaryStats;
use amalthea::comm::data_explorer_comm::ColumnValue;
use amalthea::comm::data_explorer_comm::CompareFilterParamsOp;
use amalthea::comm::data_explorer_comm::DataExplorerBackendReply;
use amalthea::comm::data_explorer_comm::DataExplorerBackendRequest;
use amalthea::comm::data_explorer_comm::DataExplorerFrontendEvent;
Expand All @@ -27,13 +26,15 @@ use amalthea::comm::data_explorer_comm::ExportDataSelectionFeatures;
use amalthea::comm::data_explorer_comm::ExportDataSelectionParams;
use amalthea::comm::data_explorer_comm::ExportFormat;
use amalthea::comm::data_explorer_comm::ExportedData;
use amalthea::comm::data_explorer_comm::FilterComparisonOp;
use amalthea::comm::data_explorer_comm::FilterResult;
use amalthea::comm::data_explorer_comm::FormatOptions;
use amalthea::comm::data_explorer_comm::GetColumnProfilesFeatures;
use amalthea::comm::data_explorer_comm::GetColumnProfilesParams;
use amalthea::comm::data_explorer_comm::GetDataValuesParams;
use amalthea::comm::data_explorer_comm::GetSchemaParams;
use amalthea::comm::data_explorer_comm::RowFilter;
use amalthea::comm::data_explorer_comm::RowFilterParams;
use amalthea::comm::data_explorer_comm::RowFilterType;
use amalthea::comm::data_explorer_comm::RowFilterTypeSupportStatus;
use amalthea::comm::data_explorer_comm::SearchSchemaFeatures;
Expand Down Expand Up @@ -790,11 +791,13 @@ impl RDataExplorer {
Ok(display_type == &ColumnDisplayType::String)
},
RowFilterType::Compare => {
if let Some(compare_params) = &filter.compare_params {
let compare_op = &compare_params.op;
match compare_op {
CompareFilterParamsOp::Eq | CompareFilterParamsOp::NotEq => Ok(true),
_ => Ok(is_compare_supported(display_type)),
if let Some(params) = &filter.params {
match params {
RowFilterParams::Comparison(comparison) => match comparison.op {
FilterComparisonOp::Eq | FilterComparisonOp::NotEq => Ok(true),
_ => Ok(is_compare_supported(display_type)),
},
_ => Err(anyhow!("Missing compare filter params")),
}
} else {
Err(anyhow!("Missing compare_params for filter"))
Expand Down Expand Up @@ -934,6 +937,7 @@ impl RDataExplorer {
},
search_schema: SearchSchemaFeatures {
support_status: SupportStatus::Unsupported,
supported_types: vec![],
},
set_row_filters: SetRowFiltersFeatures {
support_status: SupportStatus::Supported,
Expand Down Expand Up @@ -965,6 +969,11 @@ impl RDataExplorer {
},
export_data_selection: ExportDataSelectionFeatures {
support_status: SupportStatus::Supported,
supported_formats: vec![
ExportFormat::Csv,
ExportFormat::Tsv,
ExportFormat::Html,
],
},
},
};
Expand Down
18 changes: 2 additions & 16 deletions crates/ark/src/modules/positron/r_data_explorer.R
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ summary_stats_get_timezone <- function(x) {
if (!inherits(x, "POSIXt")) {
stop("Timezone can't be obtained for this object type")
}

timezone <- tz(x)

# When the timezone is reported as "", it will actually be formatted
Expand All @@ -110,15 +110,6 @@ col_filter_indices <- function(col, idx = NULL) {
# Are we working with a matrix here?
is_matrix <- is.matrix(table)

# Mapping of filter types to parameter arguments
filter_params <- list(
compare = "compare_params",
between = "between_params",
not_between = "between_params",
search = "search_params",
set_membership = "set_membership_params"
)

# Create the initial set of indices
indices <- rep(TRUE, nrow(table))
row_filters_errors <- character(length(row_filters))
Expand All @@ -137,12 +128,7 @@ col_filter_indices <- function(col, idx = NULL) {

# Get the parameters for the filter function. Not all functions have
# parameters.
param_name <- filter_params[[row_filter$filter_type]]
params <- if (is.null(param_name)) {
NULL
} else {
row_filter[[param_name]]
}
params <- row_filter$params

# Each filter function accepts the column and the parameters as
# arguments.
Expand Down
Loading

0 comments on commit d93dc2a

Please sign in to comment.