Skip to content

Commit

Permalink
Fix Loggable pretending to have semantics (#8082)
Browse files Browse the repository at this point in the history
Remove `Loggable`'s associated `Name` type and simplify the `Loggable`
and `Component` types accordingly.

A `Loggable` doesn't have a name, in fact it doesn't have any semantics
at all: it's just arrow-serializable data.
`Component` is where semantics live at.

Not only is this wrong, this adds a whole bunch of useless code that
makes future improvements (hints: tags) more complicated for no reason.
  • Loading branch information
teh-cmc authored Nov 15, 2024
1 parent 71d8601 commit 015888f
Show file tree
Hide file tree
Showing 269 changed files with 1,238 additions and 1,750 deletions.
27 changes: 13 additions & 14 deletions crates/build/re_types_builder/src/codegen/rust/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -851,13 +851,6 @@ fn quote_trait_impls_for_datatype_or_component(

let name = format_ident!("{name}");

let quoted_kind = if *kind == ObjectKind::Datatype {
quote!(Datatype)
} else {
quote!(Component)
};
let kind_name = format_ident!("{quoted_kind}Name");

let datatype = arrow_registry.get(fqname);

let optimize_for_buffer_slice = should_optimize_buffer_slice_deserialize(obj, arrow_registry);
Expand Down Expand Up @@ -1002,17 +995,21 @@ fn quote_trait_impls_for_datatype_or_component(
}
};

let quoted_impl_component = (obj.kind == ObjectKind::Component).then(|| {
quote! {
impl ::re_types_core::Component for #name {
#[inline]
fn name() -> ComponentName {
#fqname.into()
}
}
}
});

quote! {
::re_types_core::macros::impl_into_cow!(#name);

impl ::re_types_core::Loggable for #name {
type Name = ::re_types_core::#kind_name;

#[inline]
fn name() -> Self::Name {
#fqname.into()
}

#quoted_arrow_datatype

#quoted_serializer
Expand All @@ -1029,6 +1026,8 @@ fn quote_trait_impls_for_datatype_or_component(

#quoted_from_arrow
}

#quoted_impl_component
}
}

