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

min_rows and num_rows are swapped in ComputePageSizes declaration in Parquet reader #12886

Merged
merged 1 commit into from
Mar 6, 2023
Merged

min_rows and num_rows are swapped in ComputePageSizes declaration in Parquet reader #12886

merged 1 commit into from
Mar 6, 2023

Conversation

etseidl
Copy link
Contributor

@etseidl etseidl commented Mar 6, 2023

Description

Arguments are swapped in declaration (and thus docs as well). Args are passed as required by the definition when ComputePageSizes is called.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@etseidl etseidl requested a review from a team as a code owner March 6, 2023 18:47
@etseidl etseidl requested review from harrism and PointKernel March 6, 2023 18:47
@rapids-bot
Copy link

rapids-bot bot commented Mar 6, 2023

Pull requests from external contributors require approval from a rapidsai organization member with write or admin permissions before CI can begin.

@github-actions github-actions bot added the libcudf Affects libcudf (C++/CUDA) code. label Mar 6, 2023
@PointKernel PointKernel added bug Something isn't working cuIO cuIO issue non-breaking Non-breaking change labels Mar 6, 2023
@ttnghia
Copy link
Contributor

ttnghia commented Mar 6, 2023

/ok to test

PointKernel
PointKernel approved these changes Mar 6, 2023
View reviewed changes
Copy link
Member

@PointKernel PointKernel left a comment

Choose a reason for hiding this comment

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

The correction looks legit to me.

Was about to request @ttnghia 's review but apparently you are working at SOL. :)

@PointKernel
Copy link
Member

/merge

@rapids-bot rapids-bot bot merged commit a7ede21 into rapidsai:branch-23.04 Mar 6, 2023
@etseidl etseidl deleted the feature/page_sizes_fix branch March 10, 2023 02:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PointKernel PointKernel PointKernel approved these changes

@ttnghia ttnghia ttnghia approved these changes

@harrism harrism Awaiting requested review from harrism harrism is a code owner automatically assigned from rapidsai/cudf-cpp-codeowners

Assignees
No one assigned
Labels
bug Something isn't working cuIO cuIO issue libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@etseidl @ttnghia @PointKernel