Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pretty Print UnionArrays #1648

Merged
merged 6 commits into from
May 6, 2022
Merged

Pretty Print UnionArrays #1648

merged 6 commits into from
May 6, 2022

Conversation

tfeda
Copy link
Contributor

@tfeda tfeda commented May 5, 2022

Which issue does this PR close?

Closes #1594 .

Rationale for this change

What changes are included in this PR?

Pretty printing support for UnionArray. My first stab is based on how they look in the specification, but I can change it according to any suggestions:
Example:

| Teamsters  |
 "+------------+
| {a=1}      |
| {b=3.2234} |
| {b=}       |
| {a=}       |
+------------+

Are there any user-facing changes?

@github-actions github-actions bot added the arrow Changes to the arrow crate label May 5, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #1648 (3961f23) into master (e51de5e) will increase coverage by 0.01%.
The diff coverage is 91.25%.

@@            Coverage Diff             @@
##           master    #1648      +/-   ##
==========================================
+ Coverage   83.08%   83.09%   +0.01%     
==========================================
  Files         193      193              
  Lines       55778    55858      +80     
==========================================
+ Hits        46343    46416      +73     
- Misses       9435     9442       +7     
Impacted Files Coverage Δ
arrow/src/util/display.rs 71.20% <61.11%> (-0.77%) ⬇️
arrow/src/util/pretty.rs 97.20% <100.00%> (+0.66%) ⬆️
arrow/src/datatypes/datatype.rs 66.40% <0.00%> (-0.40%) ⬇️
arrow/src/array/transform/mod.rs 86.57% <0.00%> (-0.12%) ⬇️
parquet_derive/src/parquet_field.rs 66.43% <0.00%> (+0.22%) ⬆️

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 e51de5e...3961f23. Read the comment docs.

Comment on lines +735 to +738
builder.append::<Int32Type>("b", 1).unwrap();
builder.append::<Float64Type>("c", 3.2234).unwrap();
builder.append_null::<Float64Type>("c").unwrap();
builder.append_null::<Int32Type>("b").unwrap();
Copy link
Member

Choose a reason for hiding this comment

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

Can you let outer show one null value from inner UnionArray too?

Comment on lines 427 to 428
"Repl error: could not get field for the corresponding type in union array."
.to_string(),
Copy link
Member

Choose a reason for hiding this comment

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

Maybe have type_id in error string?

Copy link
Member

@viirya viirya left a comment

Choose a reason for hiding this comment

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

This looks good. Thanks @tfeda !

@viirya viirya merged commit 922bfe7 into apache:master May 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrow Changes to the arrow crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add PrettyPrint for UnionArrays
3 participants