We should as well check with upstream if the change is intended. It will break older scikit-learn versions with the newest SciPy version without a deprecation warning.

This was a rather surprising change. Similar problem w/ utils.tests.test_extmath::test_uniform_weights

For reference here is a snippet of the test failure caused by this breaking change in scipy.stats.mode:

    def _most_frequent(array, extra_value, n_repeat):
        """Compute the most frequent value in a 1d array extended with
        [extra_value] * n_repeat, where extra_value is assumed to be not part
        of the array."""
        # Compute the most frequent value in array only
        if array.size > 0:
            if array.dtype == object:
                # scipy.stats.mode is slow with object dtype array.
                # Python Counter is more efficient
                counter = Counter(array)
                most_frequent_count = counter.most_common(1)[0][1]
                # tie breaking similarly to scipy.stats.mode
                most_frequent_value = min(
                    value
                    for value, count in counter.items()
                    if count == most_frequent_count
                )
            else:
                mode = stats.mode(array)
>               most_frequent_value = mode[0][0]
E               IndexError: invalid index to scalar variable.

array      = array([1., 1., 1.])
extra_value = nan
mode       = ModeResult(mode=1.0, count=3)
n_repeat   = 0

In scipy/scipy#16418 it is argued that this change of behavior is considered a bugfix. So if scipy 1.9 gets released without a deprecation cycle, we will need to quickly release scikit-learn 1.1.2 with a backport of this fix and #23640.

I am not sure how this fix works, since depending on the scipy version, most_frequent_value and most_frequent_count will be a numpy scalar (scipy >= 1.9) or a numpy 1d array with a single value (scipy < 1.9).

Should we not do a sklearn.util.fixes.mode so that all the call of scipy.stats.mode gets fixed in a uniform manner?

Should we not do a sklearn.util.fixes.mode so that all the call of scipy.stats.mode gets fixed in a uniform manner?

This is a good suggestion. #23640 will need to be updated accordingly. Maybe let's do both in the same PR.

Indeed, this is quite funny that the current CIs do not fail. I assume that they should kind of ravel the 1-element array.

I need the change introduced by @Micky774 in #23640 as well to adopt the test.
One question is: do we want to modify weighted_mode to make is as well consistent with mode from SciPy? We would need to deprecate the behaviour so it could be part of another PR anyway.

One question is: do we want to modify weighted_mode to make is as well consistent with mode from SciPy? We would need to deprecate the behaviour so it could be part of another PR anyway.

I believe so. +1 for separate the PR to make backporting the basic compat fix to 1.1.2 easier if needed.

It should be ready for reviews.

Note that this PR will need to be adapted depending on the result of the discussion with upstream:

• BUG: breaking change: scipy.stats.mode returned value has changed shape without deprecation scipy/scipy#16418 (comment)

I put it WIP until we know what happens upstream.

SciPy moved forward with a solution here: scipy/scipy#16429

Cool. I will remove the backport and just make the change in the code then.