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

Switch to using public cudf testing utilities #431

Merged
merged 17 commits into from
Jul 21, 2021

Conversation

vyasr
Copy link
Contributor

@vyasr vyasr commented Jul 2, 2021

As of rapidsai/cudf#7495 the cudf.tests.utils module (and in particular the assert_eq function) are no longer part of the public API. This PR switches tests to use the public testing functions in the cudf.testing subpackage.

This PR is currently blocked by #430 and rapidsai/cudf#8646.

@github-actions github-actions bot added the Python Related to Python code label Jul 2, 2021
@cwharris cwharris added bug Something isn't working non-breaking Non-breaking change labels Jul 2, 2021
@harrism
Copy link
Member

harrism commented Jul 20, 2021

@vyasr can we get this finished for 21.08? If not, please retarget to 21.10.

@vyasr
Copy link
Contributor Author

vyasr commented Jul 20, 2021

@harrism sure if you think it's worthwhile I can prioritize getting this done now, it shouldn't be too hard but just didn't seem to be a priority.

@harrism
Copy link
Member

harrism commented Jul 20, 2021

Is cuSpatial testing broken without this PR? If not, it can wait until 21.10.

@vyasr
Copy link
Contributor Author

vyasr commented Jul 20, 2021

Testing isn't broken, it's just using some cudf functions that are not intended to be part of the API. We don't want cudf to have to maintain API stability for those functions since they're internal, so we need to remove their usage from here. I know this affects (at minimum) cugraph as well. Since we control these functions in cudf we can avoid making any breaking changes until other downstream packages stop using them, though, so I don't think this is urgent for this release.

That said, I've started trying to resolve these conflicts, so I can update here if finishing this for 21.08 will be feasible.

@vyasr vyasr force-pushed the fix/test_function_changes branch from 0940223 to 024f819 Compare July 21, 2021 02:10
@vyasr vyasr self-assigned this Jul 21, 2021
@vyasr vyasr marked this pull request as ready for review July 21, 2021 02:12
@vyasr vyasr requested a review from a team as a code owner July 21, 2021 02:12
@vyasr
Copy link
Contributor Author

vyasr commented Jul 21, 2021

@harrism I think this PR should be good to go if tests pass.

@vyasr
Copy link
Contributor Author

vyasr commented Jul 21, 2021

@vyasr
Copy link
Contributor Author

vyasr commented Jul 21, 2021

rerun tests

@harrism
Copy link
Member

harrism commented Jul 21, 2021

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 2344dcd into rapidsai:branch-21.08 Jul 21, 2021
@vyasr vyasr deleted the fix/test_function_changes branch July 26, 2021 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working non-breaking Non-breaking change Python Related to Python code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants