Skip to content

Commit

Permalink
Rename all instances of schema to either view_columns or `selecte…
Browse files Browse the repository at this point in the history
…d_columns`, depending on context
  • Loading branch information
abey79 committed Oct 3, 2024
1 parent d223e0e commit d549de9
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 34 deletions.
34 changes: 18 additions & 16 deletions crates/viewer/re_space_view_dataframe/src/dataframe_ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,32 +37,33 @@ pub(crate) fn dataframe_ui(
) -> Vec<HideColumnAction> {
re_tracing::profile_function!();

let schema = query_handle
let selected_columns = query_handle
.selected_contents()
.iter()
.map(|(_, desc)| desc.clone())
.collect::<Vec<_>>();

// The table id mainly drives column widths, so it should be stable across queries leading to
// the same schema. However, changing the PoV typically leads to large changes of actual
// content. Since that can affect the optimal column width, we include the PoV in the salt.
// the same set of selected columns. However, changing the PoV typically leads to large changes
// of actual content. Since that can affect the optimal column width, we include the PoV in the
// salt.
let table_id_salt = egui::Id::new("__dataframe__")
.with(&schema)
.with(&selected_columns)
.with(&query_handle.query().filtered_point_of_view);

// For the row expansion cache, we invalidate more aggressively for now.
let row_expansion_id_salt = egui::Id::new("__dataframe_row_exp__")
.with(&schema)
.with(&selected_columns)
.with(query_handle.query());

let (header_groups, header_entity_paths) = column_groups_for_entity(&schema);
let (header_groups, header_entity_paths) = column_groups_for_entity(&selected_columns);

let num_rows = query_handle.num_rows();

let mut table_delegate = DataframeTableDelegate {
ctx,
query_handle,
schema: &schema,
selected_columns: &selected_columns,
header_entity_paths,
num_rows,
display_data: Err(anyhow::anyhow!(
Expand All @@ -77,7 +78,7 @@ pub(crate) fn dataframe_ui(
hide_column_actions: vec![],
};

let num_sticky_cols = schema
let num_sticky_cols = selected_columns
.iter()
.take_while(|cd| matches!(cd, ColumnDescriptor::Control(_) | ColumnDescriptor::Time(_)))
.count();
Expand All @@ -86,7 +87,7 @@ pub(crate) fn dataframe_ui(
egui_table::Table::new()
.id_salt(table_id_salt)
.columns(
schema
selected_columns
.iter()
.map(|column_descr| {
egui_table::Column::new(200.0)
Expand Down Expand Up @@ -143,12 +144,12 @@ impl RowsDisplayData {
fn try_new(
row_indices: &Range<u64>,
row_data: Vec<Vec<Box<dyn ArrowArray>>>,
schema: &[ColumnDescriptor],
selected_columns: &[ColumnDescriptor],
query_timeline: &Timeline,
) -> Result<Self, DisplayRecordBatchError> {
let display_record_batches = row_data
.into_iter()
.map(|data| DisplayRecordBatch::try_new(&data, schema))
.map(|data| DisplayRecordBatch::try_new(&data, selected_columns))
.collect::<Result<Vec<_>, _>>()?;

let mut batch_ref_from_row = BTreeMap::new();
Expand All @@ -162,7 +163,7 @@ impl RowsDisplayData {
}

// find the time column
let query_time_column_index = schema
let query_time_column_index = selected_columns
.iter()
.find_position(|desc| match desc {
ColumnDescriptor::Time(time_column_desc) => {
Expand All @@ -173,7 +174,7 @@ impl RowsDisplayData {
.map(|(pos, _)| pos);

// find the row id column
let row_id_column_index = schema
let row_id_column_index = selected_columns
.iter()
.find_position(|desc| match desc {
ColumnDescriptor::Control(control_column_desc) => {
Expand All @@ -196,7 +197,7 @@ impl RowsDisplayData {
struct DataframeTableDelegate<'a> {
ctx: &'a ViewerContext<'a>,
query_handle: &'a QueryHandle<'a>,
schema: &'a [ColumnDescriptor],
selected_columns: &'a [ColumnDescriptor],
header_entity_paths: Vec<Option<EntityPath>>,
display_data: anyhow::Result<RowsDisplayData>,

Expand All @@ -222,7 +223,8 @@ impl<'a> egui_table::TableDelegate for DataframeTableDelegate<'a> {
.take((info.visible_rows.end - info.visible_rows.start) as usize)
.collect();

let data = RowsDisplayData::try_new(&info.visible_rows, data, self.schema, &timeline);
let data =
RowsDisplayData::try_new(&info.visible_rows, data, self.selected_columns, &timeline);

self.display_data = data.context("Failed to create display data");
}
Expand Down Expand Up @@ -264,7 +266,7 @@ impl<'a> egui_table::TableDelegate for DataframeTableDelegate<'a> {
);
}
} else if cell.row_nr == 1 {
let column = &self.schema[cell.col_range.start];
let column = &self.selected_columns[cell.col_range.start];

// if this column can actually be hidden, then that's the corresponding action
let hide_action = match column {
Expand Down
21 changes: 12 additions & 9 deletions crates/viewer/re_space_view_dataframe/src/display_record_batch.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//! Intermediate data structures to make `re_datastore`'s schemas and row data more amenable to
//! displaying in a table.
//! Intermediate data structures to make `re_datastore`'s row data more amenable to displaying in a
//! table.
use thiserror::Error;

Expand Down Expand Up @@ -187,10 +187,10 @@ pub(crate) enum DisplayColumn {
impl DisplayColumn {
#[allow(clippy::borrowed_box)] // https://github.com/rust-lang/rust-clippy/issues/11940
fn try_new(
column_schema: &ColumnDescriptor,
column_descriptor: &ColumnDescriptor,
column_data: &Box<dyn ArrowArray>,
) -> Result<Self, DisplayRecordBatchError> {
match column_schema {
match column_descriptor {
ColumnDescriptor::Control(desc) => {
if desc.component_name == RowId::name() {
let row_ids = column_data
Expand Down Expand Up @@ -373,19 +373,22 @@ pub(crate) struct DisplayRecordBatch {
}

impl DisplayRecordBatch {
/// Create a new `DisplayRecordBatch` from a `RecordBatch` and its schema.
/// Create a new `DisplayRecordBatch` from a `RecordBatch` and its list of selected columns.
///
/// The columns in the record batch must match the schema. This is guaranteed by `re_datastore`.
/// The columns in the record batch must match the selected columns. This is guaranteed by
/// `re_datastore`.
pub(crate) fn try_new(
row_data: &Vec<Box<dyn ArrowArray>>,
schema: &[ColumnDescriptor],
selected_columns: &[ColumnDescriptor],
) -> Result<Self, DisplayRecordBatchError> {
let num_rows = row_data.first().map(|arr| arr.len()).unwrap_or(0);

let columns: Result<Vec<_>, _> = schema
let columns: Result<Vec<_>, _> = selected_columns
.iter()
.zip(row_data)
.map(|(column_schema, column_data)| DisplayColumn::try_new(column_schema, column_data))
.map(|(column_descriptor, column_data)| {
DisplayColumn::try_new(column_descriptor, column_data)
})
.collect();

Ok(Self {
Expand Down
19 changes: 10 additions & 9 deletions crates/viewer/re_space_view_dataframe/src/space_view_class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ struct DataframeSpaceViewState {
/// Cache for the expanded rows.
expended_rows_cache: ExpandedRowsCache,

/// Schema for the current query, cached here for the column visibility UI.
schema: Option<Vec<ColumnDescriptor>>,
/// List of view columns for the current query, cached here for the column visibility UI.
view_columns: Option<Vec<ColumnDescriptor>>,
}

impl SpaceViewState for DataframeSpaceViewState {
Expand Down Expand Up @@ -106,12 +106,12 @@ mode sets the default time range to _everything_. You can override this in the s
) -> Result<(), SpaceViewSystemExecutionError> {
let state = state.downcast_mut::<DataframeSpaceViewState>()?;
let view_query = view_query::Query::from_blueprint(ctx, space_view_id);
let Some(schema) = &state.schema else {
let Some(view_columns) = &state.view_columns else {
// Shouldn't happen, except maybe on the first frame, which is too early
// for the user to click the menu anyway.
return Ok(());
};
view_query.selection_panel_ui(ctx, ui, space_view_id, schema)
view_query.selection_panel_ui(ctx, ui, space_view_id, view_columns)
}

fn ui(
Expand Down Expand Up @@ -150,16 +150,17 @@ mode sets the default time range to _everything_. You can override this in the s
SparseFillStrategy::None
};

let schema = query_engine.schema_for_view_contents(&view_contents);
let selection = view_query.apply_column_visibility_to_view_columns(ctx, &schema)?;
let view_columns = query_engine.schema_for_view_contents(&view_contents);
let selected_columns =
view_query.apply_column_visibility_to_view_columns(ctx, &view_columns)?;

let dataframe_query = re_chunk_store::QueryExpression2 {
view_contents: Some(view_contents),
filtered_index: view_query.timeline(ctx)?,
filtered_index_range: Some(view_query.filter_by_range()?),
filtered_point_of_view,
sparse_fill_strategy,
selection,
selection: selected_columns,

// not yet unsupported by the dataframe view
filtered_index_values: None,
Expand All @@ -171,9 +172,9 @@ mode sets the default time range to _everything_. You can override this in the s
let hide_column_actions =
dataframe_ui(ctx, ui, &query_handle, &mut state.expended_rows_cache);

view_query.handle_hide_column_actions(ctx, &schema, hide_column_actions)?;
view_query.handle_hide_column_actions(ctx, &view_columns, hide_column_actions)?;

state.schema = Some(schema);
state.view_columns = Some(view_columns);
Ok(())
}
}
Expand Down

0 comments on commit d549de9

Please sign in to comment.