From 814a7ff7b96efe2a05707074ccfd7071f61b2c3a Mon Sep 17 00:00:00 2001 From: Clement Rey Date: Fri, 12 Jan 2024 09:33:57 +0100 Subject: [PATCH 1/6] do not bake latest-at semantics in low-level range APIs --- .../examples/range_components.rs | 2 +- crates/re_data_store/src/polars_util.rs | 51 ++------ crates/re_data_store/tests/data_store.rs | 112 ++++-------------- 3 files changed, 35 insertions(+), 130 deletions(-) diff --git a/crates/re_data_store/examples/range_components.rs b/crates/re_data_store/examples/range_components.rs index 71707da88608..002a74c94dd6 100644 --- a/crates/re_data_store/examples/range_components.rs +++ b/crates/re_data_store/examples/range_components.rs @@ -1,7 +1,7 @@ //! Demonstrates usage of [`re_data_store::polars_util::range_components`]. //! //! ```text -//! POLARS_FMT_MAX_ROWS=100 cargo r -p re_data_store --example range_components +//! POLARS_FMT_MAX_ROWS=100 cargo r -p re_data_store --all-features --example range_components //! ``` use polars_core::prelude::JoinType; diff --git a/crates/re_data_store/src/polars_util.rs b/crates/re_data_store/src/polars_util.rs index 7cd0b0db322c..54ad80f8a57d 100644 --- a/crates/re_data_store/src/polars_util.rs +++ b/crates/re_data_store/src/polars_util.rs @@ -85,9 +85,6 @@ pub fn latest_components( /// Iterates over the rows of any number of components and their respective cluster keys, all from /// the single point-of-view of the `primary` component, returning an iterator of `DataFrame`s. /// -/// An initial dataframe is yielded with the latest-at state at the start of the time range, if -/// there is any. -/// /// The iterator only ever yields dataframes iff the `primary` component has changed. /// A change affecting only secondary components will not yield a dataframe. /// @@ -126,51 +123,21 @@ pub fn range_components<'a, const N: usize>( let mut state = None; - // NOTE: This will return none for `TimeInt::Min`, i.e. range queries that start infinitely far - // into the past don't have a latest-at state! - let latest_time = query.range.min.as_i64().checked_sub(1).map(Into::into); - - let mut df_latest = None; - if let Some(latest_time) = latest_time { - let df = latest_components( - store, - &LatestAtQuery::new(query.timeline, latest_time), - ent_path, - &components, - join_type, - ); - - if df.as_ref().map_or(false, |df| { - // We only care about the initial state if it A) isn't empty and B) contains any data - // at all for the primary component. - !df.is_empty() && df.column(primary.as_ref()).is_ok() - }) { - df_latest = Some(df); - } - } - let primary_col = components .iter() .find_position(|component| **component == primary) .map(|(col, _)| col) .unwrap(); // asserted on entry - // send the latest-at state before anything else - df_latest - .into_iter() - .map(move |df| (latest_time, true, df)) - // followed by the range - .chain( - store - .range(query, ent_path, components) - .map(move |(time, _, cells)| { - ( - time, - cells[primary_col].is_some(), // is_primary - dataframe_from_cells(&cells), - ) - }), - ) + store + .range(query, ent_path, components) + .map(move |(time, _, cells)| { + ( + time, + cells[primary_col].is_some(), // is_primary + dataframe_from_cells(&cells), + ) + }) .filter_map(move |(time, is_primary, df)| { state = Some(join_dataframes( cluster_key, diff --git a/crates/re_data_store/tests/data_store.rs b/crates/re_data_store/tests/data_store.rs index e1b8e32583f5..5a25c9e9e0f9 100644 --- a/crates/re_data_store/tests/data_store.rs +++ b/crates/re_data_store/tests/data_store.rs @@ -419,7 +419,6 @@ fn range_impl(store: &mut DataStore) { let ent_path = EntityPath::from("this/that"); - let frame0 = TimeInt::from(0); let frame1 = TimeInt::from(1); let frame2 = TimeInt::from(2); let frame3 = TimeInt::from(3); @@ -554,61 +553,43 @@ fn range_impl(store: &mut DataStore) { // Unit ranges (Color's PoV) + // NOTE: Check out [1] to see what the results would've looked like with latest-at semantics at + // T-1 baked in (like we used to do). + // + // [1]: + assert_range_components( TimeRange::new(frame1, frame1), [Color::name(), Position2D::name()], - &[ - ( - Some(frame0), - &[(Color::name(), &row4_3), (Position2D::name(), &row4_4)], - ), // timeless - ( - Some(frame1), - &[ - (Color::name(), &row1), - (Position2D::name(), &row4_4), // timeless - ], - ), - ], + &[( + Some(frame1), + &[ + (Color::name(), &row1), + // (Position2D::name(), &row4_4), // timeless + ], + )], ); assert_range_components( TimeRange::new(frame2, frame2), [Color::name(), Position2D::name()], - &[ - ( - Some(frame1), - &[ - (Color::name(), &row1), - (Position2D::name(), &row4_4), // timeless - ], - ), // - ], + &[], ); assert_range_components( TimeRange::new(frame3, frame3), [Color::name(), Position2D::name()], - &[ - ( - Some(frame2), - &[(Color::name(), &row1), (Position2D::name(), &row2)], - ), // - ], + &[], ); assert_range_components( TimeRange::new(frame4, frame4), [Color::name(), Position2D::name()], &[ - ( - Some(frame3), - &[(Color::name(), &row1), (Position2D::name(), &row3)], - ), ( Some(frame4), - &[(Color::name(), &row4_1), (Position2D::name(), &row3)], + &[(Color::name(), &row4_1)], // ), ( Some(frame4), - &[(Color::name(), &row4_2), (Position2D::name(), &row3)], + &[(Color::name(), &row4_2)], // ), ( Some(frame4), @@ -619,12 +600,7 @@ fn range_impl(store: &mut DataStore) { assert_range_components( TimeRange::new(frame5, frame5), [Color::name(), Position2D::name()], - &[ - ( - Some(frame4), - &[(Color::name(), &row4_3), (Position2D::name(), &row4_4)], // !!! - ), // - ], + &[], ); // Unit ranges (Position2D's PoV) @@ -632,52 +608,30 @@ fn range_impl(store: &mut DataStore) { assert_range_components( TimeRange::new(frame1, frame1), [Position2D::name(), Color::name()], - &[ - ( - Some(frame0), - &[(Position2D::name(), &row4_4), (Color::name(), &row4_3)], - ), // timeless - ], + &[], ); assert_range_components( TimeRange::new(frame2, frame2), [Position2D::name(), Color::name()], &[ - ( - Some(frame1), - &[ - (Position2D::name(), &row4_4), // timeless - (Color::name(), &row1), - ], - ), ( Some(frame2), - &[(Position2D::name(), &row2), (Color::name(), &row1)], + &[(Position2D::name(), &row2)], // ), // ], ); assert_range_components( TimeRange::new(frame3, frame3), [Position2D::name(), Color::name()], - &[ - ( - Some(frame2), - &[(Position2D::name(), &row2), (Color::name(), &row1)], - ), - ( - Some(frame3), - &[(Position2D::name(), &row3), (Color::name(), &row1)], - ), - ], + &[( + Some(frame3), + &[(Position2D::name(), &row3)], // + )], ); assert_range_components( TimeRange::new(frame4, frame4), [Position2D::name(), Color::name()], &[ - ( - Some(frame3), - &[(Position2D::name(), &row3), (Color::name(), &row1)], - ), ( Some(frame4), &[(Position2D::name(), &row4_25), (Color::name(), &row4_2)], @@ -691,12 +645,7 @@ fn range_impl(store: &mut DataStore) { assert_range_components( TimeRange::new(frame5, frame5), [Position2D::name(), Color::name()], - &[ - ( - Some(frame4), - &[(Position2D::name(), &row4_4), (Color::name(), &row4_3)], - ), // - ], + &[], ); // Full range (Color's PoV) @@ -705,16 +654,9 @@ fn range_impl(store: &mut DataStore) { TimeRange::new(frame1, frame5), [Color::name(), Position2D::name()], &[ - ( - Some(frame0), - &[(Color::name(), &row4_3), (Position2D::name(), &row4_4)], - ), // timeless ( Some(frame1), - &[ - (Color::name(), &row1), - (Position2D::name(), &row4_4), // timeless - ], + &[(Color::name(), &row1)], // ), ( Some(frame4), @@ -737,10 +679,6 @@ fn range_impl(store: &mut DataStore) { TimeRange::new(frame1, frame5), [Position2D::name(), Color::name()], &[ - ( - Some(frame0), - &[(Position2D::name(), &row4_4), (Color::name(), &row4_3)], - ), // timeless ( Some(frame2), &[(Position2D::name(), &row2), (Color::name(), &row1)], From c2bc543ad9d3ca199c28f8521ff5902f175c9502 Mon Sep 17 00:00:00 2001 From: Clement Rey Date: Fri, 12 Jan 2024 09:34:16 +0100 Subject: [PATCH 2/6] do not bake latest-at semantics in uncache query APIs --- crates/re_query/src/range.rs | 80 +++------ .../re_query/tests/archetype_range_tests.rs | 161 +----------------- 2 files changed, 31 insertions(+), 210 deletions(-) diff --git a/crates/re_query/src/range.rs b/crates/re_query/src/range.rs index d332bea52d9a..44d3ea255620 100644 --- a/crates/re_query/src/range.rs +++ b/crates/re_query/src/range.rs @@ -1,9 +1,9 @@ use itertools::Itertools as _; -use re_data_store::{DataStore, LatestAtQuery, RangeQuery}; +use re_data_store::{DataStore, RangeQuery}; use re_log_types::EntityPath; use re_types_core::{Archetype, ComponentName}; -use crate::{get_component_with_instances, ArchetypeView, ComponentWithInstances}; +use crate::{ArchetypeView, ComponentWithInstances}; // --- @@ -61,61 +61,29 @@ pub fn range_archetype<'a, A: Archetype + 'a, const N: usize>( .take(components.len()) .collect(); - // NOTE: This will return none for `TimeInt::Min`, i.e. range queries that start infinitely far - // into the past don't have a latest-at state! - let query_time = query.range.min.as_i64().checked_sub(1).map(Into::into); - - let mut cwis_latest = None; - if let Some(query_time) = query_time { - let mut cwis_latest_raw: Vec<_> = std::iter::repeat_with(|| None) - .take(components.len()) - .collect(); - - // Fetch the latest data for every single component from their respective point-of-views, - // this will allow us to build up the initial state and send an initial latest-at - // entity-view if needed. - for (i, primary) in components.iter().enumerate() { - cwis_latest_raw[i] = get_component_with_instances( - store, - &LatestAtQuery::new(query.timeline, query_time), - ent_path, - *primary, - ) - .map(|(_, row_id, cwi)| (row_id, cwi)); - } - - if cwis_latest_raw[primary_col].is_some() { - cwis_latest = Some(cwis_latest_raw); - } - } - - // send the latest-at state before anything else - cwis_latest - .into_iter() - .map(move |cwis| (query_time, true, cwis)) - .chain(store.range(query, ent_path, components).map( - move |(data_time, row_id, mut cells)| { - // NOTE: The unwrap cannot fail, the cluster key's presence is guaranteed - // by the store. - let instance_keys = cells[cluster_col].take().unwrap(); - let is_primary = cells[primary_col].is_some(); - let cwis = cells - .into_iter() - .map(|cell| { - cell.map(|cell| { - ( - row_id, - ComponentWithInstances { - instance_keys: instance_keys.clone(), /* shallow */ - values: cell, - }, - ) - }) + store + .range(query, ent_path, components) + .map(move |(data_time, row_id, mut cells)| { + // NOTE: The unwrap cannot fail, the cluster key's presence is guaranteed + // by the store. + let instance_keys = cells[cluster_col].take().unwrap(); + let is_primary = cells[primary_col].is_some(); + let cwis = cells + .into_iter() + .map(|cell| { + cell.map(|cell| { + ( + row_id, + ComponentWithInstances { + instance_keys: instance_keys.clone(), /* shallow */ + values: cell, + }, + ) }) - .collect::>(); - (data_time, is_primary, cwis) - }, - )) + }) + .collect::>(); + (data_time, is_primary, cwis) + }) .filter_map(move |(data_time, is_primary, cwis)| { for (i, cwi) in cwis .into_iter() diff --git a/crates/re_query/tests/archetype_range_tests.rs b/crates/re_query/tests/archetype_range_tests.rs index 540e3eb9b02f..bd2c807cc68e 100644 --- a/crates/re_query/tests/archetype_range_tests.rs +++ b/crates/re_query/tests/archetype_range_tests.rs @@ -70,8 +70,6 @@ fn simple_range() { // --- First test: `(timepoint1, timepoint3]` --- - // The exclusion of `timepoint1` means latest-at semantics will kick in! - let query = re_data_store::RangeQuery::new( timepoint1[0].0, TimeRange::new((timepoint1[0].1.as_i64() + 1).into(), timepoint3[0].1), @@ -102,38 +100,10 @@ fn simple_range() { // │ 1 ┆ {30.0,40.0} ┆ null │ // └─────────────┴──────────────┴─────────────────┘ - { - // Frame #123 - - let arch_view = &results[0]; - let time = arch_view.data_time().unwrap(); - - // Build expected df manually - let instances = vec![Some(InstanceKey(0)), Some(InstanceKey(1))]; - let positions = vec![ - Some(Position2D::new(1.0, 2.0)), - Some(Position2D::new(3.0, 4.0)), - ]; - let colors = vec![None, Some(Color::from_rgb(255, 0, 0))]; - let expected = DataCellRow(smallvec![ - DataCell::from_native_sparse(instances), - DataCell::from_native_sparse(positions), - DataCell::from_native_sparse(colors) - ]); - - //eprintln!("{df:?}"); - //eprintln!("{expected:?}"); - - assert_eq!(TimeInt::from(123), time); - assert_eq!( - &expected, - &arch_view.to_data_cell_row_2::().unwrap(), - ); - } { // Frame #323 - let arch_view = &results[1]; + let arch_view = &results[0]; let time = arch_view.data_time().unwrap(); // Build expected df manually @@ -161,8 +131,6 @@ fn simple_range() { // --- Second test: `[timepoint1, timepoint3]` --- - // The inclusion of `timepoint1` means latest-at semantics will _not_ kick in! - let query = re_data_store::RangeQuery::new( timepoint1[0].0, TimeRange::new(timepoint1[0].1, timepoint3[0].1), @@ -379,15 +347,6 @@ fn timeless_range() { // We expect this to generate the following `DataFrame`s: // - // Frame #123: - // ┌────────────────────┬───────────────┬─────────────────┐ - // │ InstanceKey ┆ Point2D ┆ Color │ - // ╞════════════════════╪═══════════════╪═════════════════╡ - // │ 0 ┆ {1.0,2.0} ┆ null │ - // ├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┤ - // │ 1 ┆ {3.0,4.0} ┆ 4278190080 │ - // └────────────────────┴───────────────┴─────────────────┘ - // // Frame #323: // ┌────────────────────┬───────────────┬─────────────────┐ // │ InstanceKey ┆ Point2D ┆ Color │ @@ -397,38 +356,10 @@ fn timeless_range() { // │ 1 ┆ {30.0,40.0} ┆ null │ // └────────────────────┴───────────────┴─────────────────┘ - { - // Frame #123 - - let arch_view = &results[0]; - let time = arch_view.data_time().unwrap(); - - // Build expected df manually - let instances = vec![Some(InstanceKey(0)), Some(InstanceKey(1))]; - let positions = vec![ - Some(Position2D::new(1.0, 2.0)), - Some(Position2D::new(3.0, 4.0)), - ]; - let colors = vec![None, Some(Color::from_rgb(255, 0, 0))]; - let expected = DataCellRow(smallvec![ - DataCell::from_native_sparse(instances), - DataCell::from_native_sparse(positions), - DataCell::from_native_sparse(colors) - ]); - - //eprintln!("{df:?}"); - //eprintln!("{expected:?}"); - - assert_eq!(TimeInt::from(123), time); - assert_eq!( - &expected, - &arch_view.to_data_cell_row_2::().unwrap(), - ); - } { // Frame #323 - let arch_view = &results[1]; + let arch_view = &results[0]; let time = arch_view.data_time().unwrap(); // Build expected df manually @@ -456,8 +387,6 @@ fn timeless_range() { // --- Second test: `[timepoint1, timepoint3]` --- - // The inclusion of `timepoint1` means latest-at semantics will fall back to timeless data! - let query = re_data_store::RangeQuery::new( timepoint1[0].0, TimeRange::new(timepoint1[0].1, timepoint3[0].1), @@ -470,15 +399,6 @@ fn timeless_range() { // We expect this to generate the following `DataFrame`s: // - // Frame #122: - // ┌────────────────────┬───────────────┬─────────────────┐ - // │ InstanceKey ┆ Point2D ┆ Color │ - // ╞════════════════════╪═══════════════╪═════════════════╡ - // │ 0 ┆ {10.0,20.0} ┆ null │ - // ├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┤ - // │ 1 ┆ {30.0,40.0} ┆ 4278190080 │ - // └────────────────────┴───────────────┴─────────────────┘ - // // Frame #123: // ┌────────────────────┬───────────────┬─────────────────┐ // │ InstanceKey ┆ Point2D ┆ Color │ @@ -498,36 +418,9 @@ fn timeless_range() { // └────────────────────┴───────────────┴─────────────────┘ { - // Frame #122 (all timeless) - - let arch_view = &results[0]; - let time = arch_view.data_time().unwrap(); - - // Build expected df manually - let instances = vec![Some(InstanceKey(0)), Some(InstanceKey(1))]; - let positions = vec![ - Some(Position2D::new(10.0, 20.0)), - Some(Position2D::new(30.0, 40.0)), - ]; - let colors = vec![None, Some(Color::from_rgb(255, 0, 0))]; - let expected = DataCellRow(smallvec![ - DataCell::from_native_sparse(instances), - DataCell::from_native_sparse(positions), - DataCell::from_native_sparse(colors) - ]); - - //eprintln!("{df:?}"); - //eprintln!("{expected:?}"); - - assert_eq!(TimeInt::from(122), time); - assert_eq!( - &expected, - &arch_view.to_data_cell_row_2::().unwrap(), - ); - // Frame #123 (partially timeless) - let arch_view = &results[1]; + let arch_view = &results[0]; let time = arch_view.data_time().unwrap(); // Build expected df manually @@ -536,7 +429,7 @@ fn timeless_range() { Some(Position2D::new(1.0, 2.0)), Some(Position2D::new(3.0, 4.0)), ]; - let colors = vec![None, Some(Color::from_rgb(255, 0, 0))]; + let colors: Vec> = vec![None, None]; let expected = DataCellRow(smallvec![ DataCell::from_native_sparse(instances), DataCell::from_native_sparse(positions), @@ -555,7 +448,7 @@ fn timeless_range() { { // Frame #323 - let arch_view = &results[2]; + let arch_view = &results[1]; let time = arch_view.data_time().unwrap(); // Build expected df manually @@ -802,8 +695,6 @@ fn simple_splatted_range() { // --- First test: `(timepoint1, timepoint3]` --- - // The exclusion of `timepoint1` means latest-at semantics will kick in! - let query = re_data_store::RangeQuery::new( timepoint1[0].0, TimeRange::new((timepoint1[0].1.as_i64() + 1).into(), timepoint3[0].1), @@ -816,15 +707,6 @@ fn simple_splatted_range() { // We expect this to generate the following `DataFrame`s: // - // Frame #123: - // ┌────────────────────┬───────────────┬─────────────────┐ - // │ InstanceKey ┆ Point2D ┆ Color │ - // ╞════════════════════╪═══════════════╪═════════════════╡ - // │ 0 ┆ {1.0,2.0} ┆ null │ - // ├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┤ - // │ 1 ┆ {3.0,4.0} ┆ 4278190080 │ - // └────────────────────┴───────────────┴─────────────────┘ - // // Frame #323: // ┌────────────────────┬───────────────┬─────────────────┐ // │ InstanceKey ┆ Point2D ┆ Color │ @@ -834,41 +716,12 @@ fn simple_splatted_range() { // │ 1 ┆ {30.0,40.0} ┆ 16711680 │ // └────────────────────┴───────────────┴─────────────────┘ - assert_eq!(results.len(), 2); - - { - // Frame #123 - - let arch_view = &results[0]; - let time = arch_view.data_time().unwrap(); - - // Build expected df manually - let instances = vec![Some(InstanceKey(0)), Some(InstanceKey(1))]; - let positions = vec![ - Some(Position2D::new(1.0, 2.0)), - Some(Position2D::new(3.0, 4.0)), - ]; - let colors = vec![None, Some(Color::from_rgb(255, 0, 0))]; - let expected = DataCellRow(smallvec![ - DataCell::from_native_sparse(instances), - DataCell::from_native_sparse(positions), - DataCell::from_native_sparse(colors) - ]); - - //eprintln!("{df:?}"); - //eprintln!("{expected:?}"); - - assert_eq!(TimeInt::from(123), time); - assert_eq!( - &expected, - &arch_view.to_data_cell_row_2::().unwrap(), - ); - } + assert_eq!(results.len(), 1); { // Frame #323 - let arch_view = &results[1]; + let arch_view = &results[0]; let time = arch_view.data_time().unwrap(); // Build expected df manually From f165e7e1e4953caeacba7b9fdfd0e9898f2b0bb3 Mon Sep 17 00:00:00 2001 From: Clement Rey Date: Fri, 12 Jan 2024 09:34:34 +0100 Subject: [PATCH 3/6] cached range test suite shall now pass --- crates/re_query_cache/tests/range.rs | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/crates/re_query_cache/tests/range.rs b/crates/re_query_cache/tests/range.rs index e551e46a4418..a0312ba1127a 100644 --- a/crates/re_query_cache/tests/range.rs +++ b/crates/re_query_cache/tests/range.rs @@ -17,8 +17,6 @@ use re_types_core::Loggable as _; // --- #[test] -// TODO(cmc): actually make cached range queries correct -#[should_panic(expected = "assertion failed: `(left == right)`")] fn simple_range() { let mut store = DataStore::new( re_log_types::StoreId::random(re_log_types::StoreKind::Recording), @@ -79,8 +77,6 @@ fn simple_range() { // --- First test: `(timepoint1, timepoint3]` --- - // The exclusion of `timepoint1` means latest-at semantics will kick in! - let query = re_data_store::RangeQuery::new( timepoint1[0].0, TimeRange::new((timepoint1[0].1.as_i64() + 1).into(), timepoint3[0].1), @@ -101,7 +97,7 @@ fn simple_range() { } #[test] -// TODO(cmc): cached range timeless support +// TODO(cmc): timeless support #[should_panic(expected = "assertion failed: `(left == right)`")] fn timeless_range() { let mut store = DataStore::new( @@ -196,8 +192,6 @@ fn timeless_range() { // --- First test: `(timepoint1, timepoint3]` --- - // The exclusion of `timepoint1` means latest-at semantics will kick in! - let query = re_data_store::RangeQuery::new( timepoint1[0].0, TimeRange::new((timepoint1[0].1.as_i64() + 1).into(), timepoint3[0].1), @@ -225,8 +219,6 @@ fn timeless_range() { } #[test] -// TODO(cmc): actually make cached range queries correct -#[should_panic(expected = "assertion failed: `(left == right)`")] fn simple_splatted_range() { let mut store = DataStore::new( re_log_types::StoreId::random(re_log_types::StoreKind::Recording), @@ -287,8 +279,6 @@ fn simple_splatted_range() { // --- First test: `(timepoint1, timepoint3]` --- - // The exclusion of `timepoint1` means latest-at semantics will kick in! - let query = re_data_store::RangeQuery::new( timepoint1[0].0, TimeRange::new((timepoint1[0].1.as_i64() + 1).into(), timepoint3[0].1), @@ -374,6 +364,7 @@ fn query_and_compare(store: &DataStore, query: &RangeQuery, ent_path: &EntityPat // Keep this around for the next unlucky chap. // eprintln!("(expected={expected_data_times:?}, uncached={uncached_data_times:?}, cached={cached_data_times:?})"); + // eprintln!("{}", store.to_data_table().unwrap()); similar_asserts::assert_eq!(expected_data_times, uncached_data_times); similar_asserts::assert_eq!(expected_instance_keys, uncached_instance_keys); From 16093b9fb20112db6086fe098001006901cba55f Mon Sep 17 00:00:00 2001 From: Clement Rey Date: Fri, 12 Jan 2024 09:48:01 +0100 Subject: [PATCH 4/6] update visual history UI accordingly --- crates/re_viewer/src/ui/visible_history.rs | 26 ++++++---------------- 1 file changed, 7 insertions(+), 19 deletions(-) diff --git a/crates/re_viewer/src/ui/visible_history.rs b/crates/re_viewer/src/ui/visible_history.rs index 4b050323e376..2f34afadf659 100644 --- a/crates/re_viewer/src/ui/visible_history.rs +++ b/crates/re_viewer/src/ui/visible_history.rs @@ -246,9 +246,6 @@ fn current_range_ui( is_sequence_timeline: bool, visible_history: &VisibleHistory, ) { - let from = visible_history.from(current_time.into()); - let to = visible_history.to(current_time.into()); - let (time_type, quantity_name) = if is_sequence_timeline { (TimeType::Sequence, "frame") } else { @@ -260,22 +257,13 @@ fn current_range_ui( ctx.app_options.time_zone_for_timestamps, ); - if from == to { - ui.label(format!( - "Showing last data logged on or before {quantity_name} {from_formatted}" - )); - } else { - ui.label(format!( - "Showing data between {quantity_name}s {from_formatted} and {}.", - time_type.format( - visible_history.to(current_time.into()), - ctx.app_options.time_zone_for_timestamps - ) - )) - .on_hover_text(format!( - "This includes the data current as of the starting {quantity_name}." - )); - }; + ui.label(format!( + "Showing data between {quantity_name}s {from_formatted} and {} (included).", + time_type.format( + visible_history.to(current_time.into()), + ctx.app_options.time_zone_for_timestamps + ) + )); } #[allow(clippy::too_many_arguments)] From 204689fe48263405c67a8ec32baccfdf34a665b6 Mon Sep 17 00:00:00 2001 From: Clement Rey Date: Fri, 12 Jan 2024 10:28:26 +0100 Subject: [PATCH 5/6] always display single entry plots as a point --- .../src/visualizer_system.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/crates/re_space_view_time_series/src/visualizer_system.rs b/crates/re_space_view_time_series/src/visualizer_system.rs index ba2318af15e8..6a7f4126d775 100644 --- a/crates/re_space_view_time_series/src/visualizer_system.rs +++ b/crates/re_space_view_time_series/src/visualizer_system.rs @@ -227,7 +227,17 @@ impl TimeSeriesSystem { let line_label = same_label(&points).unwrap_or_else(|| data_result.entity_path.to_string()); - self.add_line_segments(&line_label, points); + if points.len() == 1 { + self.lines.push(PlotSeries { + label: line_label, + color: points[0].attrs.color, + width: 2.0 * points[0].attrs.radius, + kind: PlotSeriesKind::Scatter, + points: vec![(points[0].time, points[0].value)], + }); + } else { + self.add_line_segments(&line_label, points); + } } Ok(()) From 07bdde90062d9f78539baeb6ebfb982251d904c4 Mon Sep 17 00:00:00 2001 From: Clement Rey Date: Mon, 15 Jan 2024 16:30:22 +0100 Subject: [PATCH 6/6] review --- crates/re_data_store/tests/data_store.rs | 5 +---- .../re_query/tests/archetype_range_tests.rs | 22 ------------------- 2 files changed, 1 insertion(+), 26 deletions(-) diff --git a/crates/re_data_store/tests/data_store.rs b/crates/re_data_store/tests/data_store.rs index 5a25c9e9e0f9..b52418325ee9 100644 --- a/crates/re_data_store/tests/data_store.rs +++ b/crates/re_data_store/tests/data_store.rs @@ -563,10 +563,7 @@ fn range_impl(store: &mut DataStore) { [Color::name(), Position2D::name()], &[( Some(frame1), - &[ - (Color::name(), &row1), - // (Position2D::name(), &row4_4), // timeless - ], + &[(Color::name(), &row1)], // )], ); assert_range_components( diff --git a/crates/re_query/tests/archetype_range_tests.rs b/crates/re_query/tests/archetype_range_tests.rs index bd2c807cc68e..980e7b5c99c9 100644 --- a/crates/re_query/tests/archetype_range_tests.rs +++ b/crates/re_query/tests/archetype_range_tests.rs @@ -82,15 +82,6 @@ fn simple_range() { // We expect this to generate the following `DataFrame`s: // - // Frame #123: - // ┌─────────────┬───────────┬──────────────┐ - // │ InstanceKey ┆ Point2D ┆ Color │ - // ╞═════════════╪═══════════╪══════════════╡ - // │ 0 ┆ {1.0,2.0} ┆ null │ - // ├╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌┤ - // │ 1 ┆ {3.0,4.0} ┆ 4278190080 │ - // └─────────────┴───────────┴──────────────┘ - // // Frame #323: // ┌─────────────┬──────────────┬─────────────────┐ // │ InstanceKey ┆ Point2D ┆ Color │ @@ -119,7 +110,6 @@ fn simple_range() { DataCell::from_native_sparse(colors) ]); - //eprintln!("{df:?}"); //eprintln!("{expected:?}"); assert_eq!(TimeInt::from(323), time); @@ -180,7 +170,6 @@ fn simple_range() { DataCell::from_native_sparse(colors) ]); - //eprintln!("{df:?}"); //eprintln!("{expected:?}"); assert_eq!(TimeInt::from(123), time); @@ -208,7 +197,6 @@ fn simple_range() { DataCell::from_native_sparse(colors) ]); - //eprintln!("{df:?}"); //eprintln!("{expected:?}"); assert_eq!(TimeInt::from(323), time); @@ -375,7 +363,6 @@ fn timeless_range() { DataCell::from_native_sparse(colors) ]); - //eprintln!("{df:?}"); //eprintln!("{expected:?}"); assert_eq!(TimeInt::from(323), time); @@ -436,7 +423,6 @@ fn timeless_range() { DataCell::from_native_sparse(colors) ]); - //eprintln!("{df:?}"); //eprintln!("{expected:?}"); assert_eq!(TimeInt::from(123), time); @@ -464,7 +450,6 @@ fn timeless_range() { DataCell::from_native_sparse(colors) ]); - //eprintln!("{df:?}"); //eprintln!("{expected:?}"); assert_eq!(TimeInt::from(323), time); @@ -541,7 +526,6 @@ fn timeless_range() { DataCell::from_native_sparse(colors) ]); - //eprintln!("{df:?}"); //eprintln!("{expected:?}"); assert_eq!(None, time); @@ -568,7 +552,6 @@ fn timeless_range() { DataCell::from_native_sparse(colors) ]); - //eprintln!("{df:?}"); //eprintln!("{expected:?}"); assert_eq!(None, time); @@ -595,7 +578,6 @@ fn timeless_range() { DataCell::from_native_sparse(colors) ]); - //eprintln!("{df:?}"); //eprintln!("{expected:?}"); assert_eq!(TimeInt::from(123), time); @@ -623,7 +605,6 @@ fn timeless_range() { DataCell::from_native_sparse(colors) ]); - //eprintln!("{df:?}"); //eprintln!("{expected:?}"); assert_eq!(TimeInt::from(323), time); @@ -742,7 +723,6 @@ fn simple_splatted_range() { DataCell::from_native_sparse(colors) ]); - //eprintln!("{df:?}"); //eprintln!("{expected:?}"); assert_eq!(TimeInt::from(323), time); @@ -802,7 +782,6 @@ fn simple_splatted_range() { DataCell::from_native_sparse(colors) ]); - //eprintln!("{df:?}"); //eprintln!("{expected:?}"); assert_eq!(TimeInt::from(123), time); @@ -833,7 +812,6 @@ fn simple_splatted_range() { DataCell::from_native_sparse(colors) ]); - //eprintln!("{df:?}"); //eprintln!("{expected:?}"); assert_eq!(TimeInt::from(323), time);