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 BooleanArray. #338

Merged
merged 2 commits into from
May 24, 2021
Merged

Doctests for BooleanArray. #338

merged 2 commits into from
May 24, 2021

Conversation

novemberkilo
Copy link
Contributor

Which issue does this PR close?

Closes #301 .

What changes are included in this PR?

Doctests for BooleanArray

Are there any user-facing changes?

No.

@codecov-commenter
Copy link

codecov-commenter commented May 22, 2021

Codecov Report

Merging #338 (c1abbe9) into master (b2de544) will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #338      +/-   ##
==========================================
+ Coverage   82.54%   82.56%   +0.01%     
==========================================
  Files         162      162              
  Lines       44047    44063      +16     
==========================================
+ Hits        36359    36379      +20     
+ Misses       7688     7684       -4     
Impacted Files Coverage Δ
arrow/src/array/array_boolean.rs 90.90% <ø> (ø)
arrow/src/array/transform/boolean.rs 76.92% <0.00%> (-7.70%) ⬇️
arrow/src/compute/kernels/cast.rs 94.53% <0.00%> (+0.04%) ⬆️
parquet/src/encodings/encoding.rs 95.04% <0.00%> (+0.19%) ⬆️
arrow/src/array/equal/list.rs 89.55% <0.00%> (+5.97%) ⬆️

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 b2de544...c1abbe9. 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 -- I have some comments , but overall this is a great step

arrow/src/array/array_boolean.rs Outdated Show resolved Hide resolved
arrow/src/array/array_boolean.rs Outdated Show resolved Hide resolved
arrow/src/array/array_boolean.rs Outdated Show resolved Hide resolved
@novemberkilo
Copy link
Contributor Author

Thanks @alamb - I have put up changes to address your comments, squashed and pushed so that I have a single commit here again.

@novemberkilo
Copy link
Contributor Author

Looks like the build failure is due to a configuration issue (rustup not found on the windows instance)?

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 !

arrow/src/array/array_boolean.rs Outdated Show resolved Hide resolved
@alamb
Copy link
Contributor

alamb commented May 24, 2021

Looks like the build failure is due to a configuration issue (rustup not found on the windows instance)?

I agree -- I will restart the tests

Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
@alamb
Copy link
Contributor

alamb commented May 24, 2021

the MIRI failure is unrelated to this PR -- it is happening on master too -- see #345.

Thanks again @novemberkilo !

@alamb alamb merged commit 488f826 into apache:master May 24, 2021
alamb added a commit that referenced this pull request May 25, 2021
* Doctests for BooleanArray.

* Update arrow/src/array/array_boolean.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 May 26, 2021
* Doctests for BooleanArray.

* Update arrow/src/array/array_boolean.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
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

More examples of how to construct Arrays
3 participants