Skip to content

Commit

Permalink
resolve clippy::unwrap_used in re_types_core (#8732)
Browse files Browse the repository at this point in the history
### Related

Part of: #6330

### What

- remove `#![allow(clippy::unwrap_used)]` & TODO comment from
`crates/store/re_types_core/src/lib.rs`
- add focused `#[allow(clippy::unwrap_used)]` directives to
`Tuid::from_arrow` fn in `crates/store/re_types_core/src/tuid.rs`

__RATIONALE:__

Given the following comment in `Tuid::from_arrow`:

```rs
// NOTE: Unwrap is safe everywhere below, datatype is checked above.
```

I figured it would be easiest & cleanest to simply add the focused
`#[allow(clippy::unwrap_used)]` directives into the `Tuid::from_arrow`
fn. I did consider some other possible refactoring but decided against
this.

Incidentally when I did try injecting a couple mutations, `cargo test -p
re_types_core --all-features` continued to succeed with these:

```diff
diff --git a/crates/store/re_types_core/src/tuid.rs b/crates/store/re_types_core/src/tuid.rs
index 155340fac4..4fdbb8f696 100644
--- a/crates/store/re_types_core/src/tuid.rs
+++ b/crates/store/re_types_core/src/tuid.rs
@@ -68,15 +68,6 @@ impl Loggable for Tuid {
     }
 
     fn from_arrow(array: &dyn ::arrow::array::Array) -> crate::DeserializationResult<Vec<Self>> {
-        let expected_datatype = Self::arrow_datatype();
-        let actual_datatype = array.data_type();
-        if actual_datatype != &expected_datatype {
-            return Err(DeserializationError::datatype_mismatch(
-                expected_datatype,
-                actual_datatype.clone(),
-            ));
-        }
-
         // NOTE: Unwrap is safe everywhere below, datatype is checked above.
         // NOTE: We don't even look at the validity, our datatype says we don't care.
 
@@ -89,13 +80,6 @@ impl Loggable for Tuid {
         let (time_ns_index, inc_index) = {
             let mut time_ns_index = None;
             let mut inc_index = None;
-            for (i, field) in array.fields().iter().enumerate() {
-                if field.name() == "time_ns" {
-                    time_ns_index = Some(i);
-                } else if field.name() == "inc" {
-                    inc_index = Some(i);
-                }
-            }
             #[allow(clippy::unwrap_used)]
             (time_ns_index.unwrap(), inc_index.unwrap())
         };
```
  • Loading branch information
brody4hire authored Jan 22, 2025
1 parent 03f758c commit 343da4a
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 3 deletions.
3 changes: 0 additions & 3 deletions crates/store/re_types_core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@
#![doc = document_features::document_features!()]
//!
// TODO(#6330): remove unwrap()
#![allow(clippy::unwrap_used)]

// ---

/// Number of decimals shown for all float display methods.
Expand Down
3 changes: 3 additions & 0 deletions crates/store/re_types_core/src/tuid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ impl Loggable for Tuid {
// NOTE: Unwrap is safe everywhere below, datatype is checked above.
// NOTE: We don't even look at the validity, our datatype says we don't care.

#[allow(clippy::unwrap_used)]
let array = array.downcast_array_ref::<StructArray>().unwrap();

// TODO(cmc): Can we rely on the fields ordering from the datatype? I would assume not
Expand All @@ -95,10 +96,12 @@ impl Loggable for Tuid {
inc_index = Some(i);
}
}
#[allow(clippy::unwrap_used)]
(time_ns_index.unwrap(), inc_index.unwrap())
};

let get_buffer = |field_index: usize| {
#[allow(clippy::unwrap_used)]
array.columns()[field_index]
.downcast_array_ref::<UInt64Array>()
.unwrap()
Expand Down

0 comments on commit 343da4a

Please sign in to comment.