scovich commented on code in PR #9563:
URL: https://github.com/apache/arrow-rs/pull/9563#discussion_r3003422083
##########
parquet-variant/src/variant.rs:
##########
@@ -1224,31 +1251,28 @@ impl<'m, 'v> Variant<'m, 'v> {
/// let v2 = Variant::from(std::f64::consts::PI);
/// assert_eq!(v2.as_f32(), Some(std::f32::consts::PI));
///
- /// // and from integers with no more than 24 bits of precision
- /// let v3 = Variant::from(16777215i64);
- /// assert_eq!(v3.as_f32(), Some(16777215.0));
+ /// // and from boolean variant
+ /// let v3 = Variant::BooleanTrue;
+ /// assert_eq!(v3.as_f32(), Some(1.0));
+ ///
+ /// // and return inf if overflow
+ /// let v4 = Variant::from(f64::MAX);
+ /// assert_eq!(v4.as_f32(), Some(f32::INFINITY));
///
/// // but not from other variants
- /// let v4 = Variant::from("hello!");
- /// assert_eq!(v4.as_f32(), None);
+ /// let v5 = Variant::from("hello!");
+ /// assert_eq!(v5.as_f32(), None);
/// ```
#[allow(clippy::cast_possible_truncation)]
Review Comment:
Probably don't need this any more since the truncation is hidden behind too
many layers for clippy to "see" now?
##########
parquet-variant/src/variant.rs:
##########
@@ -1527,7 +1543,8 @@ impl From<u8> for Variant<'_, '_> {
if let Ok(value) = i8::try_from(value) {
Variant::Int8(value)
} else {
- Variant::Int16(i16::from(value))
+ // It will always fit in i16 because u8 max is 255 and i16 max is
32767
+ Variant::Int16(num_cast(value).unwrap())
Review Comment:
nit: Seems simpler to just say
```suggestion
Variant::Int16(num_cast(value).unwrap()) // u8 -> i16 is
infallible
```
(more below)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