Expand Down
5 changes: 3 additions & 2 deletions crates/build/re_types_builder/src/codegen/rust/reflection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ pub fn generate_reflection(
use re_types_core::{
ArchetypeName,
ComponentName,
Loggable,
Component,
Loggable as _,
LoggableBatch as _,
reflection::{
ArchetypeFieldReflection,
Expand Down Expand Up @@ -103,7 +104,7 @@ fn generate_component_reflection(
let type_name = format_ident!("{}", obj.name);

let quoted_name = if true {
quote!( <#type_name as Loggable>::name() )
quote!( <#type_name as Component>::name() )
} else {
// Works too
let fqname = &obj.fqname;
Expand Down
2 changes: 1 addition & 1 deletion crates/store/re_chunk/examples/latest_at.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use re_chunk::{Chunk, LatestAtQuery, RowId, Timeline};
use re_log_types::example_components::{MyColor, MyLabel, MyPoint};
use re_types_core::Loggable as _;
use re_types_core::Component as _;

// ---

Expand Down
2 changes: 1 addition & 1 deletion crates/store/re_chunk/examples/range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use re_log_types::{
example_components::{MyColor, MyLabel, MyPoint},
ResolvedTimeRange,
};
use re_types_core::Loggable as _;
use re_types_core::Component as _;

// ---

Expand Down
2 changes: 1 addition & 1 deletion crates/store/re_chunk/src/batcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -985,7 +985,7 @@ mod tests {
use crossbeam::channel::TryRecvError;

use re_log_types::example_components::{MyPoint, MyPoint64};
use re_types_core::Loggable as _;
use re_types_core::{Component as _, Loggable as _};

use super::*;

Expand Down
2 changes: 1 addition & 1 deletion crates/store/re_chunk/src/merge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ mod tests {
use super::*;

use re_log_types::example_components::{MyColor, MyLabel, MyPoint, MyPoint64};
use re_types_core::Loggable;
use re_types_core::Component;

use crate::{Chunk, RowId, Timeline};

Expand Down
2 changes: 1 addition & 1 deletion crates/store/re_chunk/src/shuffle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ mod tests {
example_components::{MyColor, MyPoint},
EntityPath, Timeline,
};
use re_types_core::Loggable as _;
use re_types_core::Component as _;

use crate::{ChunkId, RowId};

Expand Down
16 changes: 8 additions & 8 deletions crates/store/re_chunk/src/slice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -875,7 +875,7 @@ mod tests {
example_components::{MyColor, MyLabel, MyPoint},
TimePoint,
};
use re_types_core::{ComponentBatch, Loggable};
use re_types_core::Component;

use crate::{Chunk, RowId, Timeline};

Expand Down Expand Up @@ -974,7 +974,7 @@ mod tests {

eprintln!("chunk:\n{chunk}");

let expectations: &[(_, _, Option<&dyn ComponentBatch>)] = &[
let expectations: &[(_, _, Option<&dyn re_types_core::ComponentBatch>)] = &[
(row_id1, MyPoint::name(), Some(points1 as _)),
(row_id2, MyLabel::name(), Some(labels4 as _)),
(row_id3, MyColor::name(), None),
Expand Down Expand Up @@ -1101,7 +1101,7 @@ mod tests {
eprintln!("got:\n{got}");
assert_eq!(2, got.num_rows());

let expectations: &[(_, _, Option<&dyn ComponentBatch>)] = &[
let expectations: &[(_, _, Option<&dyn re_types_core::ComponentBatch>)] = &[
(row_id3, MyPoint::name(), Some(points3 as _)),
(row_id3, MyColor::name(), None),
(row_id3, MyLabel::name(), Some(labels3 as _)),
Expand All @@ -1124,7 +1124,7 @@ mod tests {
eprintln!("got:\n{got}");
assert_eq!(5, got.num_rows());

let expectations: &[(_, _, Option<&dyn ComponentBatch>)] = &[
let expectations: &[(_, _, Option<&dyn re_types_core::ComponentBatch>)] = &[
(row_id1, MyPoint::name(), Some(points1 as _)),
(row_id1, MyColor::name(), None),
(row_id1, MyLabel::name(), Some(labels1 as _)),
Expand Down Expand Up @@ -1232,7 +1232,7 @@ mod tests {
eprintln!("got:\n{got}");
assert_eq!(1, got.num_rows());

let expectations: &[(_, _, Option<&dyn ComponentBatch>)] = &[
let expectations: &[(_, _, Option<&dyn re_types_core::ComponentBatch>)] = &[
(row_id5, MyPoint::name(), None),
(row_id5, MyColor::name(), Some(colors5 as _)),
(row_id5, MyLabel::name(), Some(labels5 as _)),
Expand All @@ -1251,7 +1251,7 @@ mod tests {
eprintln!("got:\n{got}");
assert_eq!(1, got.num_rows());

let expectations: &[(_, _, Option<&dyn ComponentBatch>)] = &[
let expectations: &[(_, _, Option<&dyn re_types_core::ComponentBatch>)] = &[
(row_id5, MyPoint::name(), None),
(row_id5, MyColor::name(), Some(colors5 as _)),
(row_id5, MyLabel::name(), Some(labels5 as _)),
Expand Down Expand Up @@ -1370,7 +1370,7 @@ mod tests {
eprintln!("got:\n{got}");
assert_eq!(filter.values_iter().filter(|&b| b).count(), got.num_rows());

let expectations: &[(_, _, Option<&dyn ComponentBatch>)] = &[
let expectations: &[(_, _, Option<&dyn re_types_core::ComponentBatch>)] = &[
(row_id1, MyPoint::name(), Some(points1 as _)),
(row_id1, MyColor::name(), None),
(row_id1, MyLabel::name(), Some(labels1 as _)),
Expand Down Expand Up @@ -1517,7 +1517,7 @@ mod tests {
eprintln!("got:\n{got}");
assert_eq!(indices.len(), got.num_rows());

let expectations: &[(_, _, Option<&dyn ComponentBatch>)] = &[
let expectations: &[(_, _, Option<&dyn re_types_core::ComponentBatch>)] = &[
(row_id1, MyPoint::name(), Some(points1 as _)),
(row_id1, MyColor::name(), None),
(row_id1, MyLabel::name(), Some(labels1 as _)),
Expand Down
2 changes: 1 addition & 1 deletion crates/store/re_chunk/src/transport.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use arrow2::{
};

use re_log_types::{EntityPath, Timeline};
use re_types_core::{Loggable as _, SizeBytes};
use re_types_core::{Component as _, Loggable as _, SizeBytes};

use crate::{Chunk, ChunkError, ChunkId, ChunkResult, RowId, TimeColumn};

Expand Down
2 changes: 1 addition & 1 deletion crates/store/re_chunk/tests/latest_at.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use nohash_hasher::IntMap;

use re_chunk::{Chunk, ComponentName, LatestAtQuery, RowId, TimePoint, Timeline};
use re_log_types::example_components::{MyColor, MyLabel, MyPoint};
use re_types_core::Loggable;
use re_types_core::{Component, Loggable};

// ---

Expand Down
2 changes: 1 addition & 1 deletion crates/store/re_chunk/tests/range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use re_log_types::{
example_components::{MyColor, MyLabel, MyPoint},
ResolvedTimeRange,
};
use re_types_core::Loggable as _;
use re_types_core::{Component as _, Loggable as _};

// ---

Expand Down
2 changes: 1 addition & 1 deletion crates/store/re_chunk_store/src/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ mod tests {
example_components::{MyColor, MyIndex, MyPoint},
EntityPath, TimeInt, TimePoint, Timeline,
};
use re_types_core::{ComponentName, Loggable as _};
use re_types_core::{Component as _, ComponentName};

use crate::{ChunkStore, GarbageCollectionOptions};

Expand Down
2 changes: 1 addition & 1 deletion crates/store/re_chunk_store/tests/correctness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use re_log_types::{
build_frame_nr, build_log_time, Duration, EntityPath, Time, TimeInt, TimePoint, TimeType,
Timeline,
};
use re_types_core::Loggable as _;
use re_types_core::Component as _;

// ---

Expand Down
2 changes: 1 addition & 1 deletion crates/store/re_chunk_store/tests/drop_time_range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use re_chunk::{Chunk, RowId};
use re_chunk_store::{ChunkStore, ChunkStoreConfig};
use re_log_types::{example_components::MyColor, ResolvedTimeRange};
use re_log_types::{EntityPath, TimePoint, Timeline};
use re_types_core::Loggable as _;
use re_types_core::Component as _;

#[test]
fn drop_time_range() -> anyhow::Result<()> {
Expand Down
2 changes: 1 addition & 1 deletion crates/store/re_chunk_store/tests/gc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use re_log_types::{
EntityPath, ResolvedTimeRange, Time, TimeType, Timeline,
};
use re_types::testing::build_some_large_structs;
use re_types_core::Loggable as _;
use re_types_core::Component as _;

// ---

Expand Down
2 changes: 1 addition & 1 deletion crates/store/re_chunk_store/tests/memory_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ use re_chunk::{
};
use re_chunk_store::{ChunkStore, ChunkStoreConfig};
use re_log_types::{TimePoint, TimeType, Timeline};
use re_types::{components::Scalar, Loggable};
use re_types::{components::Scalar, Component, Loggable};

/// The memory overhead of storing many scalars in the store.
#[test]
Expand Down
2 changes: 1 addition & 1 deletion crates/store/re_chunk_store/tests/reads.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use re_log_types::{
};
use re_types::testing::{build_some_large_structs, LargeStruct};
use re_types::ComponentNameSet;
use re_types_core::{ComponentName, Loggable as _};
use re_types_core::{Component as _, ComponentName};

// ---

Expand Down
2 changes: 1 addition & 1 deletion crates/store/re_chunk_store/tests/stats.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use re_log_types::{
example_components::{MyColor, MyPoint},
EntityPath, Timeline,
};
use re_types_core::Loggable as _;
use re_types_core::Component as _;

#[test]
fn stats() -> anyhow::Result<()> {
Expand Down
4 changes: 2 additions & 2 deletions crates/store/re_dataframe/src/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,7 @@ impl<E: StorageEngineLike> QueryHandle<E> {
(!chunk.is_empty()).then_some(chunk)
}

use re_types_core::Loggable as _;
use re_types_core::Component as _;
let component_names = [re_types_core::components::ClearIsRecursive::name()];

// All unique entity paths present in the view contents.
Expand Down Expand Up @@ -1296,7 +1296,7 @@ mod tests {
};
use re_query::StorageEngine;
use re_types::components::ClearIsRecursive;
use re_types_core::Loggable as _;
use re_types_core::Component as _;

use crate::{QueryCache, QueryEngine};

Expand Down
2 changes: 1 addition & 1 deletion crates/store/re_format_arrow/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ fn get_custom_display<'a, F: std::fmt::Write + 'a>(

if let Some(DataType::Extension(name, _, _)) = datatype {
// TODO(#1775): This should be registered dynamically.
if name.as_str() == Tuid::name() {
if name.as_str() == Tuid::NAME {
return Box::new(|w, index| {
if let Some(tuid) = parse_tuid(array, index) {
w.write_fmt(format_args!("{tuid}"))
Expand Down
Loading

0 comments on commit 015888f

Please sign in to comment.