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

Doctests for UnionBuilder and UnionArray. #603

Merged
merged 2 commits into from
Jul 26, 2021
Merged

Conversation

novemberkilo
Copy link
Contributor

Which issue does this PR close?

Closes #301.

(@alamb do you concur that this PR closes the issue. Or have I missed an array type that could use a doctest?)

What changes are included in this PR?

Repurposes existing examples for UnionBuilder and putting them into doctests.
Adds some documentation for UnionArray

Are there any user-facing changes?

No

@github-actions github-actions bot added the arrow Changes to the arrow crate label Jul 25, 2021
@codecov-commenter
Copy link

codecov-commenter commented Jul 25, 2021

Codecov Report

Merging #603 (1d49007) into master (87d2840) will increase coverage by 0.01%.
The diff coverage is 50.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #603      +/-   ##
==========================================
+ Coverage   82.41%   82.42%   +0.01%     
==========================================
  Files         167      167              
  Lines       46232    46257      +25     
==========================================
+ Hits        38100    38126      +26     
+ Misses       8132     8131       -1     
Impacted Files Coverage Δ
arrow/src/array/array_union.rs 89.26% <ø> (ø)
arrow/src/array/builder.rs 86.26% <50.00%> (+0.15%) ⬆️
arrow/src/array/ord.rs 63.33% <0.00%> (ø)
arrow/src/compute/kernels/substring.rs 98.27% <0.00%> (ø)
arrow/src/compute/kernels/partition.rs 97.65% <0.00%> (+0.15%) ⬆️
parquet/src/encodings/encoding.rs 95.04% <0.00%> (+0.19%) ⬆️

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 87d2840...1d49007. Read the comment docs.

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thanks @novemberkilo

In terms of completing the documentation, I looked at all the things that implemented Array here: https://docs.rs/arrow/5.0.0/arrow/array/trait.Array.html#implementors

I think https://docs.rs/arrow/5.0.0/arrow/array/type.LargeBinaryArray.html and https://docs.rs/arrow/5.0.0/arrow/array/type.BinaryArray.html are stll missing examples, but othrwise it is looking good.

Thanks so much @novemberkilo

arrow/src/array/array_union.rs Outdated Show resolved Hide resolved
@alamb alamb mentioned this pull request Jul 25, 2021
Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
@novemberkilo
Copy link
Contributor Author

@alamb I will add examples for LargeBinaryArray and BinaryArray to this PR in the next few days.

@alamb alamb merged commit 19dba50 into apache:master Jul 26, 2021
@alamb
Copy link
Contributor

alamb commented Jul 26, 2021

Great -- Thank you @novemberkilo -- feel free to reopen #301 if you want or just mention it.

Again, I really appreciate your help in this area, and I think future users will as well. 👍

alamb added a commit that referenced this pull request Jul 27, 2021
* Doctests for UnionBuilder and UnionArray.

* Update arrow/src/array/array_union.rs

Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>

Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
alamb added a commit that referenced this pull request Jul 28, 2021
* Doctests for UnionBuilder and UnionArray.

* Update arrow/src/array/array_union.rs

Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>

Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>

Co-authored-by: Navin <navin@novemberkilo.com>
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.

More examples of how to construct Arrays
3 participants