Change to comfy-table from prettytable-rs

[PsiACE]

Signed-off-by: Chojan Shang [email protected]

Which issue does this PR close?

Closes #69 .

Rationale for this change

comfy-table is still actively maintained.

also, solve RUSTSEC-2018-0015: term is looking for a new maintainer

What changes are included in this PR?

Change to comfy-table from prettytable-rs.

Are there any user-facing changes?

No.

[codecov-commenter]

Codecov Report

Merging #656 (6034db9) into master (fa5acd9) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #656   +/-   ##
=======================================
  Coverage   82.43%   82.43%           
=======================================
  Files         168      168           
  Lines       47340    47340           
=======================================
  Hits        39027    39027           
  Misses       8313     8313           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fa5acd9...6034db9. Read the comment docs.

[alamb]

Looks good @PsiACE -- thank you -- I will look carefully tomorrow but this looks great

[alamb]

All in all I think this is a good change. Thank you @PsiACE

I would like at least one other approval from a maintainer before merging it. Perhaps @andygrove @paddyhoran or @nevi-me have thoughts

If we do go with this change, I don't think we should backport it to the 5.X line -- but instead release it for the first time in 6.0 - 2 months or so (see notes under datafusion section below)

Maintenance

From crates.io it does appear that https://crates.io/crates/prettytable-rs was last updated almost 3 years ago, though it does seem to be very heavily used (though arrow is the third largest user)

https://crates.io/crates/comfy-table/4.0.1 was last updated a month ago and looks more actively maintained, but is less widely used.

This seems like a good tradeoff to me

Dependency Analysis

Looking at the dependencies

cargo tree -p arrow --features=prettyprint

Here are the comfy table dependencies:

├── comfy-table v4.0.1
│   ├── crossterm v0.20.0
│   │   ├── bitflags v1.2.1
│   │   ├── libc v0.2.98
│   │   ├── mio v0.7.13
│   │   │   ├── libc v0.2.98
│   │   │   └── log v0.4.14
│   │   │       └── cfg-if v1.0.0
│   │   ├── parking_lot v0.11.1
│   │   │   ├── instant v0.1.10
│   │   │   │   └── cfg-if v1.0.0
│   │   │   ├── lock_api v0.4.4
│   │   │   │   └── scopeguard v1.1.0
│   │   │   └── parking_lot_core v0.8.3
│   │   │       ├── cfg-if v1.0.0
│   │   │       ├── instant v0.1.10 (*)
│   │   │       ├── libc v0.2.98
│   │   │       └── smallvec v1.6.1
│   │   ├── signal-hook v0.3.9
│   │   │   ├── libc v0.2.98
│   │   │   └── signal-hook-registry v1.4.0
│   │   │       └── libc v0.2.98
│   │   └── signal-hook-mio v0.2.1
│   │       ├── libc v0.2.98
│   │       ├── mio v0.7.13 (*)
│   │       └── signal-hook v0.3.9 (*)
│   ├── strum v0.21.0
│   └── strum_macros v0.21.1 (proc-macro)
│       ├── heck v0.3.3
│       │   └── unicode-segmentation v1.8.0
│       ├── proc-macro2 v1.0.27
│       │   └── unicode-xid v0.2.2
│       ├── quote v1.0.9
│       │   └── proc-macro2 v1.0.27 (*)
│       └── syn v1.0.74
│           ├── proc-macro2 v1.0.27 (*)
│           ├── quote v1.0.9 (*)
│           └── unicode-xid v0.2.2

Here are the dependencies from pretty-table:

├── prettytable-rs v0.8.0
│   ├── atty v0.2.14
│   │   └── libc v0.2.98
│   ├── csv v1.1.6 (*)
│   ├── encode_unicode v0.3.6
│   ├── lazy_static v1.4.0
│   ├── term v0.5.2
│   │   ├── byteorder v1.4.3
│   │   └── dirs v1.0.5
│   │       └── libc v0.2.98
│   └── unicode-width v0.1.8

DataFusion

I also verified that https://github.com/apache/arrow-datafusion would work with this change by patching a local datafusion checkout to use this branch and then running cargo test -p datafusion

There are two differences in the tests reported, both edge cases (empty tables and floating point). I think they are changes for the better but also not strictly speaking backwards compatible. See datafusion-compile-run.txt for the full run

[paddyhoran]

It's hard to predict how a library will be maintained in the future but I agree with this change. I'm no expert on this aspect but is there anything we need to do due to it being MIT licensed?

[roee88]

Please add default-features = false. This will allow the prettyprint feature to be enabled in wasm32-wasi target in the next comfy-table release.
Ref: Nukesor/comfy-table#47

FYI @jorgecarleitao

[PsiACE]

done

[alamb]

I think this has been open long enough for comment. Merging in (will not backport, will plan to release in arrow 6.0.0)

[alamb]

Thanks again @PsiACE