Skip to content

Commit

Permalink
Disable tagging entirely for 0.21 (#8499)
Browse files Browse the repository at this point in the history
There are too many pitfalls that users can fall into because the current
logging APIs are not designed with tagging in mind, and we have not yet
communicated at all about any of this.

This has to be done below the chunk level, directly at the descriptor
level.
  • Loading branch information
teh-cmc authored Dec 17, 2024
1 parent db18508 commit 952e397
Show file tree
Hide file tree
Showing 10 changed files with 98 additions and 40 deletions.
15 changes: 3 additions & 12 deletions crates/store/re_chunk/src/batcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -735,10 +735,7 @@ impl PendingRow {
for (component_desc, array) in components {
let list_array = crate::util::arrays_to_list_array_opt(&[Some(&*array as _)]);
if let Some(list_array) = list_array {
per_name
.entry(component_desc.component_name)
.or_default()
.insert(component_desc, list_array);
per_name.insert_descriptor(component_desc, list_array);
}
}

Expand Down Expand Up @@ -874,10 +871,7 @@ impl PendingRow {
let list_array =
crate::util::arrays_to_list_array_opt(&arrays);
if let Some(list_array) = list_array {
per_name
.entry(component_desc.component_name)
.or_default()
.insert(component_desc, list_array);
per_name.insert_descriptor(component_desc, list_array);
}
}
per_name
Expand Down Expand Up @@ -922,10 +916,7 @@ impl PendingRow {
for (component_desc, arrays) in components {
let list_array = crate::util::arrays_to_list_array_opt(&arrays);
if let Some(list_array) = list_array {
per_name
.entry(component_desc.component_name)
.or_default()
.insert(component_desc, list_array);
per_name.insert_descriptor(component_desc, list_array);
}
}
per_name
Expand Down
10 changes: 5 additions & 5 deletions crates/store/re_chunk/src/chunk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,13 @@ impl ChunkComponents {
&mut self,
component_desc: ComponentDescriptor,
list_array: Arrow2ListArray<i32>,
) {
) -> Option<Arrow2ListArray<i32>> {
// TODO(cmc): revert me
let component_desc = component_desc.untagged();
self.0
.entry(component_desc.component_name)
.or_default()
.insert(component_desc, list_array);
.insert(component_desc, list_array)
}

/// Returns all list arrays for the given component name.
Expand Down Expand Up @@ -144,9 +146,7 @@ impl FromIterator<(ComponentDescriptor, Arrow2ListArray<i32>)> for ChunkComponen
let mut this = Self::default();
{
for (component_desc, list_array) in iter {
this.entry(component_desc.component_name)
.or_default()
.insert(component_desc, list_array);
this.insert_descriptor(component_desc, list_array);
}
}
this
Expand Down
5 changes: 1 addition & 4 deletions crates/store/re_chunk/src/merge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,10 +171,7 @@ impl Chunk {
let components = {
let mut per_name = ChunkComponents::default();
for (component_desc, list_array) in components {
per_name
.entry(component_desc.component_name)
.or_default()
.insert(component_desc.clone(), list_array);
per_name.insert_descriptor(component_desc.clone(), list_array);
}
per_name
};
Expand Down
4 changes: 1 addition & 3 deletions crates/store/re_chunk/src/transport.rs
Original file line number Diff line number Diff line change
Expand Up @@ -650,9 +650,7 @@ impl Chunk {
let component_desc = TransportChunk::component_descriptor_from_field(field);

if components
.entry(component_desc.component_name)
.or_default()
.insert(component_desc, column.clone() /* refcount */)
.insert_descriptor(component_desc, column.clone())
.is_some()
{
return Err(ChunkError::Malformed {
Expand Down
2 changes: 2 additions & 0 deletions crates/store/re_chunk_store/tests/gc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,8 @@ fn protected_time_ranges() -> anyhow::Result<()> {
}
}

eprintln!("{store}");

let (events, _) = store.gc(&protect_time_range(ResolvedTimeRange::new(1, 4)));
assert_eq!(events.len(), 0);

Expand Down
4 changes: 4 additions & 0 deletions crates/store/re_types_core/src/component_descriptor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,10 @@ impl ComponentDescriptor {
}
}

pub fn untagged(self) -> Self {
Self::new(self.component_name)
}

/// Unconditionally sets [`Self::archetype_name`] to the given one.
#[inline]
pub fn with_archetype_name(mut self, archetype_name: ArchetypeName) -> Self {
Expand Down
26 changes: 22 additions & 4 deletions docs/snippets/all/descriptors/descr_builtin_archetype.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,20 +56,38 @@ fn check_tags(rec: &rerun::RecordingStream) {
.collect::<Vec<_>>();
descriptors.sort();

// TODO(cmc): revert me
// let expected = vec![
// ComponentDescriptor {
// archetype_name: None,
// archetype_field_name: None,
// component_name: "rerun.components.Points3DIndicator".into(),
// },
// ComponentDescriptor {
// archetype_name: Some("rerun.archetypes.Points3D".into()),
// archetype_field_name: Some("positions".into()),
// component_name: "rerun.components.Position3D".into(),
// },
// ComponentDescriptor {
// archetype_name: Some("rerun.archetypes.Points3D".into()),
// archetype_field_name: Some("radii".into()),
// component_name: "rerun.components.Radius".into(),
// },
// ];
let expected = vec![
ComponentDescriptor {
archetype_name: None,
archetype_field_name: None,
component_name: "rerun.components.Points3DIndicator".into(),
},
ComponentDescriptor {
archetype_name: Some("rerun.archetypes.Points3D".into()),
archetype_field_name: Some("positions".into()),
archetype_name: None,
archetype_field_name: None,
component_name: "rerun.components.Position3D".into(),
},
ComponentDescriptor {
archetype_name: Some("rerun.archetypes.Points3D".into()),
archetype_field_name: Some("radii".into()),
archetype_name: None,
archetype_field_name: None,
component_name: "rerun.components.Radius".into(),
},
];
Expand Down
30 changes: 24 additions & 6 deletions docs/snippets/all/descriptors/descr_custom_archetype.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,20 +104,38 @@ fn check_tags(rec: &rerun::RecordingStream) {
.collect::<Vec<_>>();
descriptors.sort();

// TODO(cmc): revert me
// let expected = vec![
// ComponentDescriptor {
// archetype_name: None,
// archetype_field_name: None,
// component_name: "user.CustomPoints3DIndicator".into(),
// },
// ComponentDescriptor {
// archetype_name: Some("user.CustomPoints3D".into()),
// archetype_field_name: Some("colors".into()),
// component_name: "rerun.components.Color".into(),
// },
// ComponentDescriptor {
// archetype_name: Some("user.CustomPoints3D".into()),
// archetype_field_name: Some("custom_positions".into()),
// component_name: "user.CustomPosition3D".into(),
// },
// ];
let expected = vec![
ComponentDescriptor {
archetype_name: None,
archetype_field_name: None,
component_name: "user.CustomPoints3DIndicator".into(),
component_name: "rerun.components.Color".into(),
},
ComponentDescriptor {
archetype_name: Some("user.CustomPoints3D".into()),
archetype_field_name: Some("colors".into()),
component_name: "rerun.components.Color".into(),
archetype_name: None,
archetype_field_name: None,
component_name: "user.CustomPoints3DIndicator".into(),
},
ComponentDescriptor {
archetype_name: Some("user.CustomPoints3D".into()),
archetype_field_name: Some("custom_positions".into()),
archetype_name: None,
archetype_field_name: None,
component_name: "user.CustomPosition3D".into(),
},
];
Expand Down
12 changes: 10 additions & 2 deletions docs/snippets/all/descriptors/descr_custom_component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,18 @@ fn check_tags(rec: &rerun::RecordingStream) {
.collect::<Vec<_>>();
descriptors.sort();

// TODO(cmc): revert me
// let expected = vec![
// ComponentDescriptor {
// archetype_name: Some("user.CustomArchetype".into()),
// archetype_field_name: Some("custom_positions".into()),
// component_name: "user.CustomPosition3D".into(),
// }, //
// ];
let expected = vec![
ComponentDescriptor {
archetype_name: Some("user.CustomArchetype".into()),
archetype_field_name: Some("custom_positions".into()),
archetype_name: None,
archetype_field_name: None,
component_name: "user.CustomPosition3D".into(),
}, //
];
Expand Down
30 changes: 26 additions & 4 deletions rerun_py/tests/unit/test_dataframe.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,14 +104,36 @@ def test_schema_recording(self) -> None:
# Color, Points3DIndicator, Position3D, Radius, Text, TextIndicator
assert len(schema.component_columns()) == 6

# TODO(cmc): revert me
# assert schema.index_columns()[0].name == "log_tick"
# assert schema.index_columns()[1].name == "log_time"
# assert schema.index_columns()[2].name == "my_index"
# assert schema.component_columns()[0].entity_path == "/points"
# assert schema.component_columns()[0].component_name == "rerun.components.Points3DIndicator"
# assert schema.component_columns()[0].is_static is False
# assert schema.component_columns()[1].entity_path == "/points"
# assert schema.component_columns()[1].component_name == "rerun.components.Color"
# assert schema.component_columns()[1].is_static is False
# assert schema.component_columns()[2].entity_path == "/points"
# assert schema.component_columns()[2].component_name == "rerun.components.Position3D"
# assert schema.component_columns()[2].is_static is False
# assert schema.component_columns()[3].entity_path == "/points"
# assert schema.component_columns()[3].component_name == "rerun.components.Radius"
# assert schema.component_columns()[3].is_static is False
# assert schema.component_columns()[4].entity_path == "/static_text"
# assert schema.component_columns()[4].component_name == "rerun.components.TextLogIndicator"
# assert schema.component_columns()[4].is_static is True
# assert schema.component_columns()[5].entity_path == "/static_text"
# assert schema.component_columns()[5].component_name == "rerun.components.Text"
# assert schema.component_columns()[5].is_static is True
assert schema.index_columns()[0].name == "log_tick"
assert schema.index_columns()[1].name == "log_time"
assert schema.index_columns()[2].name == "my_index"
assert schema.component_columns()[0].entity_path == "/points"
assert schema.component_columns()[0].component_name == "rerun.components.Points3DIndicator"
assert schema.component_columns()[0].component_name == "rerun.components.Color"
assert schema.component_columns()[0].is_static is False
assert schema.component_columns()[1].entity_path == "/points"
assert schema.component_columns()[1].component_name == "rerun.components.Color"
assert schema.component_columns()[1].component_name == "rerun.components.Points3DIndicator"
assert schema.component_columns()[1].is_static is False
assert schema.component_columns()[2].entity_path == "/points"
assert schema.component_columns()[2].component_name == "rerun.components.Position3D"
Expand All @@ -120,10 +142,10 @@ def test_schema_recording(self) -> None:
assert schema.component_columns()[3].component_name == "rerun.components.Radius"
assert schema.component_columns()[3].is_static is False
assert schema.component_columns()[4].entity_path == "/static_text"
assert schema.component_columns()[4].component_name == "rerun.components.TextLogIndicator"
assert schema.component_columns()[4].component_name == "rerun.components.Text"
assert schema.component_columns()[4].is_static is True
assert schema.component_columns()[5].entity_path == "/static_text"
assert schema.component_columns()[5].component_name == "rerun.components.Text"
assert schema.component_columns()[5].component_name == "rerun.components.TextLogIndicator"
assert schema.component_columns()[5].is_static is True

def test_schema_view(self) -> None:
Expand Down

0 comments on commit 952e397

Please sign in to comment.