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

[REVIEW] Deprecate skiprows and num_rows in read_orc #11522

Merged
merged 2 commits into from
Aug 15, 2022
Merged

[REVIEW] Deprecate skiprows and num_rows in read_orc #11522

merged 2 commits into from
Aug 15, 2022

Conversation

galipremsagar
Copy link
Contributor

@galipremsagar galipremsagar commented Aug 12, 2022 •
edited
Loading

Description

Resolves the first step of #11519 by deprecating skiprows and num_rows in orc reader.

Checklist

@galipremsagar galipremsagar added 3 - Ready for Review Ready for review by team Python Affects Python cuDF API. 4 - Needs cuDF (Python) Reviewer labels Aug 12, 2022
@galipremsagar galipremsagar self-assigned this Aug 12, 2022
@galipremsagar galipremsagar added this to PR-WIP in v22.10 Release via automation Aug 12, 2022
@galipremsagar galipremsagar marked this pull request as ready for review August 12, 2022 15:46
@galipremsagar galipremsagar requested a review from a team as a code owner August 12, 2022 15:46
@galipremsagar galipremsagar moved this from PR-WIP to PR-Needs review in v22.10 Release Aug 12, 2022
@galipremsagar galipremsagar mentioned this pull request Aug 12, 2022
Closed
2 tasks
@galipremsagar galipremsagar added non-breaking Non-breaking change deprecation breaking Breaking change improvement Improvement / enhancement to an existing function and removed non-breaking Non-breaking change labels Aug 12, 2022
bdice
bdice approved these changes Aug 12, 2022
View reviewed changes
Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

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

LGTM with two small edits.

python/cudf/cudf/utils/ioutils.py Outdated Show resolved Hide resolved
python/cudf/cudf/utils/ioutils.py Outdated Show resolved Hide resolved
v22.10 Release automation moved this from PR-Needs review to PR-Reviewer approved Aug 12, 2022
@galipremsagar @bdice
Apply suggestions from code review
23465a3 
Co-authored-by: Bradley Dice <bdice@bradleydice.com>
@codecov
Copy link

codecov bot commented Aug 12, 2022 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (branch-22.10@2be93fe). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head 81d59e7 differs from pull request most recent head 23465a3. Consider uploading reports for the commit 23465a3 to get more accurate results

@@               Coverage Diff               @@
##             branch-22.10   #11522   +/-   ##
===============================================
  Coverage                ?   86.36%           
===============================================
  Files                   ?      145           
  Lines                   ?    22949           
  Branches                ?        0           
===============================================
  Hits                    ?    19820           
  Misses                  ?     3129           
  Partials                ?        0

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

vuule
vuule approved these changes Aug 15, 2022
View reviewed changes
Copy link
Contributor

@vuule vuule left a comment

Choose a reason for hiding this comment

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

Thank you for creating the PR :)

@galipremsagar galipremsagar added 5 - Ready to Merge Testing and reviews complete, ready to merge and removed 3 - Ready for Review Ready for review by team 4 - Needs cuIO Reviewer labels Aug 15, 2022
@galipremsagar
Copy link
Contributor Author

@gpucibot merge

@rapids-bot rapids-bot bot merged commit a221d47 into rapidsai:branch-22.10 Aug 15, 2022
v22.10 Release automation moved this from PR-Reviewer approved to Done Aug 15, 2022
@GregoryKimball GregoryKimball mentioned this pull request Feb 26, 2024
Open
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bdice bdice bdice approved these changes

@vuule vuule vuule approved these changes

@skirui-source skirui-source Awaiting requested review from skirui-source skirui-source is a code owner automatically assigned from rapidsai/cudf-python-codeowners

Assignees

@galipremsagar galipremsagar

Labels
5 - Ready to Merge Testing and reviews complete, ready to merge breaking Breaking change improvement Improvement / enhancement to an existing function Python Affects Python cuDF API.
Projects
No open projects
v22.10 Release
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@galipremsagar @bdice @vuule