zhuqi-lucas commented on code in PR #7534:
URL: https://github.com/apache/arrow-rs/pull/7534#discussion_r2100046767
##########
arrow-array/src/temporal_conversions.rs:
##########
@@ -215,28 +215,28 @@ pub(crate) fn split_second(v: i64, base: i64) -> (i64,
u32) {
(v.div_euclid(base), v.rem_euclid(base) as u32)
}
-/// converts a `i64` representing a `duration(s)` to [`Duration`]
+/// converts a `i64` representing a `duration(s)` to [`Option<Duration>`]
Review Comment:
Thank you @alamb , i tried this way, but it will make the following function
failed with match fmt with Some():
```rust
macro_rules! duration_display {
($convert:ident, $t:ty, $scale:tt) => {
impl<'a> DisplayIndexState<'a> for &'a PrimitiveArray<$t> {
type State = DurationFormat;
fn prepare(&self, options: &FormatOptions<'a>) ->
Result<Self::State, ArrowError> {
Ok(options.duration_format)
}
fn write(&self, fmt: &Self::State, idx: usize, f: &mut dyn
Write) -> FormatResult {
let v = self.value(idx);
match fmt {
DurationFormat::ISO8601 => match $convert(v) {
Some(td) => write!(f, "{}", td)?,
None => write!(f, "<invalid>")?,
},
DurationFormat::Pretty => match $convert(v) {
Some(_) => duration_fmt!(f, v, $scale)?,
None => write!(f, "<invalid>")?,
},
}
Ok(())
}
}
};
}
```
The original one will always pass option:
```rust
duration_display!(try_duration_s_to_duration, DurationSecondType, 0);
duration_display!(try_duration_ms_to_duration, DurationMillisecondType, 3);
duration_display!(try_duration_us_to_duration, DurationMicrosecondType, 6);
duration_display!(try_duration_ns_to_duration, DurationNanosecondType, 9);
```
Or, we can make the macro support both cases, if we want to remove option
for those don't need:
```rust
macro_rules! duration_display {
//–– converters that return Option<Duration> ––
($convert:ident, $t:ty, $scale:tt, option) => {
impl<'a> DisplayIndexState<'a> for &'a PrimitiveArray<$t> {
type State = DurationFormat;
fn prepare(&self, options: &FormatOptions<'a>) ->
Result<Self::State, ArrowError> {
Ok(options.duration_format)
}
fn write(&self, fmt: &Self::State, idx: usize, f: &mut dyn
Write) -> FormatResult {
let v = self.value(idx);
match fmt {
DurationFormat::ISO8601 => {
if let Some(td) = $convert(v) {
write!(f, "{}", td)?;
} else {
write!(f, "<invalid>")?;
}
}
DurationFormat::Pretty => {
if $convert(v).is_some() {
duration_fmt!(f, v, $scale)?;
} else {
write!(f, "<invalid>")?;
}
}
}
Ok(())
}
}
};
//–– converters that return plain Duration ––
($convert:ident, $t:ty, $scale:tt) => {
impl<'a> DisplayIndexState<'a> for &'a PrimitiveArray<$t> {
type State = DurationFormat;
fn prepare(&self, options: &FormatOptions<'a>) ->
Result<Self::State, ArrowError> {
Ok(options.duration_format)
}
fn write(&self, fmt: &Self::State, idx: usize, f: &mut dyn
Write) -> FormatResult {
let v = self.value(idx);
match fmt {
DurationFormat::ISO8601 => {
write!(f, "{}", $convert(v))?;
}
DurationFormat::Pretty => {
duration_fmt!(f, v, $scale)?;
}
}
Ok(())
}
}
};
}
```
And call it by:
```rust
// those four lines at the top of your file become:
// these two return Option<Duration>:
duration_display!(try_duration_s_to_duration, DurationSecondType, 0, option);
duration_display!(try_duration_ms_to_duration, DurationMillisecondType, 3,
option);
// these two return Duration:
duration_display!(duration_us_to_duration, DurationMicrosecondType, 6);
duration_display!(duration_ns_to_duration, DurationNanosecondType, 9);
```
--
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]
