Skip to content

Commit

Permalink
Allow component selectors to be specified as strings with flexible ma…
Browse files Browse the repository at this point in the history
…tching (#7695)

### What
- Resolves: #7629
- We also do this promotion for the ComponentLike in the view contents
expression on the python side

This means you can now do:
```
view = self.recording.view(index="my_index", contents={"points": "position3D"}).select("position3d")
```

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
* Using examples from latest `main` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/7695?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/7695?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG
* [x] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!
* [x] If have noted any breaking changes to the log API in
`CHANGELOG.md` and the migration guide

- [PR Build Summary](https://build.rerun.io/pr/7695)
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

To run all checks from `main`, comment on the PR with `@rerun-bot
full-check`.

---------

Co-authored-by: Clement Rey <[email protected]>
  • Loading branch information
jleibs and teh-cmc authored Oct 11, 2024
1 parent bf26fb6 commit 0045de5
Show file tree
Hide file tree
Showing 8 changed files with 124 additions and 61 deletions.
54 changes: 40 additions & 14 deletions crates/store/re_chunk_store/src/dataframe.rs
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,38 @@ impl ChunkStore {
&self,
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) {
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 +834,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
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
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 {
/// 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()
}
Expand Down
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 {
/// 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(),
component: component_name.as_str().into(),
component: component_name.into(),
}
}
}
9 changes: 9 additions & 0 deletions crates/store/re_types_core/src/loggable.rs
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
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

0 comments on commit 0045de5

Please sign in to comment.