Skip to content

Add SparseRandomProjection, AgglomerativeClustering and GaussianRandomProjection to common checks#7307

Merged
rapids-bot[bot] merged 8 commits intorapidsai:branch-25.12from
betatim:add-random-projection-to-common-tests-2
Oct 15, 2025
Merged

Add SparseRandomProjection, AgglomerativeClustering and GaussianRandomProjection to common checks#7307
rapids-bot[bot] merged 8 commits intorapidsai:branch-25.12from
betatim:add-random-projection-to-common-tests-2

Conversation

@betatim
Copy link
Copy Markdown
Member

@betatim betatim commented Oct 6, 2025

Revived version of #7136 and #7135

@betatim betatim requested a review from a team as a code owner October 6, 2025 07:18
@betatim betatim requested a review from divyegala October 6, 2025 07:18
@github-actions github-actions Bot added the Cython / Python Cython or Python issue label Oct 6, 2025
@betatim betatim added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Oct 6, 2025
@betatim betatim changed the title Add SparseRandomProjection and GaussianRandomProjection to common checks Add SparseRandomProjection, AgglomerativeClustering and GaussianRandomProjection to common checks Oct 6, 2025
Copy link
Copy Markdown
Member

@jcrist jcrist left a comment

Choose a reason for hiding this comment

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

Seems fine to me.

Two questions:

  • Given one of the tests is skipped due to causing memory errors, did you run this a few times successfully (at least locally) to see if anything else shakes out?
  • Are these xfails strict (meaning if we fix a bug they'll start failing until they're un-xfailed)?

@betatim
Copy link
Copy Markdown
Member Author

betatim commented Oct 15, 2025

If I run (just) the tests in test_sklearn_compatibility.py then nothing bad happens when the pickle test is included. So the failure is the result of some kind of interaction.

The tests aren't running in strict mode. I'd prefer to do that in a new PR.

@jcrist
Copy link
Copy Markdown
Member

jcrist commented Oct 15, 2025

/merge

@rapids-bot rapids-bot Bot merged commit 029a307 into rapidsai:branch-25.12 Oct 15, 2025
190 of 192 checks passed
@betatim
Copy link
Copy Markdown
Member Author

betatim commented Oct 15, 2025

meh, xfail_strict=True got merged in scikit-learn but either to late or didn't get backported to 1.7.2. So we need to be patient

@csadorf
Copy link
Copy Markdown
Contributor

csadorf commented Oct 15, 2025

meh, xfail_strict=True got merged in scikit-learn but either to late or didn't get backported to 1.7.2. So we need to be patient

Can we make sure to track this in a follow-up issue?

@betatim betatim deleted the add-random-projection-to-common-tests-2 branch October 15, 2025 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

4 participants