alamb commented on code in PR #11416:
URL: https://github.com/apache/datafusion/pull/11416#discussion_r1675687544


##########
datafusion/physical-plan/src/aggregates/topk/heap.rs:
##########
@@ -462,7 +486,7 @@ mod tests {
         let mut heap = TopKHeap::new(10, false);
         heap.append_or_replace(1, 1, &mut map);
 
-        let actual = heap.tree_print();
+        let actual = format!("{heap}");

Review Comment:
   Another way to write this is `heap.to_string()`, which might be more 
"idomatic"
   
   ```suggestion
           let actual = heap.to_string()
   ```



##########
datafusion/physical-plan/src/aggregates/topk/heap.rs:
##########
@@ -361,9 +385,9 @@ impl<VAL: ValueType> HeapItem<VAL> {
 impl<VAL: ValueType> Debug for HeapItem<VAL> {
     fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
         f.write_str("bucket=")?;
-        self.map_idx.fmt(f)?;
+        Debug::fmt(&self.map_idx, f)?;

Review Comment:
   is this change needed?



##########
datafusion/physical-plan/src/aggregates/topk/heap.rs:
##########
@@ -323,29 +323,53 @@ impl<VAL: ValueType> TopKHeap<VAL> {
         }
     }
 
-    #[cfg(test)]
-    fn _tree_print(&self, idx: usize) -> Option<termtree::Tree<String>> {
-        let hi = self.heap.get(idx)?;
-        match hi {
-            None => None,
-            Some(hi) => {
-                let label =
-                    format!("val={:?} idx={}, bucket={}", hi.val, idx, 
hi.map_idx);
-                let left = self._tree_print(idx * 2 + 1);
-                let right = self._tree_print(idx * 2 + 2);
-                let children = left.into_iter().chain(right);
-                let me = termtree::Tree::new(label).with_leaves(children);
-                Some(me)
+    fn _tree_print(
+        &self,
+        idx: usize,
+        prefix: String,
+        is_tail: bool,
+        output: &mut String,
+    ) {
+        if let Some(Some(hi)) = self.heap.get(idx) {
+            let connector = if idx != 0 {
+                if is_tail {

Review Comment:
   Nice!



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to