Skip to content
42 changes: 26 additions & 16 deletions arrow-cast/src/display.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,23 +57,23 @@ pub enum DurationFormat {
pub struct FormatOptions<'a> {
/// If set to `true` any formatting errors will be written to the output
/// instead of being converted into a [`std::fmt::Error`]
safe: bool,
pub safe: bool,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make fields in FormatOptions public. This is necessary as the custom ArrayFormatter must also have access to the formatting options. (see in the test)

Rather than making these public, what about adding accessors for them? I think that would make it easier to change the underlying implementation in the future without causing breaking API changes

I think by making all these fields pub it means people can construct format options explicitly like

let options = FormatOptions {
  safe: false,
  null: "", 
...
};

So adding any new field to the struct will be a breaking API change

If we keep them private fields, then we can add new fields without breaking existing peopel

/// Format string for nulls
null: &'a str,
pub null: &'a str,
/// Date format for date arrays
date_format: TimeFormat<'a>,
pub date_format: TimeFormat<'a>,
/// Format for DateTime arrays
datetime_format: TimeFormat<'a>,
pub datetime_format: TimeFormat<'a>,
/// Timestamp format for timestamp arrays
timestamp_format: TimeFormat<'a>,
pub timestamp_format: TimeFormat<'a>,
/// Timestamp format for timestamp with timezone arrays
timestamp_tz_format: TimeFormat<'a>,
pub timestamp_tz_format: TimeFormat<'a>,
/// Time format for time arrays
time_format: TimeFormat<'a>,
pub time_format: TimeFormat<'a>,
/// Duration format
duration_format: DurationFormat,
pub duration_format: DurationFormat,
/// Show types in visual representation batches
types_info: bool,
pub types_info: bool,
}

impl Default for FormatOptions<'_> {
Expand Down Expand Up @@ -170,6 +170,10 @@ impl<'a> FormatOptions<'a> {
}

/// Returns true if type info should be included in visual representation of batches
#[deprecated(
since = "58.0.0",
note = "Directly access the `types_info` field instead.`"
)]
pub const fn types_info(&self) -> bool {
self.types_info
}
Expand Down Expand Up @@ -272,14 +276,16 @@ pub struct ArrayFormatter<'a> {
}

impl<'a> ArrayFormatter<'a> {
/// Returns an [`ArrayFormatter`] using the provided formatter.
pub fn new(format: Box<dyn DisplayIndex + 'a>, safe: bool) -> Self {
Self { format, safe }
}

/// Returns an [`ArrayFormatter`] that can be used to format `array`
///
/// This returns an error if an array of the given data type cannot be formatted
pub fn try_new(array: &'a dyn Array, options: &FormatOptions<'a>) -> Result<Self, ArrowError> {
Ok(Self {
format: make_formatter(array, options)?,
safe: options.safe,
})
Ok(Self::new(make_formatter(array, options)?, options.safe))
}

/// Returns a [`ValueFormatter`] that implements [`Display`] for
Expand Down Expand Up @@ -332,12 +338,15 @@ fn make_formatter<'a>(
}

/// Either an [`ArrowError`] or [`std::fmt::Error`]
enum FormatError {
pub enum FormatError {
/// An error occurred while formatting the array
Format(std::fmt::Error),
/// An Arrow error occurred while formatting the array.
Arrow(ArrowError),
}

type FormatResult = Result<(), FormatError>;
/// The result of formatting an array element via [`DisplayIndex::write`].
pub type FormatResult = Result<(), FormatError>;

impl From<std::fmt::Error> for FormatError {
fn from(value: std::fmt::Error) -> Self {
Expand All @@ -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;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should add a len method to make the trait more complete (?).

}

Expand Down
Loading
Loading