ENH/DEPR add new response_method and deprecate needs_* in make_scorer

[glemaitre]

closes #26817

While working in #26120, I find myself doing dirty tricks to overwrite the response method used in _ThresholdScorer while we should be able to pass a list of priorities and delegate to _get_response_values.

I introduce response_method in make_scorer that remove the need to needs_proba and needs_threshold. I assume that the API is simplified due to the work done with _get_response_values. Now, we don't need to have different type of scorers (i.e. _PredictScorer, _ProbaScorer, and ThresholdScorer). All are handled in a single class because all the complexity is delegated to the _get_response_values to which we provide the response method to try.

I need to alternate the provided scorer in the _MultiMetricScorer to be sure to hit the cache. It means that we need to decide on a single method to use for a single estimator. At the end, it simplifies also when the cache policy is beneficial.

[github-actions]

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

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

[glemaitre]

ping @betatim on this one ;)

[betatim]

While working in #26120, I find myself doing dirty tricks to overwrite the response method used in _ThresholdScorer while we should be able to pass a list of priorities and delegate to _get_response_values.

I kinda see why it is needed, but I also kinda don't like having an extra argument to make_scorer that is only used sometimes. Do you need to be able to configure the list or would it be enough to change _ThresholdScorer to use a list of options (the "list of str") instead of just "predict"?

If we settle on adding an argument to make_scorer I think we need some input validation to warn/error when users use it but don't set use_threshold=True (or think about whether it makes sense to all this even if use_threshold=False).

The bug fix part for ClassifierChain I am happy with, just a few style nits.

[thomasjpfan]

Does resolving #26817 require a new respond_method? If they are unrelated, can this be split into two PRs?

• Adding multi-label-indicator support
• Add respond_method to adjust the order for needs_threshold=True

[glemaitre]

I open #27002 to solve the issue of multilabel-indcator on a separate PR. I am putting this PR on draft until merging the other PR.

The other PR will target 1.3.1.

[glemaitre]

Going back to this PR now that we solve the multi-label bug and I need this PR for #26120.

[glemaitre]

Do you need to be able to configure the list or would it be enough to change _ThresholdScorer to use a list of options (the "list of str") instead of just "predict"?

This is exactly this. I would like to favor predict_proba or decision_function depending on the use case.

If we settle on adding an argument to make_scorer I think we need some input validation to warn/error when users use it but don't set use_threshold=True (or think about whether it makes sense to all this even if use_threshold=False).

We can error when passing something to this new argument while not setting use_threshold=True.

[glemaitre]

Maybe @betatim @thomasjpfan you want to have a new look at this one.
@adrinjalali @ogrisel for some extra reviews (I would be fine with only 2 reviewers ;))

[adrinjalali]

I'm finding the combination of needs_proba, needs_threshold and response_method confusing. Seems like with the introduction of response_method we can deprecated one of needs_proba/needs_threshold

Specifically, seems like we can deprecate _ProbaScorer?

I also kinda feel like the changes here aren't tested, but codecov obviously doesn't agree 😁

(didn't mean to "approve" 😝 )

[glemaitre]

I'm finding the combination of needs_proba, needs_threshold and response_method confusing. Seems like with the introduction of response_method we can deprecated one of needs_proba/needs_threshold

I think that we can deprecate both of them indeed: if response_method="predict_proba", then you it would be equivalent to needs_proba=True. If "decision_function" in the response_method

[glemaitre]

Whatever changes below is only because of the new API.

[glemaitre]

I added a couple of comments to help at reviewing this PR.
Usually, the complex part is about the caching mechanism and _MultimetricScorer.

[adrinjalali]

Nice!

[adrinjalali]

Deprecation warnings making tests fail, otherwise LGTM.

[thomasjpfan]

Thanks for the PR! Overall LGTM