❌ Linting issues

This PR is introducing linting issues. Here's a summary of the issues. Note that you can avoid having linting issues by enabling pre-commit hooks. Instructions to enable them can be found here.

You can see the details of the linting issues under the lint job here

black

black detected issues. Please run black . locally and push the changes. Here you can see the detected issues. Note that running black might also fix some of the issues which might be detected by ruff. Note that the installed black version is black=24.3.0.

--- /home/runner/work/scikit-learn/scikit-learn/sklearn/inspection/tests/test_permutation_importance.py	2025-04-10 11:09:15.474510+00:00
+++ /home/runner/work/scikit-learn/scikit-learn/sklearn/inspection/tests/test_permutation_importance.py	2025-04-10 11:09:33.370666+00:00
@@ -555,11 +555,11 @@
         y,
         n_repeats=n_repeats_test,
         scoring="neg_mean_squared_error",
         feature_indices=feature_indices_test,
     )
-    assert results.importances.shape == (len(feature_indices_test),n_repeats_test)
+    assert results.importances.shape == (len(feature_indices_test), n_repeats_test)
 
 
 def test_permutation_importance_feature_indices_out_of_range_error():
     """Check you get error when feature_indices are out of range."""
     n_features = 10
would reformat /home/runner/work/scikit-learn/scikit-learn/sklearn/inspection/tests/test_permutation_importance.py

Oh no! 💥 💔 💥
1 file would be reformatted, 918 files would be left unchanged.

_{Generated for commit: fc98ab6. Link to the linter CI: here}

hmm not sure how to fix that failing test:

FAILED tests/test_public_functions.py::test_function_param_validation[sklearn.inspection.permutation_importance] - AssertionError: Mismatch between _parameter_constraints and the parameters

@andrewm4894 Actually, we had a similar feature request in the past: #18694

In general, I think that the scope of #18694 is better defined for the tabular learning use case but using the propose API there, you could request N random features and make a single permutation and repeat the process.

You will need to combine the results but this is already something that you were planning.

It would be great that you have a look at the propose API in the issue.

@andrewm4894 Actually, we had a similar feature request in the past: #18694

@glemaitre oh interesting, I had thought about similar but felt this change a bit smaller and simple in some sense so maybe had more of a chance.

But agree, being able to pass on a list of features as a subset is probably the most general case and would serve what I need here too.

Although @glemaitre this is a small and simple pr here while you linked to an issue but not sure if there is a draft pr anywhere?

(I will try think what that might look like but not sure how complex/big or not it might be)

Eg maybe a trade off between small incremental change vs a bigger one that maybe is more flexible longer term. Let's maybe see what maintainers think a potential way forward might be (I'm new here, first pr to the repo, so will defer to others).

not sure if there is a draft pr anywhere?

Nop I don't think that we got any PR on the subject.

Eg maybe a trade off between small incremental change vs a bigger one that maybe is more flexible longer term. Let's maybe see what maintainers think a potential way forward might be (I'm new here, first pr to the repo, so will defer to others).

Basically the API in the issue is much better suited than going on the n_features line because we cannot use the code as-is.

@glemaitre 👋 i made some changes here to switch to feature_indices approach that i think matches sort of api desired in #18694

this way user can just pass in a random list of feature indecies if they want or a more specific one if they only want to look at a specific feature or set of sub features.

just went with feature_indices name but maybe there is a better more conventional name for similar params elsewhere perhaps?

@glemaitre 👋 do you think the updated approach in this pr to use the more flexible API is a way forward?

I'm a little bit blocked by this in work so keen to try push to see if feasible to get something merged in the next little while or if I'll need to maybe try something else in work as a work around.

@glemaitre any idea if I can find someone to try help me get this reviewed and merged over next few weeks?

@glemaitre any idea if i can try make another run at seeing if feasible to consider getting this merged?