Refactor usage of unwrap in re_entity_db, re_format_arrow and re_log_…

…types

crates/re_entity_db/src/entity_tree.rs

@@ -362,8 +362,7 @@ impl EntityTree {
             if cell.component_name() == ClearIsRecursive::name() {
                 let is_recursive = cell
                     .try_to_native_mono::<ClearIsRecursive>()
-                    .unwrap()
-                    .map_or(false, |settings| settings.0);
+                    .map_or(false, |opt| opt.map_or(false, |settings| settings.0));
 
                 self.on_added_clear(clear_cascade, store_diff, is_recursive);
             }

crates/re_entity_db/src/lib.rs

@@ -4,9 +4,6 @@
 #![doc = document_features::document_features!()]
 //!
 
-// TODO(#3408): remove unwrap()
-#![allow(clippy::unwrap_used)]
-
 pub mod entity_db;
 pub mod entity_properties;
 pub mod entity_tree;

crates/re_format_arrow/src/lib.rs

@@ -1,8 +1,5 @@
 //! Formatting for tables of Arrow arrays
 
-// TODO(#3408): remove unwrap()
-#![allow(clippy::unwrap_used)]
-
 use std::fmt::Formatter;
 
 use arrow2::{
@@ -247,7 +244,10 @@ where
             .iter()
             .map(|disp| {
                 let mut string = String::new();
-                (disp)(&mut string, row).unwrap();
+                if (disp)(&mut string, row).is_err() {
+                    // Seems to be okay to silently ignore errors here, but reset the string just in case
+                    string.clear();
+                }
                 let chars: Vec<_> = string.chars().collect();
                 if chars.len() > WIDTH_UPPER_BOUNDARY as usize {
                     Cell::new(

crates/re_log_types/src/arrow_msg.rs

@@ -187,6 +187,7 @@ impl<'de> serde::Deserialize<'de> for ArrowMsg {
                             chunks.len()
                         )));
                     }
+                    #[allow(clippy::unwrap_used)] // is_empty check above
                     let chunk = chunks.into_iter().next().unwrap();
 
                     Ok(ArrowMsg {

crates/re_log_types/src/data_cell.rs

@@ -231,6 +231,8 @@ impl DataCell {
     where
         C: Component + Clone + 'a,
     {
+        // NOTE: see function description why it's okay here
+        #[allow(clippy::unwrap_used)]
         Self::try_from_native(values).unwrap()
     }
 
@@ -246,6 +248,8 @@ impl DataCell {
     where
         C: Component + Clone + 'a,
     {
+        // NOTE: see function description why it's okay here
+        #[allow(clippy::unwrap_used)]
         Self::try_from_native_sparse(values).unwrap()
     }
 
@@ -295,6 +299,8 @@ impl DataCell {
     /// See [`Self::try_from_arrow`] for the fallible alternative.
     #[inline]
     pub fn from_arrow(name: ComponentName, values: Box<dyn arrow2::array::Array>) -> Self {
+        // NOTE: see function description why it's okay here
+        #[allow(clippy::unwrap_used)]
         Self::try_from_arrow(name, values).unwrap()
     }
 
@@ -333,6 +339,8 @@ impl DataCell {
     /// See [`Self::try_from_arrow_empty`] for a fallible alternative.
     #[inline]
     pub fn from_arrow_empty(name: ComponentName, datatype: arrow2::datatypes::DataType) -> Self {
+        // NOTE: see function description why it's okay here
+        #[allow(clippy::unwrap_used)]
         Self::try_from_arrow_empty(name, datatype).unwrap()
     }
 
@@ -381,7 +389,7 @@ impl DataCell {
 
         let datatype = ListArray::<i32>::default_datatype(datatype);
         let offsets = Offsets::try_from_lengths(std::iter::once(self.num_instances() as usize))
-            .unwrap()
+            .unwrap_or_default()
             .into();
         let validity = None;
 
@@ -436,6 +444,8 @@ impl DataCell {
     /// See [`Self::try_to_native`] for a fallible alternative.
     #[inline]
     pub fn to_native<'a, C: Component + 'a>(&'a self) -> Vec<C> {
+        // NOTE: see function description why it's okay here
+        #[allow(clippy::unwrap_used)]
         self.try_to_native().unwrap()
     }
 
@@ -456,6 +466,8 @@ impl DataCell {
     /// See [`Self::try_to_native_opt`] for a fallible alternative.
     #[inline]
     pub fn to_native_opt<'a, C: Component + 'a>(&'a self) -> Vec<Option<C>> {
+        // NOTE: see function description why it's okay here
+        #[allow(clippy::unwrap_used)]
         self.try_to_native_opt().unwrap()
     }
 }
@@ -510,6 +522,7 @@ impl DataCell {
 
         fn is_sorted_and_unique_primitive<T: NativeType + PartialOrd>(arr: &dyn Array) -> bool {
             // NOTE: unwrap cannot fail, checked by caller just below
+            #[allow(clippy::unwrap_used)]
             let values = arr.as_any().downcast_ref::<PrimitiveArray<T>>().unwrap();
             values.values().windows(2).all(|v| v[0] < v[1])
         }

crates/re_log_types/src/data_table.rs

@@ -437,7 +437,9 @@ impl DataTable {
             for cell in cells.0 {
                 let component = cell.component_name();
                 // NOTE: unwrap cannot fail, all arrays pre-allocated above.
-                columns.get_mut(&component).unwrap()[i] = Some(cell);
+                #[allow(clippy::unwrap_used)]
+                let column = columns.get_mut(&component).unwrap();
+                column[i] = Some(cell);
             }
         }
 
@@ -790,10 +792,10 @@ impl DataTable {
             let offsets = Offsets::try_from_lengths(column.iter().map(|cell| {
                 cell.as_ref()
                     .map_or(0, |cell| cell.num_instances() as usize)
-            }))
+            }));
             // NOTE: cannot fail, `data` has as many instances as `column`
-            .unwrap()
-            .into();
+            #[allow(clippy::unwrap_used)]
+            let offsets = offsets.unwrap().into();
 
             #[allow(clippy::from_iter_instead_of_collect)]
             let validity = Bitmap::from_iter(column.iter().map(|cell| cell.is_some()));
@@ -911,12 +913,14 @@ impl DataTable {
         };
 
         // NOTE: the unwrappings cannot fail since control_index() makes sure the index is valid
+        #[allow(clippy::unwrap_used)]
         let col_row_id = RowId::from_arrow(
             chunk
                 .get(control_index(RowId::name().as_str())?)
                 .unwrap()
                 .as_ref(),
         )?;
+        #[allow(clippy::unwrap_used)]
         let col_entity_path = EntityPath::from_arrow(
             chunk
                 .get(control_index(EntityPath::name().as_str())?)
@@ -976,10 +980,11 @@ impl DataTable {
             }
         };
 
+        // NOTE: unwrap cannot fail here, datatype checked above
+        #[allow(clippy::unwrap_used)]
         let col_time = column
             .as_any()
             .downcast_ref::<PrimitiveArray<i64>>()
-            // NOTE: cannot fail, datatype checked above
             .unwrap();
         let col_time: TimeOptVec = col_time.into_iter().map(|time| time.copied()).collect();
 
@@ -1100,7 +1105,8 @@ impl DataTable {
         rows_by_timeline2.remove(&Timeline::log_time());
 
         for (timeline, rows1) in &mut rows_by_timeline1 {
-            let rows2 = rows_by_timeline2.get_mut(timeline).unwrap(); // safe
+            #[allow(clippy::unwrap_used)] // safe, the keys are checked above
+            let rows2 = rows_by_timeline2.get_mut(timeline).unwrap();
 
             // NOTE: We need both sets of rows to follow a common natural order for the comparison
             // to make sense.
@@ -1196,30 +1202,29 @@ impl DataTable {
                             c2.component_name(),
                         ));
 
-                        fn cell_to_bytes(cell: DataCell) -> Vec<u8> {
+                        fn cell_to_bytes(cell: DataCell) -> anyhow::Result<Vec<u8>> {
                             let row = DataRow::from_cells1(
                                 RowId::ZERO,
                                 "cell",
                                 TimePoint::default(),
                                 cell,
-                            )
-                            .unwrap();
+                            )?;
                             let table = DataTable::from_rows(TableId::ZERO, [row]);
 
-                            let msg = table.to_arrow_msg().unwrap();
+                            let msg = table.to_arrow_msg()?;
 
                             use arrow2::io::ipc::write::StreamWriter;
                             let mut buf = Vec::<u8>::new();
                             let mut writer = StreamWriter::new(&mut buf, Default::default());
-                            writer.start(&msg.schema, None).unwrap();
-                            writer.write(&msg.chunk, None).unwrap();
-                            writer.finish().unwrap();
+                            writer.start(&msg.schema, None)?;
+                            writer.write(&msg.chunk, None)?;
+                            writer.finish()?;
 
-                            buf
+                            Ok(buf)
                         }
 
-                        let c1_bytes = cell_to_bytes(c1.clone());
-                        let c2_bytes = cell_to_bytes(c2.clone());
+                        let c1_bytes = cell_to_bytes(c1.clone())?;
+                        let c2_bytes = cell_to_bytes(c2.clone())?;
 
                         size_mismatches.push(format!(
                             "IPC size is {} vs {} bytes",
@@ -1266,6 +1271,8 @@ impl DataTable {
 /// Crafts a simple but interesting [`DataTable`].
 #[cfg(not(target_arch = "wasm32"))]
 impl DataTable {
+    /// NOTE: because everything here is predetermined and there is no input we assume it's safe here
+    #[allow(clippy::unwrap_used)]
     pub fn example(timeless: bool) -> DataTable {
         use crate::{
             example_components::{MyColor, MyLabel, MyPoint},

crates/re_log_types/src/data_table_batcher.rs

@@ -405,6 +405,7 @@ impl DataTableBatcher {
     pub fn tables(&self) -> Receiver<DataTable> {
         // NOTE: `rx_tables` is only ever taken when the batcher as a whole is dropped, at which
         // point it is impossible to call this method.
+        #![allow(clippy::unwrap_used)]
         self.inner.rx_tables.clone().unwrap()
     }
 }

crates/re_log_types/src/example_components.rs

@@ -2,7 +2,7 @@
 
 use std::sync::Arc;
 
-use re_types_core::{Loggable, SizeBytes};
+use re_types_core::{DeserializationError, Loggable, SizeBytes};
 
 // ----------------------------------------------------------------------------
 
@@ -112,19 +112,25 @@ impl Loggable for MyPoint {
         let array = data
             .as_any()
             .downcast_ref::<arrow2::array::StructArray>()
-            .unwrap();
+            .ok_or(DeserializationError::downcast_error::<
+                arrow2::array::StructArray,
+            >())?;
 
         let x_array = array.values()[0].as_ref();
         let y_array = array.values()[1].as_ref();
 
         let xs = x_array
             .as_any()
             .downcast_ref::<arrow2::array::Float32Array>()
-            .unwrap();
+            .ok_or(DeserializationError::downcast_error::<
+                arrow2::array::Float32Array,
+            >())?;
         let ys = y_array
             .as_any()
             .downcast_ref::<arrow2::array::Float32Array>()
-            .unwrap();
+            .ok_or(DeserializationError::downcast_error::<
+                arrow2::array::Float32Array,
+            >())?;
 
         Ok(xs
             .values_iter()

crates/re_log_types/src/lib.rs

@@ -17,9 +17,6 @@
 //! e.g. the entity `foo/bar/baz` is has the transform that is the product of
 //! `foo.transform * foo/bar.transform * foo/bar/baz.transform`.
 
-// TODO(#3408): remove unwrap()
-#![allow(clippy::unwrap_used)]
-
 pub mod arrow_msg;
 pub mod example_components;
 pub mod hash;

crates/re_log_types/src/time.rs

@@ -1,3 +1,5 @@
+use anyhow::Result;
+use re_log::ResultExt;
 use std::ops::RangeInclusive;
 use time::{format_description::FormatItem, OffsetDateTime, UtcOffset};
 
@@ -77,25 +79,26 @@ impl Time {
         parsed_format: &Vec<FormatItem<'_>>,
         time_zone_for_timestamps: TimeZone,
     ) -> String {
-        match time_zone_for_timestamps {
-            TimeZone::Local => {
-                if let Ok(local_offset) = UtcOffset::current_local_offset() {
-                    // Return in the local timezone.
-                    let local_datetime = datetime.to_offset(local_offset);
-                    local_datetime.format(&parsed_format).unwrap()
-                } else {
-                    // Fallback to UTC.
-                    // Skipping `err` description from logging because as of writing it doesn't add much, see
-                    // https://github.com/time-rs/time/blob/v0.3.29/time/src/error/indeterminate_offset.rs
-                    re_log::warn_once!("Failed to access local timezone offset to UTC.");
-                    format!("{}Z", datetime.format(&parsed_format).unwrap())
+        let r = (|| -> Result<String, time::error::Format> {
+            match time_zone_for_timestamps {
+                TimeZone::Local => {
+                    if let Ok(local_offset) = UtcOffset::current_local_offset() {
+                        // Return in the local timezone.
+                        let local_datetime = datetime.to_offset(local_offset);
+                        local_datetime.format(&parsed_format)
+                    } else {
+                        // Fallback to UTC.
+                        // Skipping `err` description from logging because as of writing it doesn't add much, see
+                        // https://github.com/time-rs/time/blob/v0.3.29/time/src/error/indeterminate_offset.rs
+                        re_log::warn_once!("Failed to access local timezone offset to UTC.");
+                        Ok(format!("{}Z", datetime.format(&parsed_format)?))
+                    }
                 }
+                TimeZone::Utc => Ok(format!("{}Z", datetime.format(&parsed_format)?)),
+                TimeZone::UnixEpoch => datetime.format(&parsed_format),
             }
-            TimeZone::Utc => {
-                format!("{}Z", datetime.format(&parsed_format).unwrap())
-            }
-            TimeZone::UnixEpoch => datetime.format(&parsed_format).unwrap(),
-        }
+        })();
+        r.ok_or_log_error().unwrap_or_default()
     }
 
     /// Human-readable formatting
@@ -120,6 +123,7 @@ impl Time {
 
             let date_is_today = datetime.date() == OffsetDateTime::now_utc().date();
             let date_format = format!("[year]-[month]-[day] {time_format}");
+            #[allow(clippy::unwrap_used)] // date_format is okay!
             let parsed_format = if date_is_today {
                 time::format_description::parse(&time_format).unwrap()
             } else {
@@ -167,6 +171,7 @@ impl Time {
                         TimeZone::Utc | TimeZone::Local => "[hour]:[minute]:[second]",
                     }
                 };
+                #[allow(clippy::unwrap_used)] // time_format is okay!
                 let parsed_format = time::format_description::parse(time_format).unwrap();
 
                 return Self::time_string(datetime, &parsed_format, time_zone_for_timestamps);
@@ -202,10 +207,15 @@ impl Time {
         time_format: &str,
         time_zone_for_timestamps: TimeZone,
     ) -> Option<String> {
-        self.to_datetime().map(|datetime| {
-            let parsed_format = time::format_description::parse(time_format).unwrap();
-            Self::time_string(datetime, &parsed_format, time_zone_for_timestamps)
-        })
+        if let Some(datetime) = self.to_datetime() {
+            let parsed_format = time::format_description::parse(time_format).ok()?;
+            return Some(Self::time_string(
+                datetime,
+                &parsed_format,
+                time_zone_for_timestamps,
+            ));
+        }
+        None
     }
 
     #[inline]