alamb commented on a change in pull request #975:
URL: https://github.com/apache/arrow-rs/pull/975#discussion_r757452673
##########
File path: arrow/src/util/pretty.rs
##########
@@ -42,6 +43,12 @@ pub fn print_batches(results: &[RecordBatch]) -> Result<()> {
Ok(())
}
+pub fn write_batches<W: Write>(buf: &mut W, results: &[RecordBatch]) ->
Result<()> {
Review comment:
That is an excellent point @matthewmturner
> second, assuming that doesnt work could you just explain what the
difference is between pretty_format_batches and the displayable function you
proposed?
The only difference I am aware of is that `pretty_format_batches` requires a
`String` (so allocates some memory and puts the formatted batches there). Thus
it is not as efficient
Though now that you mention this, perhaps we could *change*
`pretty_format_batches` to something like the following (basically to get rid
of the `to_string()`:
```rust
///! Create a visual representation of record batches
pub fn pretty_format_batches(results: &[RecordBatch]) -> Result<impl
Display> {
create_table(results)
}
///! Create a visual representation of columns
pub fn pretty_format_columns(col_name: &str, results: &[ArrayRef]) ->
Result<impl Display> {
create_column(col_name, results)
}
```
This would be a API change because callers would now have to call
`to_string()` if they wanted a string, so instead of
```rust
let s = pretty_format_batches(&batches)?;
```
It would look more like
```rust
let s = pretty_format_batches(&batches)?.to_string();
```
But the benefit is that the `String` would no longer be created unless it
was necessary
--
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]
