alamb commented on code in PR #6006:
URL: https://github.com/apache/arrow-rs/pull/6006#discussion_r1667746254
##########
arrow-cast/src/display.rs:
##########
@@ -654,73 +654,189 @@ impl<'a> DisplayIndex for &'a
PrimitiveArray<IntervalYearMonthType> {
let years = (interval / 12_f64).floor();
let month = interval - (years * 12_f64);
- write!(
- f,
- "{years} years {month} mons 0 days 0 hours 0 mins 0.00 secs",
- )?;
+ write!(f, "{years} years {month} mons",)?;
Ok(())
}
}
impl<'a> DisplayIndex for &'a PrimitiveArray<IntervalDayTimeType> {
fn write(&self, idx: usize, f: &mut dyn Write) -> FormatResult {
let value = self.value(idx);
+ let mut first_part = true;
- let secs = value.milliseconds / 1_000;
+ if value.days != 0 {
+ write!(f, "{} days", value.days)?;
+ first_part = false;
+ }
+
+ if value.milliseconds != 0 {
+ let millis_fmt = MillisecondsFormatter {
+ milliseconds: value.milliseconds,
+ first_part,
+ };
+
+ f.write_fmt(format_args!("{millis_fmt}"))?;
+ }
+
+ Ok(())
+ }
+}
+
+impl<'a> DisplayIndex for &'a PrimitiveArray<IntervalMonthDayNanoType> {
+ fn write(&self, idx: usize, f: &mut dyn Write) -> FormatResult {
+ let value = self.value(idx);
+ let mut first_part = true;
+
+ if value.months != 0 {
+ write!(f, "{} mons", value.months)?;
+ first_part = false;
+ }
+
+ if value.days != 0 {
+ if first_part {
+ write!(f, "{} days", value.days)?;
+ first_part = false;
+ } else {
+ write!(f, " {} days", value.days)?;
+ }
+ }
+
+ if value.nanoseconds != 0 {
+ let nano_fmt = NanosecondsFormatter {
+ nanoseconds: value.nanoseconds,
+ first_part,
+ };
+ f.write_fmt(format_args!("{nano_fmt}"))?;
+ }
+
+ Ok(())
+ }
+}
+
+struct NanosecondsFormatter {
+ nanoseconds: i64,
+ first_part: bool,
+}
+
+impl Display for NanosecondsFormatter {
+ fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+ let mut first_part = self.first_part;
+
+ let secs = self.nanoseconds / 1_000_000_000;
let mins = secs / 60;
let hours = mins / 60;
let secs = secs - (mins * 60);
let mins = mins - (hours * 60);
- let milliseconds = value.milliseconds % 1_000;
+ let nanoseconds = self.nanoseconds % 1_000_000_000;
- let secs_sign = if secs < 0 || milliseconds < 0 {
- "-"
- } else {
- ""
- };
+ if hours != 0 {
+ if first_part {
+ write!(f, "{} hours", hours)?;
+ first_part = false;
+ } else {
+ write!(f, " {} hours", hours)?;
+ }
+ }
+
+ if mins != 0 {
+ if first_part {
+ write!(f, "{} mins", mins)?;
+ first_part = false;
+ } else {
+ write!(f, " {} mins", mins)?;
+ }
+ }
+
+ if secs != 0 || nanoseconds != 0 {
+ let secs_sign = if secs < 0 || nanoseconds < 0 { "-" } else { "" };
+
+ if first_part {
+ write!(
+ f,
+ "{}{}.{:09} secs",
+ secs_sign,
+ secs.abs(),
+ nanoseconds.abs()
+ )?;
+ } else {
+ write!(
+ f,
+ " {}{}.{:09} secs",
+ secs_sign,
+ secs.abs(),
+ nanoseconds.abs()
+ )?;
+ }
Review Comment:
You could probably avoid some repetition here if with a constant str.
Something like
```suggestion
let prefix = if first_part { " " } else { "" }
write!(
f,
"{prefix}{}{}.{:09} secs",
secs_sign,
secs.abs(),
nanoseconds.abs()
)
```
However that might be slightly slower now it has to check `prefix`. You
could possibly even use a field like `prefix: &'static str` instead of `bool
first_part` if you wanted to make it even more concise.
A similar comment applies to the other clauses
I don't think this is required, I just wanted to mention it
--
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]
