From e94873d2be654ead12430a27739646d6abc6409b Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Fri, 11 Oct 2024 09:44:13 -0400 Subject: [PATCH 1/8] Rename component -> component_name --- crates/store/re_chunk_store/src/dataframe.rs | 18 ++++++------ crates/store/re_dataframe/src/query.rs | 28 +++++++++---------- .../src/view_query/blueprint.rs | 8 ++++-- rerun_py/src/dataframe.rs | 4 +-- 4 files changed, 29 insertions(+), 29 deletions(-) diff --git a/crates/store/re_chunk_store/src/dataframe.rs b/crates/store/re_chunk_store/src/dataframe.rs index 10433e05e1af..18a25c1dbe0c 100644 --- a/crates/store/re_chunk_store/src/dataframe.rs +++ b/crates/store/re_chunk_store/src/dataframe.rs @@ -422,9 +422,7 @@ pub struct ComponentColumnSelector { pub entity_path: EntityPath, /// Semantic name associated with this data. - // - // TODO(cmc): this should be `component_name`. - pub component: ComponentName, + pub component_name: ComponentName, /// How to join the data into the `RecordBatch`. // @@ -437,7 +435,7 @@ impl From for ComponentColumnSelector { fn from(desc: ComponentColumnDescriptor) -> Self { Self { entity_path: desc.entity_path.clone(), - component: desc.component_name, + component_name: desc.component_name, join_encoding: desc.join_encoding, } } @@ -449,7 +447,7 @@ impl ComponentColumnSelector { pub fn new(entity_path: EntityPath) -> Self { Self { entity_path, - component: C::name(), + component_name: C::name(), join_encoding: JoinEncoding::default(), } } @@ -459,7 +457,7 @@ impl ComponentColumnSelector { pub fn new_for_component_name(entity_path: EntityPath, component: ComponentName) -> Self { Self { entity_path, - component, + component_name: component, join_encoding: JoinEncoding::default(), } } @@ -476,7 +474,7 @@ impl std::fmt::Display for ComponentColumnSelector { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { let Self { entity_path, - component, + component_name: component, join_encoding: _, } = self; @@ -799,7 +797,7 @@ impl ChunkStore { is_tombstone, is_semantically_empty, } = self - .lookup_column_metadata(&selector.entity_path, &selector.component) + .lookup_column_metadata(&selector.entity_path, &selector.component_name) .unwrap_or(ColumnMetadata { is_static: false, is_indicator: false, @@ -808,7 +806,7 @@ impl ChunkStore { }); let datatype = self - .lookup_datatype(&selector.component) + .lookup_datatype(&selector.component_name) .cloned() .unwrap_or_else(|| ArrowDatatype::Null); @@ -816,7 +814,7 @@ impl ChunkStore { entity_path: selector.entity_path.clone(), archetype_name: None, archetype_field_name: None, - component_name: selector.component, + component_name: selector.component_name, store_datatype: ArrowListArray::::default_datatype(datatype.clone()), join_encoding: selector.join_encoding, is_static, diff --git a/crates/store/re_dataframe/src/query.rs b/crates/store/re_dataframe/src/query.rs index 2eb70680b956..75e0a0a4ca5f 100644 --- a/crates/store/re_dataframe/src/query.rs +++ b/crates/store/re_dataframe/src/query.rs @@ -351,7 +351,7 @@ impl QueryHandle<'_> { ColumnSelector::Component(selected_column) => { let ComponentColumnSelector { entity_path: selected_entity_path, - component: selected_component_name, + component_name: selected_component_name, join_encoding: _, } = selected_column; @@ -418,7 +418,7 @@ impl QueryHandle<'_> { if let Some(pov) = self.query.filtered_point_of_view.as_ref() { if pov.entity_path == column.entity_path - && pov.component == column.component_name + && pov.component_name == column.component_name { view_pov_chunks_idx = Some(idx); } @@ -1437,7 +1437,7 @@ mod tests { let mut query = QueryExpression::new(timeline); query.filtered_point_of_view = Some(ComponentColumnSelector { entity_path: "no/such/entity".into(), - component: MyPoint::name(), + component_name: MyPoint::name(), join_encoding: Default::default(), }); eprintln!("{query:#?}:"); @@ -1464,7 +1464,7 @@ mod tests { let mut query = QueryExpression::new(timeline); query.filtered_point_of_view = Some(ComponentColumnSelector { entity_path: entity_path.clone(), - component: "AComponentColumnThatDoesntExist".into(), + component_name: "AComponentColumnThatDoesntExist".into(), join_encoding: Default::default(), }); eprintln!("{query:#?}:"); @@ -1491,7 +1491,7 @@ mod tests { let mut query = QueryExpression::new(timeline); query.filtered_point_of_view = Some(ComponentColumnSelector { entity_path: entity_path.clone(), - component: MyPoint::name(), + component_name: MyPoint::name(), join_encoding: Default::default(), }); eprintln!("{query:#?}:"); @@ -1528,7 +1528,7 @@ mod tests { let mut query = QueryExpression::new(timeline); query.filtered_point_of_view = Some(ComponentColumnSelector { entity_path: entity_path.clone(), - component: MyColor::name(), + component_name: MyColor::name(), join_encoding: Default::default(), }); eprintln!("{query:#?}:"); @@ -1739,22 +1739,22 @@ mod tests { query.selection = Some(vec![ ColumnSelector::Component(ComponentColumnSelector { entity_path: entity_path.clone(), - component: MyColor::name(), + component_name: MyColor::name(), join_encoding: Default::default(), }), ColumnSelector::Component(ComponentColumnSelector { entity_path: entity_path.clone(), - component: MyColor::name(), + component_name: MyColor::name(), join_encoding: Default::default(), }), ColumnSelector::Component(ComponentColumnSelector { entity_path: "non_existing_entity".into(), - component: MyColor::name(), + component_name: MyColor::name(), join_encoding: Default::default(), }), ColumnSelector::Component(ComponentColumnSelector { entity_path: entity_path.clone(), - component: "AComponentColumnThatDoesntExist".into(), + component_name: "AComponentColumnThatDoesntExist".into(), join_encoding: Default::default(), }), ]); @@ -1828,17 +1828,17 @@ mod tests { // ColumnSelector::Component(ComponentColumnSelector { entity_path: entity_path.clone(), - component: MyPoint::name(), + component_name: MyPoint::name(), join_encoding: Default::default(), }), ColumnSelector::Component(ComponentColumnSelector { entity_path: entity_path.clone(), - component: MyColor::name(), + component_name: MyColor::name(), join_encoding: Default::default(), }), ColumnSelector::Component(ComponentColumnSelector { entity_path: entity_path.clone(), - component: MyLabel::name(), + component_name: MyLabel::name(), join_encoding: Default::default(), }), ]); @@ -2020,7 +2020,7 @@ mod tests { let mut query = QueryExpression::new(timeline); query.filtered_point_of_view = Some(ComponentColumnSelector { entity_path: entity_path.clone(), - component: MyPoint::name(), + component_name: MyPoint::name(), join_encoding: Default::default(), }); eprintln!("{query:#?}:"); diff --git a/crates/viewer/re_space_view_dataframe/src/view_query/blueprint.rs b/crates/viewer/re_space_view_dataframe/src/view_query/blueprint.rs index 17fa3dd4725d..9fae39314efc 100644 --- a/crates/viewer/re_space_view_dataframe/src/view_query/blueprint.rs +++ b/crates/viewer/re_space_view_dataframe/src/view_query/blueprint.rs @@ -134,8 +134,10 @@ impl Query { } ColumnSelector::Component(desc) => { - let blueprint_component_descriptor = - datatypes::ComponentColumnSelector::new(&desc.entity_path, desc.component); + let blueprint_component_descriptor = datatypes::ComponentColumnSelector::new( + &desc.entity_path, + desc.component_name, + ); selected_columns .component_columns @@ -258,7 +260,7 @@ impl Query { } => { selected_columns.retain(|column| match column { ColumnSelector::Component(desc) => { - desc.entity_path != entity_path || desc.component != component_name + desc.entity_path != entity_path || desc.component_name != component_name } ColumnSelector::Time(_) => true, }); diff --git a/rerun_py/src/dataframe.rs b/rerun_py/src/dataframe.rs index 00c573c9d964..feaffd6cca89 100644 --- a/rerun_py/src/dataframe.rs +++ b/rerun_py/src/dataframe.rs @@ -133,7 +133,7 @@ impl PyComponentColumnSelector { fn new(entity_path: &str, component_name: ComponentLike) -> Self { Self(ComponentColumnSelector { entity_path: entity_path.into(), - component: component_name.0, + component_name: component_name.0, join_encoding: Default::default(), }) } @@ -150,7 +150,7 @@ impl PyComponentColumnSelector { format!( "Component({}:{})", self.0.entity_path, - self.0.component.short_name() + self.0.component_name.short_name() ) } } From 8dfcad9e19dab5072564a2f4dece3f0742d7fe59 Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Fri, 11 Oct 2024 10:15:22 -0400 Subject: [PATCH 2/8] Make ComponentSelector a string --- crates/store/re_chunk_store/src/dataframe.rs | 27 +++++++++------- crates/store/re_dataframe/src/query.rs | 8 +++-- crates/store/re_types_core/src/loggable.rs | 6 ++++ rerun_py/src/dataframe.rs | 34 ++++++++++++++++---- 4 files changed, 53 insertions(+), 22 deletions(-) diff --git a/crates/store/re_chunk_store/src/dataframe.rs b/crates/store/re_chunk_store/src/dataframe.rs index 18a25c1dbe0c..6920ead9411b 100644 --- a/crates/store/re_chunk_store/src/dataframe.rs +++ b/crates/store/re_chunk_store/src/dataframe.rs @@ -278,8 +278,8 @@ impl std::fmt::Display for ComponentColumnDescriptor { impl ComponentColumnDescriptor { #[inline] - pub fn matches(&self, entity_path: &EntityPath, component_name: &ComponentName) -> bool { - &self.entity_path == entity_path && &self.component_name == component_name + pub fn matches(&self, entity_path: &EntityPath, component_name: &str) -> bool { + &self.entity_path == entity_path && self.component_name.matches(component_name) } fn metadata(&self) -> arrow2::datatypes::Metadata { @@ -422,7 +422,7 @@ pub struct ComponentColumnSelector { pub entity_path: EntityPath, /// Semantic name associated with this data. - pub component_name: ComponentName, + pub component_name: String, /// How to join the data into the `RecordBatch`. // @@ -435,7 +435,7 @@ impl From for ComponentColumnSelector { fn from(desc: ComponentColumnDescriptor) -> Self { Self { entity_path: desc.entity_path.clone(), - component_name: desc.component_name, + component_name: desc.component_name.to_string(), join_encoding: desc.join_encoding, } } @@ -447,17 +447,17 @@ impl ComponentColumnSelector { pub fn new(entity_path: EntityPath) -> Self { Self { entity_path, - component_name: C::name(), + component_name: C::name().to_string(), join_encoding: JoinEncoding::default(), } } /// Select a component based on its [`EntityPath`] and [`ComponentName`]. #[inline] - pub fn new_for_component_name(entity_path: EntityPath, component: ComponentName) -> Self { + pub fn new_for_component_name(entity_path: EntityPath, component_name: ComponentName) -> Self { Self { entity_path, - component_name: component, + component_name: component_name.to_string(), join_encoding: JoinEncoding::default(), } } @@ -474,11 +474,11 @@ impl std::fmt::Display for ComponentColumnSelector { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { let Self { entity_path, - component_name: component, + component_name, join_encoding: _, } = self; - f.write_fmt(format_args!("{entity_path}@{}", component.short_name())) + f.write_fmt(format_args!("{entity_path}:{component_name}")) } } @@ -791,13 +791,16 @@ impl ChunkStore { &self, selector: &ComponentColumnSelector, ) -> ComponentColumnDescriptor { + // TODO(jleibs): More intelligent mapping + let component_name = ComponentName::from(selector.component_name.clone()); + let ColumnMetadata { is_static, is_indicator, is_tombstone, is_semantically_empty, } = self - .lookup_column_metadata(&selector.entity_path, &selector.component_name) + .lookup_column_metadata(&selector.entity_path, &component_name) .unwrap_or(ColumnMetadata { is_static: false, is_indicator: false, @@ -806,7 +809,7 @@ impl ChunkStore { }); let datatype = self - .lookup_datatype(&selector.component_name) + .lookup_datatype(&component_name) .cloned() .unwrap_or_else(|| ArrowDatatype::Null); @@ -814,7 +817,7 @@ impl ChunkStore { entity_path: selector.entity_path.clone(), archetype_name: None, archetype_field_name: None, - component_name: selector.component_name, + component_name, store_datatype: ArrowListArray::::default_datatype(datatype.clone()), join_encoding: selector.join_encoding, is_static, diff --git a/crates/store/re_dataframe/src/query.rs b/crates/store/re_dataframe/src/query.rs index 75e0a0a4ca5f..9f7fa5e48ef9 100644 --- a/crates/store/re_dataframe/src/query.rs +++ b/crates/store/re_dataframe/src/query.rs @@ -364,7 +364,7 @@ impl QueryHandle<'_> { }) .find(|(_idx, view_descr)| { view_descr.entity_path == *selected_entity_path - && view_descr.component_name == *selected_component_name + && view_descr.component_name.matches(selected_component_name) }) .map_or_else( || { @@ -374,7 +374,9 @@ impl QueryHandle<'_> { entity_path: selected_entity_path.clone(), archetype_name: None, archetype_field_name: None, - component_name: *selected_component_name, + component_name: ComponentName::from( + selected_component_name.clone(), + ), store_datatype: arrow2::datatypes::DataType::Null, join_encoding: JoinEncoding::default(), is_static: false, @@ -418,7 +420,7 @@ impl QueryHandle<'_> { if let Some(pov) = self.query.filtered_point_of_view.as_ref() { if pov.entity_path == column.entity_path - && pov.component_name == column.component_name + && column.component_name.matches(&pov.component_name) { view_pov_chunks_idx = Some(idx); } diff --git a/crates/store/re_types_core/src/loggable.rs b/crates/store/re_types_core/src/loggable.rs index cacb8400960a..b444fde82553 100644 --- a/crates/store/re_types_core/src/loggable.rs +++ b/crates/store/re_types_core/src/loggable.rs @@ -184,6 +184,12 @@ impl ComponentName { None // A user component } } + + pub fn matches(&self, other: &str) -> bool { + self.0.as_str() == other + || self.full_name().to_lowercase() == other.to_lowercase() + || self.short_name().to_lowercase() == other.to_lowercase() + } } // --- diff --git a/rerun_py/src/dataframe.rs b/rerun_py/src/dataframe.rs index feaffd6cca89..9109dc77adc7 100644 --- a/rerun_py/src/dataframe.rs +++ b/rerun_py/src/dataframe.rs @@ -149,8 +149,7 @@ impl PyComponentColumnSelector { fn __repr__(&self) -> String { format!( "Component({}:{})", - self.0.entity_path, - self.0.component_name.short_name() + self.0.entity_path, self.0.component_name ) } } @@ -298,19 +297,19 @@ impl<'py> IndexValuesLike<'py> { } } -struct ComponentLike(re_sdk::ComponentName); +struct ComponentLike(String); impl FromPyObject<'_> for ComponentLike { fn extract_bound(component: &Bound<'_, PyAny>) -> PyResult { if let Ok(component_str) = component.extract::() { - Ok(Self(component_str.into())) + Ok(Self(component_str)) } else if let Ok(component_str) = component .getattr("_BATCH_TYPE") .and_then(|batch_type| batch_type.getattr("_ARROW_TYPE")) .and_then(|arrow_type| arrow_type.getattr("_TYPE_NAME")) .and_then(|type_name| type_name.extract::()) { - Ok(Self(component_str.into())) + Ok(Self(component_str)) } else { return Err(PyTypeError::new_err( "ComponentLike input must be a string or Component class.", @@ -597,6 +596,18 @@ impl PyRecording { } } + fn find_best_component(&self, entity_path: &EntityPath, component_name: &str) -> ComponentName { + let selector = ComponentColumnSelector { + entity_path: entity_path.clone(), + component_name: component_name.into(), + join_encoding: Default::default(), + }; + + self.store + .resolve_component_selector(&selector) + .component_name + } + /// Convert a `ViewContentsLike` into a `ViewContentsSelector`. /// /// ```pytholn @@ -631,6 +642,7 @@ impl PyRecording { // `Union[ComponentLike, Sequence[ComponentLike]]]` let mut contents = ViewContentsSelector::default(); + for (key, value) in dict { let key = key.extract::().map_err(|_err| { PyTypeError::new_err( @@ -644,7 +656,7 @@ impl PyRecording { )) })?; - let components: BTreeSet = if let Ok(component) = + let component_strs: BTreeSet = if let Ok(component) = value.extract::() { std::iter::once(component.0).collect() @@ -659,7 +671,15 @@ impl PyRecording { contents.append( &mut engine .iter_entity_paths(&path_filter) - .map(|p| (p, Some(components.clone()))) + .map(|entity_path| { + let components = component_strs + .iter() + .map(|component_name| { + self.find_best_component(&entity_path, component_name) + }) + .collect(); + (entity_path, Some(components)) + }) .collect(), ); } From 14e3f424d1ad1a12c51bb5bb7ff46efec921419c Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Fri, 11 Oct 2024 10:22:58 -0400 Subject: [PATCH 3/8] Cleanup API usage --- crates/store/re_dataframe/src/query.rs | 20 +++++++++---------- .../component_column_selector_ext.rs | 3 +-- .../component_column_selector_ext.rs | 5 ++--- .../src/view_query/blueprint.rs | 15 +++++++------- 4 files changed, 21 insertions(+), 22 deletions(-) diff --git a/crates/store/re_dataframe/src/query.rs b/crates/store/re_dataframe/src/query.rs index 9f7fa5e48ef9..96dc02798b79 100644 --- a/crates/store/re_dataframe/src/query.rs +++ b/crates/store/re_dataframe/src/query.rs @@ -1439,7 +1439,7 @@ mod tests { let mut query = QueryExpression::new(timeline); query.filtered_point_of_view = Some(ComponentColumnSelector { entity_path: "no/such/entity".into(), - component_name: MyPoint::name(), + component_name: MyPoint::name().to_string(), join_encoding: Default::default(), }); eprintln!("{query:#?}:"); @@ -1493,7 +1493,7 @@ mod tests { let mut query = QueryExpression::new(timeline); query.filtered_point_of_view = Some(ComponentColumnSelector { entity_path: entity_path.clone(), - component_name: MyPoint::name(), + component_name: MyPoint::name().to_string(), join_encoding: Default::default(), }); eprintln!("{query:#?}:"); @@ -1530,7 +1530,7 @@ mod tests { let mut query = QueryExpression::new(timeline); query.filtered_point_of_view = Some(ComponentColumnSelector { entity_path: entity_path.clone(), - component_name: MyColor::name(), + component_name: MyColor::name().to_string(), join_encoding: Default::default(), }); eprintln!("{query:#?}:"); @@ -1741,17 +1741,17 @@ mod tests { query.selection = Some(vec![ ColumnSelector::Component(ComponentColumnSelector { entity_path: entity_path.clone(), - component_name: MyColor::name(), + component_name: MyColor::name().to_string(), join_encoding: Default::default(), }), ColumnSelector::Component(ComponentColumnSelector { entity_path: entity_path.clone(), - component_name: MyColor::name(), + component_name: MyColor::name().to_string(), join_encoding: Default::default(), }), ColumnSelector::Component(ComponentColumnSelector { entity_path: "non_existing_entity".into(), - component_name: MyColor::name(), + component_name: MyColor::name().to_string(), join_encoding: Default::default(), }), ColumnSelector::Component(ComponentColumnSelector { @@ -1830,17 +1830,17 @@ mod tests { // ColumnSelector::Component(ComponentColumnSelector { entity_path: entity_path.clone(), - component_name: MyPoint::name(), + component_name: MyPoint::name().to_string(), join_encoding: Default::default(), }), ColumnSelector::Component(ComponentColumnSelector { entity_path: entity_path.clone(), - component_name: MyColor::name(), + component_name: MyColor::name().to_string(), join_encoding: Default::default(), }), ColumnSelector::Component(ComponentColumnSelector { entity_path: entity_path.clone(), - component_name: MyLabel::name(), + component_name: MyLabel::name().to_string(), join_encoding: Default::default(), }), ]); @@ -2022,7 +2022,7 @@ mod tests { let mut query = QueryExpression::new(timeline); query.filtered_point_of_view = Some(ComponentColumnSelector { entity_path: entity_path.clone(), - component_name: MyPoint::name(), + component_name: MyPoint::name().to_string(), join_encoding: Default::default(), }); eprintln!("{query:#?}:"); diff --git a/crates/store/re_types/src/blueprint/components/component_column_selector_ext.rs b/crates/store/re_types/src/blueprint/components/component_column_selector_ext.rs index 18957e74b779..59f52df4a530 100644 --- a/crates/store/re_types/src/blueprint/components/component_column_selector_ext.rs +++ b/crates/store/re_types/src/blueprint/components/component_column_selector_ext.rs @@ -1,9 +1,8 @@ use re_log_types::EntityPath; -use re_types_core::ComponentName; impl super::ComponentColumnSelector { /// Create a [`Self`] from an [`EntityPath`] and a [`ComponentName`]. - pub fn new(entity_path: &EntityPath, component_name: ComponentName) -> Self { + pub fn new(entity_path: &EntityPath, component_name: &str) -> Self { crate::blueprint::datatypes::ComponentColumnSelector::new(entity_path, component_name) .into() } diff --git a/crates/store/re_types/src/blueprint/datatypes/component_column_selector_ext.rs b/crates/store/re_types/src/blueprint/datatypes/component_column_selector_ext.rs index ca74f5fc2a04..3481bae9f127 100644 --- a/crates/store/re_types/src/blueprint/datatypes/component_column_selector_ext.rs +++ b/crates/store/re_types/src/blueprint/datatypes/component_column_selector_ext.rs @@ -1,12 +1,11 @@ use re_log_types::EntityPath; -use re_types_core::ComponentName; impl super::ComponentColumnSelector { /// Create a [`Self`] from an [`EntityPath`] and a [`ComponentName`]. - pub fn new(entity_path: &EntityPath, component_name: ComponentName) -> Self { + pub fn new(entity_path: &EntityPath, component_name: &str) -> Self { Self { entity_path: entity_path.into(), - component: component_name.as_str().into(), + component: component_name.into(), } } } diff --git a/crates/viewer/re_space_view_dataframe/src/view_query/blueprint.rs b/crates/viewer/re_space_view_dataframe/src/view_query/blueprint.rs index 9fae39314efc..36dabf2911bb 100644 --- a/crates/viewer/re_space_view_dataframe/src/view_query/blueprint.rs +++ b/crates/viewer/re_space_view_dataframe/src/view_query/blueprint.rs @@ -133,15 +133,15 @@ impl Query { .push(desc.timeline.as_str().into()); } - ColumnSelector::Component(desc) => { - let blueprint_component_descriptor = datatypes::ComponentColumnSelector::new( - &desc.entity_path, - desc.component_name, + ColumnSelector::Component(selector) => { + let blueprint_component_selector = datatypes::ComponentColumnSelector::new( + &selector.entity_path, + &selector.component_name, ); selected_columns .component_columns - .push(blueprint_component_descriptor); + .push(blueprint_component_selector); } } } @@ -259,8 +259,9 @@ impl Query { component_name, } => { selected_columns.retain(|column| match column { - ColumnSelector::Component(desc) => { - desc.entity_path != entity_path || desc.component_name != component_name + ColumnSelector::Component(selector) => { + selector.entity_path != entity_path + || !component_name.matches(&selector.component_name) } ColumnSelector::Time(_) => true, }); From 539cc24c0d00e7f20670a0d32f7ba1c0ad622513 Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Fri, 11 Oct 2024 10:40:35 -0400 Subject: [PATCH 4/8] Flexible matching of selector -> descriptor --- crates/store/re_chunk_store/src/dataframe.rs | 24 ++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/crates/store/re_chunk_store/src/dataframe.rs b/crates/store/re_chunk_store/src/dataframe.rs index 6920ead9411b..601f0dbacdf1 100644 --- a/crates/store/re_chunk_store/src/dataframe.rs +++ b/crates/store/re_chunk_store/src/dataframe.rs @@ -791,8 +791,28 @@ impl ChunkStore { &self, selector: &ComponentColumnSelector, ) -> ComponentColumnDescriptor { - // TODO(jleibs): More intelligent mapping - let component_name = ComponentName::from(selector.component_name.clone()); + // Happy path if this string is a valid component + let direct_component = ComponentName::from(selector.component_name.clone()); + + let component_name = if self.all_components().contains(&direct_component) { + direct_component + } else { + self.all_components_for_entity(&selector.entity_path) + // First just check on the entity since this is the most likely place to find it. + .and_then(|components| { + components + .into_iter() + .find(|component_name| component_name.matches(&selector.component_name)) + }) + // Fall back on matching any component in the store + .or_else(|| { + self.all_components() + .into_iter() + .find(|component_name| component_name.matches(&selector.component_name)) + }) + // Finally fall back on the direct component name + .unwrap_or(direct_component) + }; let ColumnMetadata { is_static, From 0d822401f49b50ae42912a34fed81c4549ea5346 Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Fri, 11 Oct 2024 10:40:44 -0400 Subject: [PATCH 5/8] Unit-test --- rerun_py/tests/unit/test_dataframe.py | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/rerun_py/tests/unit/test_dataframe.py b/rerun_py/tests/unit/test_dataframe.py index 5532ca283fbc..072542d239c9 100644 --- a/rerun_py/tests/unit/test_dataframe.py +++ b/rerun_py/tests/unit/test_dataframe.py @@ -108,19 +108,22 @@ def test_full_view(self) -> None: def test_select_columns(self) -> None: view = self.recording.view(index="my_index", contents="points") index_col = rr.dataframe.IndexColumnSelector("my_index") - pos = rr.dataframe.ComponentColumnSelector("points", rr.components.Position3D) - batches = view.select(index_col, pos) + selectors = [rr.components.Position3D, "rerun.components.Position3D", "Position3D", "position3D"] + for selector in selectors: + pos = rr.dataframe.ComponentColumnSelector("points", selector) - table = pa.Table.from_batches(batches, batches.schema) - # points - assert table.num_columns == 2 - assert table.num_rows == 2 + batches = view.select(index_col, pos) - assert table.column("my_index")[0].equals(self.expected_index0[0]) - assert table.column("my_index")[1].equals(self.expected_index1[0]) - assert table.column("/points:Position3D")[0].values.equals(self.expected_pos0) - assert table.column("/points:Position3D")[1].values.equals(self.expected_pos1) + table = pa.Table.from_batches(batches, batches.schema) + # points + assert table.num_columns == 2 + assert table.num_rows == 2 + + assert table.column("my_index")[0].equals(self.expected_index0[0]) + assert table.column("my_index")[1].equals(self.expected_index1[0]) + assert table.column("/points:Position3D")[0].values.equals(self.expected_pos0) + assert table.column("/points:Position3D")[1].values.equals(self.expected_pos1) def test_index_values(self) -> None: view = self.recording.view(index="my_index", contents="points") @@ -248,6 +251,8 @@ def test_view_syntax(self) -> None: {"points": [rr.components.Position3D]}, {"points": "rerun.components.Position3D"}, {"points/**": "rerun.components.Position3D"}, + {"points/**": "Position3D"}, + {"points/**": "position3D"}, ] for expr in good_content_expressions: From fab4a9d637b96dad15b6ff4af443de34700bc187 Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Fri, 11 Oct 2024 10:45:48 -0400 Subject: [PATCH 6/8] Update some docstrings --- crates/store/re_chunk_store/src/dataframe.rs | 3 +++ crates/store/re_types_core/src/loggable.rs | 3 +++ 2 files changed, 6 insertions(+) diff --git a/crates/store/re_chunk_store/src/dataframe.rs b/crates/store/re_chunk_store/src/dataframe.rs index 601f0dbacdf1..1c1671c94f37 100644 --- a/crates/store/re_chunk_store/src/dataframe.rs +++ b/crates/store/re_chunk_store/src/dataframe.rs @@ -422,6 +422,9 @@ pub struct ComponentColumnSelector { pub entity_path: EntityPath, /// Semantic name associated with this data. + /// + /// This string will be flexibly matched against the available component names. + /// Valid matches are case invariant matches of either the full name or the short name. pub component_name: String, /// How to join the data into the `RecordBatch`. diff --git a/crates/store/re_types_core/src/loggable.rs b/crates/store/re_types_core/src/loggable.rs index b444fde82553..9889be90aeef 100644 --- a/crates/store/re_types_core/src/loggable.rs +++ b/crates/store/re_types_core/src/loggable.rs @@ -185,6 +185,9 @@ impl ComponentName { } } + /// Determine if component matches a string + /// + /// Valid matches are case invariant matches of either the full name or the short name. pub fn matches(&self, other: &str) -> bool { self.0.as_str() == other || self.full_name().to_lowercase() == other.to_lowercase() From 281a034db6bce00f361ed6f874b008fcaead6861 Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Fri, 11 Oct 2024 11:28:53 -0400 Subject: [PATCH 7/8] Comment/issue about string-interning --- crates/store/re_chunk_store/src/dataframe.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/store/re_chunk_store/src/dataframe.rs b/crates/store/re_chunk_store/src/dataframe.rs index 1c1671c94f37..42c6a33e0082 100644 --- a/crates/store/re_chunk_store/src/dataframe.rs +++ b/crates/store/re_chunk_store/src/dataframe.rs @@ -795,6 +795,8 @@ impl ChunkStore { selector: &ComponentColumnSelector, ) -> ComponentColumnDescriptor { // Happy path if this string is a valid component + // TODO(#7699) This currently interns every string ever queried which could be wasteful, especially + // in long-running servers. In practice this probably doesn't matter. let direct_component = ComponentName::from(selector.component_name.clone()); let component_name = if self.all_components().contains(&direct_component) { From 76c6255f883c28f651a35acd057434d9dc2710a7 Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Fri, 11 Oct 2024 11:45:22 -0400 Subject: [PATCH 8/8] Fix docstring --- .../src/blueprint/components/component_column_selector_ext.rs | 2 +- .../src/blueprint/datatypes/component_column_selector_ext.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/store/re_types/src/blueprint/components/component_column_selector_ext.rs b/crates/store/re_types/src/blueprint/components/component_column_selector_ext.rs index 59f52df4a530..c6a88aca9307 100644 --- a/crates/store/re_types/src/blueprint/components/component_column_selector_ext.rs +++ b/crates/store/re_types/src/blueprint/components/component_column_selector_ext.rs @@ -1,7 +1,7 @@ use re_log_types::EntityPath; impl super::ComponentColumnSelector { - /// Create a [`Self`] from an [`EntityPath`] and a [`ComponentName`]. + /// Create a [`Self`] from an [`EntityPath`] and a [`re_types_core::ComponentName`] expressed as string. pub fn new(entity_path: &EntityPath, component_name: &str) -> Self { crate::blueprint::datatypes::ComponentColumnSelector::new(entity_path, component_name) .into() diff --git a/crates/store/re_types/src/blueprint/datatypes/component_column_selector_ext.rs b/crates/store/re_types/src/blueprint/datatypes/component_column_selector_ext.rs index 3481bae9f127..0f63362a38f1 100644 --- a/crates/store/re_types/src/blueprint/datatypes/component_column_selector_ext.rs +++ b/crates/store/re_types/src/blueprint/datatypes/component_column_selector_ext.rs @@ -1,7 +1,7 @@ use re_log_types::EntityPath; impl super::ComponentColumnSelector { - /// Create a [`Self`] from an [`EntityPath`] and a [`ComponentName`]. + /// Create a [`Self`] from an [`EntityPath`] and a [`re_types_core::ComponentName`] expressed as string. pub fn new(entity_path: &EntityPath, component_name: &str) -> Self { Self { entity_path: entity_path.into(),