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.

Sign up for GitHub

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

Jump to bottom

Refactor std::array usage in row group index writing in ORC #6807

Merged
merged 7 commits into from
Dec 2, 2020
Merged

Refactor std::array usage in row group index writing in ORC #6807

merged 7 commits into from
Dec 2, 2020

Conversation

rgsl888prabhu
Copy link
Contributor

@rgsl888prabhu rgsl888prabhu commented Nov 19, 2020 •
edited
Loading

Replaced std::array with named struct.
closes #6401

@rgsl888prabhu rgsl888prabhu requested review from a team as code owners November 19, 2020 16:44
@rgsl888prabhu rgsl888prabhu self-assigned this Nov 19, 2020
@rgsl888prabhu rgsl888prabhu added this to PR-WIP in v0.17 Release via automation Nov 19, 2020
@GPUtester
Copy link
Collaborator

Please update the changelog in order to start CI tests.

View the gpuCI docs here.

@rgsl888prabhu rgsl888prabhu changed the title [REVIEW] Refactor array usage and add test coverage for row group index writing [REVIEW] Refactor std::array usage and add test coverage for row group index writing Nov 19, 2020
@rgsl888prabhu rgsl888prabhu changed the title [REVIEW] Refactor std::array usage and add test coverage for row group index writing [REVIEW] Refactor std::array usage and add test coverage for row group index writing in ORC Nov 19, 2020
@rgsl888prabhu rgsl888prabhu moved this from PR-WIP to PR-Needs review in v0.17 Release Nov 19, 2020
@codecov
Copy link

codecov bot commented Nov 19, 2020 •
edited
Loading

Codecov Report

Merging #6807 (9f78e51) into branch-0.17 (83d2146) will increase coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff               @@
##           branch-0.17    #6807      +/-   ##
===============================================
+ Coverage        81.92%   81.94%   +0.02%     
===============================================
  Files               96       96              
  Lines            16161    16164       +3     
===============================================
+ Hits             13240    13246       +6     
+ Misses            2921     2918       -3
Impacted Files Coverage Δ
python/cudf/cudf/_fuzz_testing/fuzzer.py 0.00% <0.00%> (ø)
python/cudf/cudf/utils/hash_vocab_utils.py 100.00% <0.00%> (ø)
python/cudf/cudf/core/abc.py 91.48% <0.00%> (+4.25%) ⬆️
python/cudf/cudf/utils/gpu_utils.py 58.53% <0.00%> (+4.87%) ⬆️

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 83d2146...9f78e51. Read the comment docs.

vuule
vuule requested changes Nov 20, 2020
View reviewed changes
cpp/src/io/orc/writer_impl.cu Outdated Show resolved Hide resolved
cpp/src/io/orc/writer_impl.cu Outdated Show resolved Hide resolved
python/cudf/cudf/tests/test_orc.py Outdated Show resolved Hide resolved
v0.17 Release automation moved this from PR-Needs review to PR-Reviewer approved Nov 20, 2020
@vuule vuule changed the title [REVIEW] Refactor std::array usage and add test coverage for row group index writing in ORC [REVIEW] Refactor std::array usage in row group index writing in ORC Nov 21, 2020
vuule
vuule reviewed Nov 21, 2020
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
@vuule vuule added the non-breaking Non-breaking change label Dec 2, 2020
@vuule
Copy link
Contributor

vuule commented Dec 2, 2020 •
edited
Loading

@kkraus14 This PR used to include Python changes so it requires a Python review. Can you please approve (as there are no Python changes)?
Edit: looks like Python review is not required to merge. Keeping the comment just in case it's not okay to merge as-is.

@vuule vuule added the libcudf Affects libcudf (C++/CUDA) code. label Dec 2, 2020
@rgsl888prabhu rgsl888prabhu added the improvement Improvement / enhancement to an existing function label Dec 2, 2020
kkraus14
kkraus14 approved these changes Dec 2, 2020
View reviewed changes
Copy link
Collaborator

@kkraus14 kkraus14 left a comment

Choose a reason for hiding this comment

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

Approving for Python anyway

@harrism
Copy link
Member

harrism commented Dec 2, 2020

Setting Okay to automerge.

@rgsl888prabhu
Copy link
Contributor Author

@harrism should I remove change log ?

@rgsl888prabhu rgsl888prabhu changed the title [REVIEW] Refactor std::array usage in row group index writing in ORC Refactor std::array usage in row group index writing in ORC Dec 2, 2020
@rapids-bot rapids-bot bot merged commit ff66c5e into rapidsai:branch-0.17 Dec 2, 2020
v0.17 Release automation moved this from PR-Reviewer approved to Done Dec 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vuule vuule vuule approved these changes

@kkraus14 kkraus14 kkraus14 approved these changes

@karthikeyann karthikeyann karthikeyann approved these changes

@cwharris cwharris Awaiting requested review from cwharris

@galipremsagar galipremsagar Awaiting requested review from galipremsagar

Assignees

@rgsl888prabhu rgsl888prabhu

Labels
cuIO cuIO issue improvement Improvement / enhancement to an existing function libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
No open projects
v0.17 Release
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG] write_index_stream in ORC writer indexing error
6 participants
@rgsl888prabhu @GPUtester @vuule @harrism @kkraus14 @karthikeyann