[arrow-datafusion] branch main updated: Unify formatting of both groups and files up to 5 elements (#6637)

[al...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

alamb pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-datafusion.git


The following commit(s) were added to refs/heads/main by this push:
     new 708044ce92 Unify formatting of both groups and files up to 5 elements (#6637)
708044ce92 is described below

commit 708044ce92b08b351c241a85bc395d093cc7af7d
Author: Kirill Zaborsky <qr...@gmail.com>
AuthorDate: Sun Jun 11 21:25:39 2023 +0300

    Unify formatting of both groups and files up to 5 elements (#6637)
    
    This fixes inconsistency between rustdocs for Display of files and
    file groups by applying the same function formatting up to 5
    elements. Also adds corresponding unit tests.
---
 .../core/src/datasource/physical_plan/mod.rs       | 91 ++++++++++++++++------
 1 file changed, 68 insertions(+), 23 deletions(-)

diff --git a/datafusion/core/src/datasource/physical_plan/mod.rs b/datafusion/core/src/datasource/physical_plan/mod.rs
index 54b916788c..78a8c487a2 100644
--- a/datafusion/core/src/datasource/physical_plan/mod.rs
+++ b/datafusion/core/src/datasource/physical_plan/mod.rs
@@ -312,27 +312,19 @@ struct FileGroupsDisplay<'a>(&'a [Vec<PartitionedFile>]);
 
 impl<'a> Display for FileGroupsDisplay<'a> {
     fn fmt(&self, f: &mut Formatter) -> FmtResult {
-        let n_group = self.0.len();
-        let groups = if n_group == 1 { "group" } else { "groups" };
-        write!(f, "{{{n_group} {groups}: [")?;
+        let n_groups = self.0.len();
+        let groups = if n_groups == 1 { "group" } else { "groups" };
+        write!(f, "{{{n_groups} {groups}: [")?;
         // To avoid showing too many partitions
         let max_groups = 5;
-        for (idx, group) in self.0.iter().take(max_groups).enumerate() {
-            if idx > 0 {
-                write!(f, ", ")?;
-            }
-            write!(f, "{}", FileGroupDisplay(group))?;
-        }
-        // Remaining elements are showed as `...` (to indicate there is more)
-        if n_group > max_groups {
-            write!(f, ", ...")?;
-        }
-        write!(f, "]}}")?;
-        Ok(())
+        fmt_up_to_n_elements(self.0, max_groups, f, |group, f| {
+            write!(f, "{}", FileGroupDisplay(group))
+        })?;
+        write!(f, "]}}")
     }
 }
 
-/// A wrapper to customize partitioned file display
+/// A wrapper to customize partitioned group of files display
 ///
 /// Prints in the format:
 /// ```text
@@ -343,20 +335,42 @@ pub(crate) struct FileGroupDisplay<'a>(pub &'a [PartitionedFile]);
 
 impl<'a> Display for FileGroupDisplay<'a> {
     fn fmt(&self, f: &mut Formatter) -> FmtResult {
-        let group = self.0;
         write!(f, "[")?;
-        for (idx, pf) in group.iter().enumerate() {
-            if idx > 0 {
-                write!(f, ", ")?;
-            }
+        // To avoid showing too many files
+        let max_files = 5;
+        fmt_up_to_n_elements(self.0, max_files, f, |pf, f| {
             write!(f, "{}", pf.object_meta.location.as_ref())?;
             if let Some(range) = pf.range.as_ref() {
                 write!(f, ":{}..{}", range.start, range.end)?;
             }
+            Ok(())
+        })?;
+        write!(f, "]")
+    }
+}
+
+/// helper to format an array of up to N elements
+fn fmt_up_to_n_elements<E, F>(
+    elements: &[E],
+    n: usize,
+    f: &mut Formatter,
+    format_element: F,
+) -> FmtResult
+where
+    F: Fn(&E, &mut Formatter) -> FmtResult,
+{
+    let len = elements.len();
+    for (idx, element) in elements.iter().take(n).enumerate() {
+        if idx > 0 {
+            write!(f, ", ")?;
         }
-        write!(f, "]")?;
-        Ok(())
+        format_element(element, f)?;
+    }
+    // Remaining elements are showed as `...` (to indicate there is more)
+    if len > n {
+        write!(f, ", ...")?;
     }
+    Ok(())
 }
 
 /// A wrapper to customize partitioned file display
@@ -1298,6 +1312,22 @@ mod tests {
         assert_eq!(&FileGroupsDisplay(&files).to_string(), expected);
     }
 
+    #[test]
+    fn file_groups_display_too_many() {
+        let files = [
+            vec![partitioned_file("foo"), partitioned_file("bar")],
+            vec![partitioned_file("baz")],
+            vec![partitioned_file("qux")],
+            vec![partitioned_file("quux")],
+            vec![partitioned_file("quuux")],
+            vec![partitioned_file("quuuux")],
+            vec![],
+        ];
+
+        let expected = "{7 groups: [[foo, bar], [baz], [qux], [quux], [quuux], ...]}";
+        assert_eq!(&FileGroupsDisplay(&files).to_string(), expected);
+    }
+
     #[test]
     fn file_group_display_many() {
         let files = vec![partitioned_file("foo"), partitioned_file("bar")];
@@ -1306,6 +1336,21 @@ mod tests {
         assert_eq!(&FileGroupDisplay(&files).to_string(), expected);
     }
 
+    #[test]
+    fn file_group_display_too_many() {
+        let files = vec![
+            partitioned_file("foo"),
+            partitioned_file("bar"),
+            partitioned_file("baz"),
+            partitioned_file("qux"),
+            partitioned_file("quux"),
+            partitioned_file("quuux"),
+        ];
+
+        let expected = "[foo, bar, baz, qux, quux, ...]";
+        assert_eq!(&FileGroupDisplay(&files).to_string(), expected);
+    }
+
     /// create a PartitionedFile for testing
     fn partitioned_file(path: &str) -> PartitionedFile {
         let object_meta = ObjectMeta {