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.

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

Replace column/table test utilities with macros #12242

Merged
merged 7 commits into from
Nov 29, 2022

Conversation

PointKernel
Copy link
Member

@PointKernel PointKernel commented Nov 24, 2022

w.r.t. #11007

This PR moves cudf::test::expect_column/table* utilities to the detail namespace and replace their use cases with corresponding macros when possible.

Description

Checklist

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

@PointKernel PointKernel added tests Unit testing for project libcudf Affects libcudf (C++/CUDA) code. tech debt improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Nov 24, 2022
@PointKernel PointKernel self-assigned this Nov 24, 2022
@codecov
Copy link

codecov bot commented Nov 24, 2022

Codecov Report

❗ No coverage uploaded for pull request base (branch-23.02@3bf6d1d). Click here to learn what that means.
Patch has no changes to coverable lines.

Additional details and impacted files
@@               Coverage Diff               @@
##             branch-23.02   #12242   +/-   ##
===============================================
  Coverage                ?   88.20%           
===============================================
  Files                   ?      137           
  Lines                   ?    22660           
  Branches                ?        0           
===============================================
  Hits                    ?    19988           
  Misses                  ?     2672           
  Partials                ?        0           

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

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@PointKernel PointKernel marked this pull request as ready for review November 28, 2022 19:21
@PointKernel PointKernel requested a review from a team as a code owner November 28, 2022 19:21
Copy link
Contributor

@davidwendt davidwendt left a comment

Choose a reason for hiding this comment

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

LGTM

@PointKernel PointKernel added the 3 - Ready for Review Ready for review by team label Nov 28, 2022
@PointKernel
Copy link
Member Author

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 9af3ed8 into rapidsai:branch-23.02 Nov 29, 2022
@PointKernel PointKernel deleted the use-test-expect-macros branch November 29, 2022 03:10
@karthikeyann
Copy link
Contributor

@PointKernel Thank you for the cleanup. This cpp API usage still happens.
is there a way we can avoid using the cpp API directly instead of macro?
One idea is to leave a not in the API's documentation.

@PointKernel
Copy link
Member Author

Is there a way we can avoid using the cpp API directly instead of macro?

I moved those utilities to detail namespace mainly for this reason.

One idea is to leave a not in the API's documentation.

Good point. I will put up a PR updating docs and comments for this.

rapids-bot bot pushed a commit to rapidsai/cuspatial that referenced this pull request Dec 5, 2022
Following rapidsai/cudf#12242, this PR updates libcuspatial tests with cudf test macros.

Authors:
  - Michael Wang (https://github.com/isVoid)

Approvers:
  - Mark Harris (https://github.com/harrism)
  - H. Thomson Comer (https://github.com/thomcom)

URL: #841
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for Review Ready for review by team improvement Improvement / enhancement to an existing function libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change tests Unit testing for project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants