Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow component selectors to be specified as strings with flexible matching #7695

Merged
merged 9 commits into from
Oct 11, 2024
Merged

Allow component selectors to be specified as strings with flexible matching #7695

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
e94873d
Rename component -> component_name
jleibs Oct 11, 2024
8dfcad9
Make ComponentSelector a string
jleibs Oct 11, 2024
14e3f42
Cleanup API usage
jleibs Oct 11, 2024
539cc24
Flexible matching of selector -> descriptor
jleibs Oct 11, 2024
0d82240
Unit-test
jleibs Oct 11, 2024
fab4a9d
Update some docstrings
jleibs Oct 11, 2024
95e98bc
Merge branch 'main' into jleibs/dataframe_component_promotion
teh-cmc Oct 11, 2024
281a034
Comment/issue about string-interning
jleibs Oct 11, 2024
76c6255
Fix docstring
jleibs Oct 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 38 additions & 14 deletions crates/store/re_chunk_store/src/dataframe.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -422,9 +422,10 @@ pub struct ComponentColumnSelector {
pub entity_path: EntityPath,

/// Semantic name associated with this data.
//
// TODO(cmc): this should be `component_name`.
pub component: ComponentName,
///
/// 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`.
//
Expand All @@ -437,7 +438,7 @@ impl From<ComponentColumnDescriptor> for ComponentColumnSelector {
fn from(desc: ComponentColumnDescriptor) -> Self {
Self {
entity_path: desc.entity_path.clone(),
component: desc.component_name,
component_name: desc.component_name.to_string(),
join_encoding: desc.join_encoding,
}
}
Expand All @@ -449,17 +450,17 @@ impl ComponentColumnSelector {
pub fn new<C: re_types_core::Component>(entity_path: EntityPath) -> Self {
Self {
entity_path,
component: 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,
component_name: component_name.to_string(),
join_encoding: JoinEncoding::default(),
}
}
Expand All @@ -476,11 +477,11 @@ impl std::fmt::Display for ComponentColumnSelector {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let Self {
entity_path,
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}"))
}
}

Expand Down Expand Up @@ -793,13 +794,36 @@ impl ChunkStore {
&self,
selector: &ComponentColumnSelector,
) -> ComponentColumnDescriptor {
// Happy path if this string is a valid component
let direct_component = ComponentName::from(selector.component_name.clone());
Copy link
Member

Choose a reason for hiding this comment

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

Hmmm, I guess that means we're going to intern a bunch of random strings indefinitely.

This is hardly a problem in practice, but might deserve a NOTE.

Copy link
Member Author

Choose a reason for hiding this comment

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

Issue: #7699


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,
is_indicator,
is_tombstone,
is_semantically_empty,
} = self
.lookup_column_metadata(&selector.entity_path, &selector.component)
.lookup_column_metadata(&selector.entity_path, &component_name)
.unwrap_or(ColumnMetadata {
is_static: false,
is_indicator: false,
Expand All @@ -808,15 +832,15 @@ impl ChunkStore {
});

let datatype = self
.lookup_datatype(&selector.component)
.lookup_datatype(&component_name)
.cloned()
.unwrap_or_else(|| ArrowDatatype::Null);

ComponentColumnDescriptor {
entity_path: selector.entity_path.clone(),
archetype_name: None,
archetype_field_name: None,
component_name: selector.component,
component_name,
store_datatype: ArrowListArray::<i32>::default_datatype(datatype.clone()),
join_encoding: selector.join_encoding,
is_static,
Expand Down
34 changes: 18 additions & 16 deletions crates/store/re_dataframe/src/query.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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(
|| {
Expand All @@ -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,
Expand Down Expand Up @@ -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 == column.component_name
&& column.component_name.matches(&pov.component_name)
{
view_pov_chunks_idx = Some(idx);
}
Expand Down Expand Up @@ -1437,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: MyPoint::name(),
component_name: MyPoint::name().to_string(),
join_encoding: Default::default(),
});
eprintln!("{query:#?}:");
Expand All @@ -1464,7 +1466,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:#?}:");
Expand All @@ -1491,7 +1493,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().to_string(),
join_encoding: Default::default(),
});
eprintln!("{query:#?}:");
Expand Down Expand Up @@ -1528,7 +1530,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().to_string(),
join_encoding: Default::default(),
});
eprintln!("{query:#?}:");
Expand Down Expand Up @@ -1739,22 +1741,22 @@ mod tests {
query.selection = Some(vec![
ColumnSelector::Component(ComponentColumnSelector {
entity_path: entity_path.clone(),
component: MyColor::name(),
component_name: MyColor::name().to_string(),
join_encoding: Default::default(),
}),
ColumnSelector::Component(ComponentColumnSelector {
entity_path: entity_path.clone(),
component: MyColor::name(),
component_name: MyColor::name().to_string(),
join_encoding: Default::default(),
}),
ColumnSelector::Component(ComponentColumnSelector {
entity_path: "non_existing_entity".into(),
component: MyColor::name(),
component_name: MyColor::name().to_string(),
join_encoding: Default::default(),
}),
ColumnSelector::Component(ComponentColumnSelector {
entity_path: entity_path.clone(),
component: "AComponentColumnThatDoesntExist".into(),
component_name: "AComponentColumnThatDoesntExist".into(),
join_encoding: Default::default(),
}),
]);
Expand Down Expand Up @@ -1828,17 +1830,17 @@ mod tests {
//
ColumnSelector::Component(ComponentColumnSelector {
entity_path: entity_path.clone(),
component: MyPoint::name(),
component_name: MyPoint::name().to_string(),
join_encoding: Default::default(),
}),
ColumnSelector::Component(ComponentColumnSelector {
entity_path: entity_path.clone(),
component: MyColor::name(),
component_name: MyColor::name().to_string(),
join_encoding: Default::default(),
}),
ColumnSelector::Component(ComponentColumnSelector {
entity_path: entity_path.clone(),
component: MyLabel::name(),
component_name: MyLabel::name().to_string(),
join_encoding: Default::default(),
}),
]);
Expand Down Expand Up @@ -2020,7 +2022,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().to_string(),
join_encoding: Default::default(),
});
eprintln!("{query:#?}:");
Expand Down
3 changes: 1 addition & 2 deletions crates/store/re_types/src/blueprint/components/component_column_selector_ext.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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()
}
Expand Down
5 changes: 2 additions & 3 deletions crates/store/re_types/src/blueprint/datatypes/component_column_selector_ext.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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(),
}
}
}
9 changes: 9 additions & 0 deletions crates/store/re_types_core/src/loggable.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,15 @@ impl ComponentName {
None // A user component
}
}

/// 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()
|| self.short_name().to_lowercase() == other.to_lowercase()
}
}

// ---
Expand Down
15 changes: 9 additions & 6 deletions crates/viewer/re_space_view_dataframe/src/view_query/blueprint.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,13 +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);
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);
}
}
}
Expand Down Expand Up @@ -257,8 +259,9 @@ impl Query {
component_name,
} => {
selected_columns.retain(|column| match column {
ColumnSelector::Component(desc) => {
desc.entity_path != entity_path || desc.component != component_name
ColumnSelector::Component(selector) => {
selector.entity_path != entity_path
|| !component_name.matches(&selector.component_name)
}
ColumnSelector::Time(_) => true,
});
Expand Down
Loading
Loading