`_safe_indexing` triggers `SettingWithCopyWarning` when used with `slice`

### Describe the bug

Here's something I noticed while looking into https://github.com/scikit-learn/scikit-learn/pull/31127

The test
```
pytest sklearn/utils/tests/test_indexing.py::test_safe_indexing_pandas_no_settingwithcopy_warning
```
checks that a copy is produced, and that no `SettingWithCopyWarning` is produced

Indeed, no copy is raised, but why is using `_safe_indexing` with a slice allowed to not make a copy? Is this intentional?

Based on responses, I can suggest what to do instead in https://github.com/scikit-learn/scikit-learn/pull/31127

(I am a little surprised that this always makes copies, given that a lot of the discussion in https://github.com/scikit-learn/scikit-learn/issues/28341 centered around wanting to avoid copies)

### Steps/Code to Reproduce

```python
import numpy as np

from sklearn.utils import _safe_indexing
import pandas as pd

X = pd.DataFrame({"a": [1, 2, 3], "b": [3, 4, 5]})
subset = _safe_indexing(X, slice(0, 2), axis=0)
subset.iloc[0, 0] = 10
```

### Expected Results

No `SettingWithCopyWarning`

### Actual Results

```
/home/marcogorelli/scikit-learn-dev/t.py:13: SettingWithCopyWarning: 
A value is trying to be set on a copy of a slice from a DataFrame

See the caveats in the documentation: https://pandas.pydata.org/pandas-docs/stable/user_guide/indexing.html#returning-a-view-versus-a-copy
  subset.iloc[0, 0] = 10
```

### Versions

```shell
System:
    python: 3.11.11 (main, Dec  4 2024, 08:55:07) [GCC 11.4.0]
executable: /home/marcogorelli/scikit-learn-dev/.venv/bin/python
   machine: Linux-5.15.167.4-microsoft-standard-WSL2-x86_64-with-glibc2.35

Python dependencies:
      sklearn: 1.7.dev0
          pip: 24.2
   setuptools: None
        numpy: 2.1.0
        scipy: 1.14.0
       Cython: 3.0.11
       pandas: 2.2.2
   matplotlib: None
       joblib: 1.4.2
threadpoolctl: 3.5.0

Built with OpenMP: True

threadpoolctl info:
       user_api: blas
   internal_api: openblas
    num_threads: 16
         prefix: libscipy_openblas
       filepath: /home/marcogorelli/scikit-learn-dev/.venv/lib/python3.11/site-packages/numpy.libs/libscipy_openblas64_-ff651d7f.so
        version: 0.3.27
threading_layer: pthreads
   architecture: SkylakeX

       user_api: blas
   internal_api: openblas
    num_threads: 16
         prefix: libscipy_openblas
       filepath: /home/marcogorelli/scikit-learn-dev/.venv/lib/python3.11/site-packages/scipy.libs/libscipy_openblas-c128ec02.so
        version: 0.3.27.dev
threading_layer: pthreads
   architecture: SkylakeX

       user_api: openmp
   internal_api: openmp
    num_threads: 16
         prefix: libgomp
       filepath: /usr/lib/x86_64-linux-gnu/libgomp.so.1.0.0
        version: None
```

COMMENT:
Let me jump in with a couple of thoughts and historical views.

Let's put aside the `slice` part to better grasp the expectation behind `_safe_indexing`. From the top of the head, the 99% of use-cases is along the line: get some rows to work with them without altering them. So a kind of read-only subsampling and thus no need for copying would be the best option. However, I don't think that we never implemented a defensive read-only mechanism (via a flag or something similar) and we most probably expect a defensive copy.

The 1% that I'm aware in scikit-learn that would need to modify `X` is the permutation importance algorithm where we want to make a copy and permute the rows of a column:

https://github.com/scikit-learn/scikit-learn/issues/31284

Now about the `slice`: 100% of the internal use cases only rely on array-like indexing and do not use slices. I think that slice in `_safe_indexing` was supported because we wanted to have the same indexing capabilities than NumPy arrays. When it comes to the public API, `slice` would only be accepted (as far I can recall) in `ColumnTransformer` where columns can be specified with a `slice` to dispatch them into a transformer.

So to come back on the original test that could fail, it was written when dealing with sampling during cross-validation and `slice` are not accepted in this case. Therefore, it never has been tested in terms of unit tests.

So for the time being, I would maybe suggest to:

- trigger a copy for `slice` to have the same behaviour than other indexing method
- question whether or not we should deprecate this option. I would guess that using slice with integer to select dataset should not be the most used feature and if it used with string, then I would almost consider it as a bad practice because it is extremely implicit regarding the column selected.

Labels: Bug