Skip to content

Move cuml.accel tests out into own directory#6547

Merged
rapids-bot[bot] merged 1 commit intorapidsai:branch-25.06from
jcrist:move-cuml-accel-tests
Apr 16, 2025
Merged

Move cuml.accel tests out into own directory#6547
rapids-bot[bot] merged 1 commit intorapidsai:branch-25.06from
jcrist:move-cuml-accel-tests

Conversation

@jcrist
Copy link
Copy Markdown
Member

@jcrist jcrist commented Apr 15, 2025

Previously tests for cuml.accel were found with the cuml/tests/ directory. This was problematic because these tests only made sense to run with cuml.accel on, meaning some either the test runner had to explicitly ignore them or the tests needed to be marked to skip if cuml.accel wasn't enabled.

Since we always run the accelerator tests in their own test run anyway, it makes more sense to extract these tests out into a separate tests directory. This is also what cudf did for their cudf.pandas tests, so there is already precedent here.

Previously tests for `cuml.accel` were found with the `cuml/tests/`
directory. This was problematic because these tests only made sense to
run with `cuml.accel` on, meaning some either the test runner had to
explicitly ignore them or the tests needed to be marked to skip if
`cuml.accel` wasn't enabled.

Since we always run the accelerator tests in their own test run anyway,
it makes more sense to extract these tests out into a separate tests
directory. This is also what `cudf` did for their `cudf.pandas` tests,
so there is already precedent here.
@jcrist jcrist requested review from a team as code owners April 15, 2025 21:07
@github-actions github-actions Bot added Cython / Python Cython or Python issue ci labels Apr 15, 2025
@jcrist jcrist added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Apr 15, 2025
@jcrist jcrist self-assigned this Apr 15, 2025
Copy link
Copy Markdown
Member

@jameslamb jameslamb left a comment

Choose a reason for hiding this comment

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

Preemptively giving you a ci-codeowners review, since this is mainly just a moving-files-around PR. I'll defer to other cuML maintainers on the substance of these changes (though they seem like an improvement to me!).

@jameslamb jameslamb removed the request for review from raydouglass April 15, 2025 21:37
@jcrist
Copy link
Copy Markdown
Member Author

jcrist commented Apr 16, 2025

/merge

@rapids-bot rapids-bot Bot merged commit 156a2cd into rapidsai:branch-25.06 Apr 16, 2025
78 of 79 checks passed
@jcrist jcrist deleted the move-cuml-accel-tests branch April 17, 2025 17:21
Ofek-Haim pushed a commit to Ofek-Haim/cuml that referenced this pull request May 13, 2025
Previously tests for `cuml.accel` were found with the `cuml/tests/` directory. This was problematic because these tests only made sense to run with `cuml.accel` on, meaning some either the test runner had to explicitly ignore them or the tests needed to be marked to skip if `cuml.accel` wasn't enabled.

Since we always run the accelerator tests in their own test run anyway, it makes more sense to extract these tests out into a separate tests directory. This is also what `cudf` did for their `cudf.pandas` tests, so there is already precedent here.

Authors:
  - Jim Crist-Harif (https://github.com/jcrist)

Approvers:
  - James Lamb (https://github.com/jameslamb)
  - Divye Gala (https://github.com/divyegala)

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

Labels

ci Cython / Python Cython or Python issue improvement Improvement / enhancement to an existing function non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants