alamb commented on code in PR #8829:
URL: https://github.com/apache/arrow-rs/pull/8829#discussion_r2539713099
##########
arrow-cast/src/display.rs:
##########
@@ -169,10 +217,161 @@ impl<'a> FormatOptions<'a> {
Self { types_info, ..self }
}
- /// Returns true if type info should be included in visual representation
of batches
+ /// Overrides the [`ArrayFormatterFactory`] used to instantiate custom
[`ArrayFormatter`]s.
+ pub const fn with_formatter_factory(
+ self,
+ formatter_factory: &'a dyn ArrayFormatterFactory,
+ ) -> Self {
+ Self {
+ formatter_factory: Some(formatter_factory),
+ ..self
+ }
+ }
+
+ /// Removes the [`ArrayFormatterFactory`] used to instantiate custom
[`ArrayFormatter`]s. This
+ /// will cause pretty-printers to use the default [`ArrayFormatter`]s.
+ pub const fn without_formatter_factory(self) -> Self {
+ Self {
+ formatter_factory: None,
+ ..self
+ }
+ }
+
+ /// Returns whether formatting errors should be written to the output
instead of being converted
Review Comment:
👍 thank you -- these are nice additions
##########
arrow-cast/src/display.rs:
##########
@@ -53,7 +54,12 @@ pub enum DurationFormat {
/// By default nulls are formatted as `""` and temporal types formatted
/// according to RFC3339
///
-#[derive(Debug, Clone, PartialEq, Eq, Hash)]
+/// # Equality
Review Comment:
👍
##########
arrow-cast/src/pretty.rs:
##########
@@ -1283,4 +1428,247 @@ mod tests {
let actual: Vec<&str> = iso.lines().collect();
assert_eq!(expected_iso, actual, "Actual result:\n{iso}");
}
+
+ //
+ // Custom Formatting
+ //
+
+ /// The factory that will create the [`ArrayFormatter`]s.
+ struct TestFormatters {}
+
+ impl ArrayFormatterFactory for TestFormatters {
+ fn create_display_index<'formatter>(
+ &self,
+ array: &'formatter dyn Array,
+ options: &'formatter FormatOptions<'formatter>,
+ field: Option<&'formatter Field>,
+ ) -> Result<Option<ArrayFormatter<'formatter>>, ArrowError> {
+ if field
+ .map(|f| f.extension_type_name() == Some("my_money"))
+ .unwrap_or(false)
+ {
+ // We assume that my_money always is an Int32.
+ let array = array.as_primitive();
+ let display_index = Box::new(MyMoneyFormatter { array, options
});
+ return Ok(Some(ArrayFormatter::new(display_index,
options.safe)));
+ }
+
+ if array.data_type() == &DataType::Int32 {
+ // We assume that my_money always is an Int32.
+ let array = array.as_primitive();
+ let display_index = Box::new(MyInt32Formatter { array, options
});
+ return Ok(Some(ArrayFormatter::new(display_index,
options.safe)));
+ }
+
+ Ok(None)
+ }
+ }
+
+ /// The actual formatter
+ struct MyMoneyFormatter<'a> {
+ array: &'a Int32Array,
+ options: &'a FormatOptions<'a>,
+ }
+
+ impl<'a> DisplayIndex for MyMoneyFormatter<'a> {
+ fn write(&self, idx: usize, f: &mut dyn Write) ->
crate::display::FormatResult {
+ match self.array.is_valid(idx) {
+ true => write!(f, "{} €", self.array.value(idx))?,
+ false => write!(f, "{}", self.options.null)?,
+ }
+
+ Ok(())
+ }
+ }
+
+ /// The actual formatter
+ struct MyInt32Formatter<'a> {
+ array: &'a Int32Array,
+ options: &'a FormatOptions<'a>,
+ }
+
+ impl<'a> DisplayIndex for MyInt32Formatter<'a> {
+ fn write(&self, idx: usize, f: &mut dyn Write) ->
crate::display::FormatResult {
+ match self.array.is_valid(idx) {
+ true => write!(f, "{} (32-Bit)", self.array.value(idx))?,
+ false => write!(f, "{}", self.options.null)?,
+ }
+
+ Ok(())
+ }
+ }
+
+ #[test]
+ fn test_format_batches_with_custom_formatters() {
+ // define a schema.
+ let options = FormatOptions::new().with_null("<NULL>");
+ let money_metadata = HashMap::from([(
+ extension::EXTENSION_TYPE_NAME_KEY.to_owned(),
+ "my_money".to_owned(),
+ )]);
+ let schema = Arc::new(Schema::new(vec![
+ Field::new("income", DataType::Int32,
true).with_metadata(money_metadata.clone()),
+ ]));
+
+ // define data.
+ let batch = RecordBatch::try_new(
+ schema,
+ vec![Arc::new(array::Int32Array::from(vec![
+ Some(1),
+ None,
+ Some(10),
+ Some(100),
+ ]))],
+ )
+ .unwrap();
+
+ let mut buf = String::new();
+ write!(
+ &mut buf,
+ "{}",
+ pretty_format_batches_with_options_and_formatters(
+ &[batch],
+ &options,
+ Some(&TestFormatters {})
+ )
+ .unwrap()
+ )
+ .unwrap();
+
+ let s = [
+ "+--------+",
+ "| income |",
+ "+--------+",
+ "| 1 € |",
+ "| <NULL> |",
+ "| 10 € |",
+ "| 100 € |",
+ "+--------+",
+ ];
+ let expected = s.join("\n");
+ assert_eq!(expected, buf);
+ }
+
+ #[test]
+ fn
test_format_batches_with_custom_formatters_custom_schema_overrules_batch_schema()
{
+ // define a schema.
+ let options = FormatOptions::new();
+ let money_metadata = HashMap::from([(
+ extension::EXTENSION_TYPE_NAME_KEY.to_owned(),
+ "my_money".to_owned(),
+ )]);
+ let schema = Arc::new(Schema::new(vec![
+ Field::new("income", DataType::Int32,
true).with_metadata(money_metadata.clone()),
+ ]));
+
+ // define data.
+ let batch = RecordBatch::try_new(
+ schema,
+ vec![Arc::new(array::Int32Array::from(vec![
+ Some(1),
+ None,
+ Some(10),
+ Some(100),
+ ]))],
+ )
+ .unwrap();
+
+ let mut buf = String::new();
+ write!(
+ &mut buf,
+ "{}",
+ create_table(
+ // No metadata compared to
test_format_batches_with_custom_formatters
+ Some(Arc::new(Schema::new(vec![Field::new(
+ "income",
+ DataType::Int32,
+ true
+ ),]))),
+ &[batch],
+ &options,
+ Some(&TestFormatters {})
+ )
+ .unwrap()
+ )
+ .unwrap();
+
+ // No € formatting as in test_format_batches_with_custom_formatters
+ let s = [
+ "+--------------+",
Review Comment:
this makes sense
##########
arrow-cast/src/display.rs:
##########
@@ -352,7 +361,8 @@ impl From<ArrowError> for FormatError {
}
/// [`Display`] but accepting an index
-trait DisplayIndex {
+pub trait DisplayIndex {
+ /// Write the value of the underlying array at `idx` to `f`.
fn write(&self, idx: usize, f: &mut dyn Write) -> FormatResult;
Review Comment:
I recommend waiting until we have a usecase before we expand the API surface
##########
arrow-cast/src/display.rs:
##########
@@ -169,10 +217,161 @@ impl<'a> FormatOptions<'a> {
Self { types_info, ..self }
}
- /// Returns true if type info should be included in visual representation
of batches
+ /// Overrides the [`ArrayFormatterFactory`] used to instantiate custom
[`ArrayFormatter`]s.
+ pub const fn with_formatter_factory(
+ self,
+ formatter_factory: &'a dyn ArrayFormatterFactory,
+ ) -> Self {
Review Comment:
I think it is more common in the arrow APIs to pass the Option instead of
having both a with and without method
```suggestion
/// Overrides the [`ArrayFormatterFactory`] used to instantiate custom
[`ArrayFormatter`]s.
pub const fn with_formatter_factory(
self,
formatter_factory: Option<&'a dyn ArrayFormatterFactory>,
) -> Self {
```
Then you can remove the other method
--
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]
