From c48b3cb9304040f0abd0278090dfbaac6ba5f889 Mon Sep 17 00:00:00 2001 From: Clement Rey Date: Tue, 17 Dec 2024 10:36:26 +0100 Subject: [PATCH 1/4] make proper use of `ChunkComponents::insert_descriptor` everywhere --- crates/store/re_chunk/src/batcher.rs | 15 +++------------ crates/store/re_chunk/src/chunk.rs | 8 +++----- crates/store/re_chunk/src/merge.rs | 5 +---- crates/store/re_chunk/src/transport.rs | 4 +--- 4 files changed, 8 insertions(+), 24 deletions(-) diff --git a/crates/store/re_chunk/src/batcher.rs b/crates/store/re_chunk/src/batcher.rs index d6cb0f38e643..cefbaeed9a42 100644 --- a/crates/store/re_chunk/src/batcher.rs +++ b/crates/store/re_chunk/src/batcher.rs @@ -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); } } @@ -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 @@ -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 diff --git a/crates/store/re_chunk/src/chunk.rs b/crates/store/re_chunk/src/chunk.rs index 8184bb5178d7..0d39331121db 100644 --- a/crates/store/re_chunk/src/chunk.rs +++ b/crates/store/re_chunk/src/chunk.rs @@ -66,11 +66,11 @@ impl ChunkComponents { &mut self, component_desc: ComponentDescriptor, list_array: Arrow2ListArray, - ) { + ) -> Option> { 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. @@ -144,9 +144,7 @@ impl FromIterator<(ComponentDescriptor, Arrow2ListArray)> 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 diff --git a/crates/store/re_chunk/src/merge.rs b/crates/store/re_chunk/src/merge.rs index 09e347f4221f..e61467ab69c2 100644 --- a/crates/store/re_chunk/src/merge.rs +++ b/crates/store/re_chunk/src/merge.rs @@ -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 }; diff --git a/crates/store/re_chunk/src/transport.rs b/crates/store/re_chunk/src/transport.rs index 6e5bbcdcb3f7..874ba3034ff9 100644 --- a/crates/store/re_chunk/src/transport.rs +++ b/crates/store/re_chunk/src/transport.rs @@ -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 { From b659be606570a26a314833059c28580f9f2a4e83 Mon Sep 17 00:00:00 2001 From: Clement Rey Date: Tue, 17 Dec 2024 11:09:11 +0100 Subject: [PATCH 2/4] disable tagging --- crates/store/re_chunk/src/chunk.rs | 2 ++ crates/store/re_chunk_store/tests/gc.rs | 2 ++ crates/store/re_types_core/src/component_descriptor.rs | 4 ++++ 3 files changed, 8 insertions(+) diff --git a/crates/store/re_chunk/src/chunk.rs b/crates/store/re_chunk/src/chunk.rs index 0d39331121db..25ace6e31545 100644 --- a/crates/store/re_chunk/src/chunk.rs +++ b/crates/store/re_chunk/src/chunk.rs @@ -67,6 +67,8 @@ impl ChunkComponents { component_desc: ComponentDescriptor, list_array: Arrow2ListArray, ) -> Option> { + // TODO(cmc): revert me + let component_desc = component_desc.untagged(); self.0 .entry(component_desc.component_name) .or_default() diff --git a/crates/store/re_chunk_store/tests/gc.rs b/crates/store/re_chunk_store/tests/gc.rs index cc3e0601b170..d461fe2ac79e 100644 --- a/crates/store/re_chunk_store/tests/gc.rs +++ b/crates/store/re_chunk_store/tests/gc.rs @@ -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); diff --git a/crates/store/re_types_core/src/component_descriptor.rs b/crates/store/re_types_core/src/component_descriptor.rs index 4e3f07693df9..0d0d10c6cf45 100644 --- a/crates/store/re_types_core/src/component_descriptor.rs +++ b/crates/store/re_types_core/src/component_descriptor.rs @@ -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 { From 0387be2015789e6d74b90f179f0de4676364ea84 Mon Sep 17 00:00:00 2001 From: Clement Rey Date: Tue, 17 Dec 2024 11:09:20 +0100 Subject: [PATCH 3/4] update roundtrips accordingly --- .../descriptors/descr_builtin_archetype.rs | 26 +++++++++++++--- .../all/descriptors/descr_custom_archetype.rs | 30 +++++++++++++++---- .../all/descriptors/descr_custom_component.rs | 12 ++++++-- 3 files changed, 56 insertions(+), 12 deletions(-) diff --git a/docs/snippets/all/descriptors/descr_builtin_archetype.rs b/docs/snippets/all/descriptors/descr_builtin_archetype.rs index 48ddffd25d5a..5562420b50cc 100644 --- a/docs/snippets/all/descriptors/descr_builtin_archetype.rs +++ b/docs/snippets/all/descriptors/descr_builtin_archetype.rs @@ -56,6 +56,24 @@ fn check_tags(rec: &rerun::RecordingStream) { .collect::>(); 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, @@ -63,13 +81,13 @@ fn check_tags(rec: &rerun::RecordingStream) { 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(), }, ]; diff --git a/docs/snippets/all/descriptors/descr_custom_archetype.rs b/docs/snippets/all/descriptors/descr_custom_archetype.rs index 83cab7434ce6..f9804b0f1280 100644 --- a/docs/snippets/all/descriptors/descr_custom_archetype.rs +++ b/docs/snippets/all/descriptors/descr_custom_archetype.rs @@ -104,20 +104,38 @@ fn check_tags(rec: &rerun::RecordingStream) { .collect::>(); 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(), }, ]; diff --git a/docs/snippets/all/descriptors/descr_custom_component.rs b/docs/snippets/all/descriptors/descr_custom_component.rs index 3ad9e97628bd..14e3f20505bb 100644 --- a/docs/snippets/all/descriptors/descr_custom_component.rs +++ b/docs/snippets/all/descriptors/descr_custom_component.rs @@ -59,10 +59,18 @@ fn check_tags(rec: &rerun::RecordingStream) { .collect::>(); 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(), }, // ]; From 5296d2ae8a2a9971843e69df00822f6cc07bbc4a Mon Sep 17 00:00:00 2001 From: Clement Rey Date: Tue, 17 Dec 2024 11:35:59 +0100 Subject: [PATCH 4/4] forgot about the py tests --- rerun_py/tests/unit/test_dataframe.py | 30 +++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/rerun_py/tests/unit/test_dataframe.py b/rerun_py/tests/unit/test_dataframe.py index 067f7fa63b34..a2e3e29ac13e 100644 --- a/rerun_py/tests/unit/test_dataframe.py +++ b/rerun_py/tests/unit/test_dataframe.py @@ -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" @@ -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: