alamb commented on code in PR #3514:
URL: https://github.com/apache/arrow-rs/pull/3514#discussion_r1090659093
##########
arrow-cast/src/display.rs:
##########
@@ -524,6 +721,22 @@ pub fn array_value_to_string(
}
}
+pub fn temporal_array_value_to_string(
Review Comment:
Can we please add some doc comments about what this does (specifically what
`DataTypes` it accepts) and what the format is used for?
I realize you didn't add this function so we can do so as a follow on PR I
think
##########
arrow-cast/src/display.rs:
##########
@@ -28,6 +28,13 @@ use arrow_array::*;
use arrow_buffer::ArrowNativeType;
use arrow_schema::*;
use chrono::prelude::SecondsFormat;
+use chrono::{DateTime, Utc};
+
+fn invalid_cast_error(dt: &str, col_idx: usize, row_idx: usize) -> ArrowError {
+ ArrowError::CastError(format!(
+ "Cannot cast to {dt} at col index: {col_idx} row index: {row_idx}"
Review Comment:
I like the addition of column index to the error message 👍
##########
arrow-csv/src/writer.rs:
##########
@@ -177,88 +158,74 @@ impl<W: Write> Writer<W> {
DataType::UInt16 => write_primitive_value::<UInt16Type>(col,
row_index),
DataType::UInt32 => write_primitive_value::<UInt32Type>(col,
row_index),
DataType::UInt64 => write_primitive_value::<UInt64Type>(col,
row_index),
- DataType::Boolean => {
- let c =
col.as_any().downcast_ref::<BooleanArray>().unwrap();
- c.value(row_index).to_string()
- }
- DataType::Utf8 => {
- let c =
col.as_any().downcast_ref::<StringArray>().unwrap();
- c.value(row_index).to_owned()
- }
- DataType::LargeUtf8 => {
- let c =
col.as_any().downcast_ref::<LargeStringArray>().unwrap();
- c.value(row_index).to_owned()
- }
- DataType::Date32 => {
- write_temporal_value!(
- col,
- Date32Array,
- &self.date_format,
- col_index,
- row_index,
- value_as_date,
- "Date32"
- )
- }
- DataType::Date64 => {
- write_temporal_value!(
+ DataType::Boolean => array_value_to_string(col,
row_index)?.to_string(),
Review Comment:
👍 that certainly looks better
##########
arrow-cast/src/display.rs:
##########
@@ -133,57 +140,176 @@ macro_rules! make_string_interval_month_day_nano {
}
macro_rules! make_string_date {
- ($array_type:ty, $column: ident, $row: ident) => {{
- let array = $column.as_any().downcast_ref::<$array_type>().unwrap();
+ ($array_type:ty, $dt:expr, $column: ident, $col_idx:ident, $row_idx:
ident) => {{
+ Ok($column
+ .as_any()
+ .downcast_ref::<$array_type>()
+ .ok_or_else(|| invalid_cast_error($dt, $col_idx, $row_idx))?
+ .value_as_date($row_idx)
+ .ok_or_else(|| invalid_cast_error($dt, $col_idx, $row_idx))?
+ .to_string())
+ }};
+}
- Ok(array
- .value_as_date($row)
- .map(|d| d.to_string())
- .unwrap_or_else(|| "ERROR CONVERTING DATE".to_string()))
Review Comment:
does this change the behavior with invalid dates (.e.g it will error rather
than successfully write out "ERROR CONVERTING DATE")?
##########
arrow-csv/src/writer.rs:
##########
@@ -463,6 +384,19 @@ impl WriterBuilder {
self
}
+ /// Use RFC3339 format for date/time/timestamps by clearing all
Review Comment:
👍
--
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]
