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

Remove deprecated code. #10450

Merged
merged 3 commits into from
Mar 24, 2022
Merged

Remove deprecated code. #10450

merged 3 commits into from
Mar 24, 2022

Conversation

vyasr
Copy link
Contributor

@vyasr vyasr commented Mar 17, 2022 •
edited
Loading

This PR removes various pieces of deprecated code. These removals are also helpful for certain ongoing refactoring tasks.

Resolves #10166
Resolves #9316

@vyasr vyasr added 3 - Ready for Review Ready for review by team code quality Python Affects Python cuDF API. improvement Improvement / enhancement to an existing function breaking Breaking change labels Mar 17, 2022
@vyasr vyasr added this to the CuDF Python Refactoring milestone Mar 17, 2022
@vyasr vyasr requested a review from a team as a code owner March 17, 2022 17:58
@vyasr vyasr self-assigned this Mar 17, 2022
@vyasr vyasr requested review from shwina and bdice and removed request for a team March 17, 2022 17:58
bdice
bdice approved these changes Mar 17, 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.

Nice. All looks good to me, though one test needs to be fixed (a separate PR might be best).

python/cudf/cudf/tests/test_dataframe.py
@@ -4540,9 +4539,6 @@ def test_empty_df_astype(dtype, args):
),
pytest.param("other", marks=pytest.mark.xfail(raises=ValueError)),
"ignore",
pytest.param(
"warn", marks=pytest.mark.filterwarnings("ignore:Traceback")
),
],
)
def test_series_astype_error_handling(errors):
Copy link
Contributor

@bdice bdice Mar 17, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This test shouldn't use xfail for "raise" or "other". We should ensure the appropriate error is raised with pytest.raises instead of just permitting the test to fail. This can be done in a separate PR if desired, since this PR is strictly removing deprecated features.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, I'll keep this in mind for future work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also... why is this test (of Series) in test_dataframe.py...

@bdice
Copy link
Contributor

bdice commented Mar 17, 2022

CI failures are from 22.06 images that probably don't exist yet:

FATAL: Could not pull image: gpuci/rapidsai-driver:22.06-cuda11.5-devel-centos7-py3.8

@vyasr
Copy link
Contributor Author

vyasr commented Mar 17, 2022

CI failures are from 22.06 images that probably don't exist yet:

FATAL: Could not pull image: gpuci/rapidsai-driver:22.06-cuda11.5-devel-centos7-py3.8

Yeah I don't expect this to pass for a while. We'll need new images and for ops to set up the whole pipeline for 22.06. I just opened the PR so that I could base future work on this branch.

@vyasr
Copy link
Contributor Author

vyasr commented Mar 23, 2022

rerun tests

3 similar comments
@vyasr
Copy link
Contributor Author

vyasr commented Mar 23, 2022

rerun tests

@vyasr
Copy link
Contributor Author

vyasr commented Mar 23, 2022

rerun tests

@jjacobelli
Copy link
Contributor

rerun tests

@codecov
Copy link

codecov bot commented Mar 24, 2022 •
edited
Loading

Codecov Report

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

❗ Current head 6489e0c differs from pull request most recent head fd4af2d. Consider uploading reports for the commit fd4af2d to get more accurate results

@@               Coverage Diff               @@
##             branch-22.06   #10450   +/-   ##
===============================================
  Coverage                ?   86.31%           
===============================================
  Files                   ?      140           
  Lines                   ?    22312           
  Branches                ?        0           
===============================================
  Hits                    ?    19259           
  Misses                  ?     3053           
  Partials                ?        0

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 79f2b4c...fd4af2d. Read the comment docs.

@vyasr
Copy link
Contributor Author

vyasr commented Mar 24, 2022

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 8c7260f into rapidsai:branch-22.06 Mar 24, 2022
@vyasr vyasr deleted the remove_deprecations branch March 31, 2022 20:09
@vyasr vyasr mentioned this pull request Apr 4, 2022
Closed
rlratzel added a commit to ChuckHastings/cugraph that referenced this pull request Apr 5, 2022
@rlratzel
Changed cudf_obj.iteritems() to cudf_obj.items() since interitems() w…
6f02931 
…as removed in rapidsai/cudf#10450
@vyasr vyasr mentioned this pull request Jul 13, 2022
Closed
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

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

Assignees

@vyasr vyasr

Labels
3 - Ready for Review Ready for review by team breaking Breaking change improvement Improvement / enhancement to an existing function Python Affects Python cuDF API.
Projects
None yet
Milestone
cuDF Python Refactoring
Development

Successfully merging this pull request may close these issues.
3 participants
@vyasr @bdice @jjacobelli