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

[FEA] Add third-party library integration testing of cudf.pandas to cudf #16645

Merged
merged 33 commits into from
Aug 30, 2024
Merged

[FEA] Add third-party library integration testing of cudf.pandas to cudf #16645

merged 33 commits into from
Aug 30, 2024

Conversation

Matt711
Copy link
Contributor

@Matt711 Matt711 commented Aug 23, 2024

Description

Closes #16580

Checklist

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

@Matt711 Matt711 added feature request New feature or request non-breaking Non-breaking change cudf.pandas Issues specific to cudf.pandas labels Aug 23, 2024
@Matt711 Matt711 self-assigned this Aug 23, 2024
@github-actions github-actions bot added the Python Affects Python cuDF API. label Aug 23, 2024
Matt711
Matt711 commented Aug 23, 2024
View reviewed changes
Copy link
Contributor Author

@Matt711 Matt711 left a comment

Choose a reason for hiding this comment

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

I think I've migrated the files we need from cudf-pandas-integration (ie. the tests and some of the shell scripts). And I've combined the three jobs before in to one.

I'm not sure if there's a proper way to test these changes, so I'll open the PR for review and get some feedback as it is.

@Matt711 Matt711 marked this pull request as ready for review August 23, 2024 16:58
@Matt711 Matt711 requested review from a team as code owners August 23, 2024 16:58
vyasr
vyasr requested changes Aug 23, 2024
View reviewed changes
Copy link
Contributor

@vyasr vyasr left a comment

Choose a reason for hiding this comment

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

I've recommended moving the test from nightly.yaml to test.yaml, but for the lifetime of this PR can we also add it to pr.yaml? That way we can test it out during this PR and iron out any issues, then remove that before the end so that the test only runs nightly.

.github/workflows/nightly.yaml Outdated Show resolved Hide resolved
python/cudf/cudf_pandas_tests/third_party_integration_tests/ci/ci_run_library_tests.sh Outdated Show resolved Hide resolved
python/cudf/cudf_pandas_tests/third_party_integration_tests/ci/ci_run_library_tests.sh Outdated Show resolved Hide resolved
python/cudf/cudf_pandas_tests/third_party_integration_tests/ci/ci_run_library_tests.sh Outdated Show resolved Hide resolved
python/cudf/cudf_pandas_tests/third_party_integration_tests/ci/extract_lib.sh Outdated Show resolved Hide resolved
@copy-pr-bot copy-pr-bot
Copy link

copy-pr-bot bot commented Aug 24, 2024

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@Matt711 Matt711 mentioned this pull request Aug 27, 2024
Merged
3 tasks
Matt711
Matt711 commented Aug 28, 2024
View reviewed changes
.github/workflows/pr.yaml Outdated Show resolved Hide resolved
@Matt711 Matt711 added the 2 - In Progress Currently a work in progress label Aug 28, 2024
@Matt711 Matt711 force-pushed the feat/cudf-pandas-integration-tests branch from c794216 to 9561ea2 Compare August 28, 2024 22:31
@Matt711
Copy link
Contributor Author

Matt711 commented Aug 29, 2024

/ok to test

@Matt711 Matt711 added 3 - Ready for Review Ready for review by team and removed 2 - In Progress Currently a work in progress labels Aug 29, 2024
@Matt711 Matt711 requested a review from vyasr August 29, 2024 04:03
bdice
bdice approved these changes Aug 29, 2024
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.

I didn't read the tests that closely since we have previously reviewed them. I focused on the integration pieces. This seems fine to me! Thanks for working on this.

ci/cudf_pandas_scripts/third-party-integration/ci_run_library_tests.sh Outdated Show resolved Hide resolved
python/cudf/cudf_pandas_tests/third_party_integration_tests/dependencies.yaml
Comment on lines +138 to +141
- matrix:
cuda: "11"
packages:
- cuda-version=11.8
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need both cuda: "11" and cuda: "11.8"? We typically do not list major-only versions in our dependencies files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I got errors like "Cannot find matching value in matrix with cuda:11/12", so I added both.

vyasr
vyasr approved these changes Aug 29, 2024
View reviewed changes
Copy link
Contributor

@vyasr vyasr left a comment

Choose a reason for hiding this comment

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

LGTM. I didn't re-review the actual tests since those have been looked at before, just the CI/GHA scripts and dependencies.yaml.

ci/cudf_pandas_scripts/run_tests.sh Show resolved Hide resolved
python/cudf/cudf_pandas_tests/third_party_integration_tests/ci/ci_run_library_tests.sh Outdated Show resolved Hide resolved
.github/workflows/pr.yaml Outdated Show resolved Hide resolved
@Matt711
Copy link
Contributor Author

Matt711 commented Aug 29, 2024

/ok to test

@Matt711
Copy link
Contributor Author

Matt711 commented Aug 30, 2024

/merge

@rapids-bot rapids-bot bot merged commit 62a53b3 into rapidsai:branch-24.10 Aug 30, 2024
88 checks passed
@Matt711 Matt711 mentioned this pull request Aug 30, 2024
Merged
3 tasks
rapids-bot bot pushed a commit that referenced this pull request Aug 30, 2024
@Matt711
[BUG] Add gpu node type to cudf-pandas 3rd-party integration nightly …
5a81a80 
…CI job (#16704)

Following up #16645, and adding a gpu node type to the nightly CI job

Authors:
  - Matthew Murray (https://github.com/Matt711)

Approvers:
  - Vyas Ramasubramani (https://github.com/vyasr)

URL: #16704
@vyasr vyasr mentioned this pull request Sep 3, 2024
Merged
vyasr added a commit to rapidsai/workflows that referenced this pull request Sep 3, 2024
@vyasr
Remove standalone cudf-pandas integration tests job (#60)
95fd62e 
https://github.com/rapidsai/cudf-pandas-integration/ has been archived
and the tests were added into the cudf repo in
rapidsai/cudf#16645.
@Matt711 Matt711 mentioned this pull request Sep 3, 2024
Merged
3 tasks
rapids-bot bot pushed a commit that referenced this pull request Sep 4, 2024
@Matt711
Refactor cudf pandas integration tests CI (#16728)
26091a4 
Following up #16645 with a couple improvements

Authors:
  - Matthew Murray (https://github.com/Matt711)

Approvers:
  - Bradley Dice (https://github.com/bdice)

URL: #16728
res-life pushed a commit to res-life/cudf that referenced this pull request Sep 11, 2024
@Matt711 @nvliyuan
Refactor cudf pandas integration tests CI (rapidsai#16728)
a7c4595 
Following up rapidsai#16645 with a couple improvements

Authors:
  - Matthew Murray (https://github.com/Matt711)

Approvers:
  - Bradley Dice (https://github.com/bdice)

URL: rapidsai#16728
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vyasr vyasr vyasr approved these changes

@bdice bdice bdice approved these changes

@KyleFromNVIDIA KyleFromNVIDIA Awaiting requested review from KyleFromNVIDIA KyleFromNVIDIA is a code owner automatically assigned from rapidsai/ci-codeowners

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

Assignees

@Matt711 Matt711

Labels
3 - Ready for Review Ready for review by team cudf.pandas Issues specific to cudf.pandas feature request New feature or request non-breaking Non-breaking change Python Affects Python cuDF API.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[FEA] Add integration testing of cudf.pandas
4 participants
@Matt711 @vyasr @bdice @galipremsagar