Talip7/scikit-learn-issues-multilabel huggingface dataset:
RFC adopt narwhals for dataframe support

At least as of [SLEP018](https://scikit-learn-enhancement-proposals.readthedocs.io/en/latest/slep018/proposal.html), scikit-learn supports dataframes passed as `X`. In #25896 is a further place of current discussions.

This issue is to discuss whether or not, or in which form, a future scikit-learn should depend on [narwhals](https://github.com/narwhals-dev/narwhals) for general dataframe support.

`+` wide df support
`+` less maintenance within scikit-learn
`-` external dependency

@scikit-learn/core-devs @MarcoGorelli

COMMENT:
> Once that's out the door, I'll turn my focus to this issue

Alright, got stable.v2 out, sorted out some other things, and got married + honeymooned, so it's now time to get back into this!

I've added a commit to https://github.com/scikit-learn/scikit-learn/pull/31127, and it looks like it's now green (apart from [macOS pylatest_conda_forge_arm](https://github.com/scikit-learn/scikit-learn/actions/runs/19765174226/job/56636173202?pr=31127#logs), but I think that's unrelated?)

Between vendoring and adding as dependency, I'd advocate that the latter provides a better user experience:
- Narwhals is tested against nightly builds of all major dataframe libraries, so this means it can be adapted (if need be) as quickly as possible. We release frequently (usually weekly), and users can easily `pip install -U narwhals` if necessary to get latest fixes / improvements
- If there's an issue with vendored code, however, users need to wait for a new scikit-learn release, which usually takes several months

Regarding stability, Nathan from Plotly [publicly vouched for Narwhals](https://github.com/mlflow/mlflow/issues/16518#issuecomment-3133109741):

> can vouch for stability and backwards compatibility of Narwhals based on our experience integrating into plotly.py (see plotly/plotly.py#4790). You can read a bit more about our outcomes migrating from dataframe interchange to Narwhals here: https://plotly.com/blog/chart-smarter-not-harder-universal-dataframe-support/

We're all fans of scikit-learn in Narwhals and would be committed to providing support as best we can

Labels: RFC
RFC adopt narwhals for dataframe support

At least as of [SLEP018](https://scikit-learn-enhancement-proposals.readthedocs.io/en/latest/slep018/proposal.html), scikit-learn supports dataframes passed as `X`. In #25896 is a further place of current discussions.

This issue is to discuss whether or not, or in which form, a future scikit-learn should depend on [narwhals](https://github.com/narwhals-dev/narwhals) for general dataframe support.

`+` wide df support
`+` less maintenance within scikit-learn
`-` external dependency

@scikit-learn/core-devs @MarcoGorelli

COMMENT:
> A lot more people now use polars, and our code breaks here and there with it, and we need to do continuous patch work to fix those issues. Not adopting narwhals is adding quite a bit of technical debt to our library at this point which I'm not very happy about.

Do we know for a fact that adopting narwhals will fix these problems?

In skrub, we keep running in challenges that cannot be magically solved, because of fundamental differences in the models underlying pandas and polars.

Labels: RFC
RFC adopt narwhals for dataframe support

At least as of [SLEP018](https://scikit-learn-enhancement-proposals.readthedocs.io/en/latest/slep018/proposal.html), scikit-learn supports dataframes passed as `X`. In #25896 is a further place of current discussions.

This issue is to discuss whether or not, or in which form, a future scikit-learn should depend on [narwhals](https://github.com/narwhals-dev/narwhals) for general dataframe support.

`+` wide df support
`+` less maintenance within scikit-learn
`-` external dependency

@scikit-learn/core-devs @MarcoGorelli

COMMENT:
The linked PR keeps splits `_safe_indexing` into:
- a pandas-specific part
- a Narwhals part, which can handle Polars / PyArrow

Other projects have done similar things, like Altair, who have a [pandas-specific function](https://github.com/vega/altair/blob/6667dc4f78cdf2e08c1f7e2eb95568fcbc370977/altair/utils/core.py#L333-L462) as well as a more general Narwhals one

For backwards-compatibility, and to work around historical design decisions in pandas which other libraries have avoided (such as the infamous `SettingWithCopyWarning`, #31290 ), some pandas-specific paths are sometimes warranted

> Do we know for a fact that adopting narwhals will fix these problems?

I don't know about the current issues, but it would've solved https://github.com/scikit-learn/scikit-learn/pull/31040/files#diff-962bea3f949b0a1e7b2ad40d2879f5d7ba1e6cd7da168daa82f84a87e800715e without needing an extra PyArrow-specific path - note how in #31127 , `_polars_indexing` and `_pyarrow_indexing` get merged into a single `_narwhals_indexing`

There may well be more though - when Plotly switched to Narwhals, four historic bugs got fixed for free: https://github.com/plotly/plotly.py/pull/4921

Labels: RFC
RFC Move SLEPs to the main scikit-learn website

## Background 
The website for scikit-learn enhancement proposals (SLEP) at https://scikit-learn-enhancement-proposals.readthedocs.io/ is very hard to find if you don't know what you are looking for. A second difficulty is to know which SLEP is (fully) implemented in which scikit-learn release, see, e.g., #31037.

## Proposition
Move SLEP website to the main scikit-learn website at https://scikit-learn.org.

@scikit-learn/core-devs @scikit-learn/communication-team @scikit-learn/documentation-team ping

COMMENT:
I'm +1 on this, and the migration seems pretty straightforward. With SLEP on main website cross-refering is much less error-prone (developer perspective) and searching is much easier (user perspective).

Labels: Documentation, RFC
RFC Move SLEPs to the main scikit-learn website

## Background 
The website for scikit-learn enhancement proposals (SLEP) at https://scikit-learn-enhancement-proposals.readthedocs.io/ is very hard to find if you don't know what you are looking for. A second difficulty is to know which SLEP is (fully) implemented in which scikit-learn release, see, e.g., #31037.

## Proposition
Move SLEP website to the main scikit-learn website at https://scikit-learn.org.

@scikit-learn/core-devs @scikit-learn/communication-team @scikit-learn/documentation-team ping

COMMENT:
+1

Labels: Documentation, RFC
RFC Move SLEPs to the main scikit-learn website

## Background 
The website for scikit-learn enhancement proposals (SLEP) at https://scikit-learn-enhancement-proposals.readthedocs.io/ is very hard to find if you don't know what you are looking for. A second difficulty is to know which SLEP is (fully) implemented in which scikit-learn release, see, e.g., #31037.

## Proposition
Move SLEP website to the main scikit-learn website at https://scikit-learn.org.

@scikit-learn/core-devs @scikit-learn/communication-team @scikit-learn/documentation-team ping

COMMENT:
I'm +1 on the intent.

In terms of realization, I think that I would prefer if we would try to use a separate repo and build mechanism, for multiple reasons. It can probably be done via a github pages hosting at scikit-learn.org/enhancement-proposals (or similar)

Labels: Documentation, RFC
RFC Move SLEPs to the main scikit-learn website

## Background 
The website for scikit-learn enhancement proposals (SLEP) at https://scikit-learn-enhancement-proposals.readthedocs.io/ is very hard to find if you don't know what you are looking for. A second difficulty is to know which SLEP is (fully) implemented in which scikit-learn release, see, e.g., #31037.

## Proposition
Move SLEP website to the main scikit-learn website at https://scikit-learn.org.

@scikit-learn/core-devs @scikit-learn/communication-team @scikit-learn/documentation-team ping

COMMENT:
I would try to fix the mentioned issues differently (possibly in a simpler manner):
- discoverability: add a link from scikit-learn.org somewhere (headers/footers/somewhere else?).
- which version is the SLEP implemented in: an additional field in the SLEP document?

It would be nice to clarify whether you mean the SLEP rst should live in the main scikit-learn repo (seems like it would add more friction to SLEP which I don't think is needed) or whether only the SLEP HTML should be available under the `scikit-learn.org` URL (Gaël's suggestion in https://github.com/scikit-learn/scikit-learn/issues/31039#issuecomment-2742529493).

Labels: Documentation, RFC
⚠️ CI failed on Linux_Runs.pylatest_conda_forge_mkl (last failure: Mar 20, 2025) ⚠️

**CI failed on [Linux_Runs.pylatest_conda_forge_mkl](https://dev.azure.com/scikit-learn/scikit-learn/_build/results?buildId=74894&view=logs&j=dde5042c-7464-5d47-9507-31bdd2ee0a3a)** (Mar 20, 2025)
- Test Collection Failure

COMMENT:
## CI is no longer failing! ✅

[Successful run](https://dev.azure.com/scikit-learn/scikit-learn/_build/results?buildId=74931&view=logs&j=dde5042c-7464-5d47-9507-31bdd2ee0a3a) on Mar 21, 2025

Labels: Needs Triage
⚠️ CI failed on Linux_Runs.pylatest_conda_forge_mkl (last failure: Mar 20, 2025) ⚠️

**CI failed on [Linux_Runs.pylatest_conda_forge_mkl](https://dev.azure.com/scikit-learn/scikit-learn/_build/results?buildId=74894&view=logs&j=dde5042c-7464-5d47-9507-31bdd2ee0a3a)** (Mar 20, 2025)
- Test Collection Failure

COMMENT:
The March 20 failure is another instance of #30761

Labels: Needs Triage
⚠️ CI failed on Linux_Runs.pylatest_conda_forge_mkl (last failure: Mar 20, 2025) ⚠️

**CI failed on [Linux_Runs.pylatest_conda_forge_mkl](https://dev.azure.com/scikit-learn/scikit-learn/_build/results?buildId=74894&view=logs&j=dde5042c-7464-5d47-9507-31bdd2ee0a3a)** (Mar 20, 2025)
- Test Collection Failure

COMMENT:
So closing as a duplicate of that one?

Labels: Needs Triage
`weighted_percentile` should error/warn when all sample weights 0

### Describe the bug

Noticed while working on #29431









### Steps/Code to Reproduce

See the following test:

https://github.com/scikit-learn/scikit-learn/blob/cd0478f42b2c873853e6317e3c4f2793dc149636/sklearn/utils/tests/test_stats.py#L67-L73

### Expected Results

Error or warning should probably be given. You're effectively asking for a quantile of a empty array.

### Actual Results

When all sample weights are 0, what happens is that `percentile_in_sorted` (as in the index of desired observation in array is the) is `101` (the last item). We should probably add a check and give a warning when `sample_weights` is all zero

cc @ogrisel @glemaitre 

### Versions

```shell
n/a
```

COMMENT:
cc @antoinebaker too on this one.

Labels: Bug
`weighted_percentile` should error/warn when all sample weights 0

### Describe the bug

Noticed while working on #29431









### Steps/Code to Reproduce

See the following test:

https://github.com/scikit-learn/scikit-learn/blob/cd0478f42b2c873853e6317e3c4f2793dc149636/sklearn/utils/tests/test_stats.py#L67-L73

### Expected Results

Error or warning should probably be given. You're effectively asking for a quantile of a empty array.

### Actual Results

When all sample weights are 0, what happens is that `percentile_in_sorted` (as in the index of desired observation in array is the) is `101` (the last item). We should probably add a check and give a warning when `sample_weights` is all zero

cc @ogrisel @glemaitre 

### Versions

```shell
n/a
```

COMMENT:
Note that I see this affects nan behaviour as well:

https://github.com/scikit-learn/scikit-learn/blob/68483539614102ba8e083277ed7123e6a9fece53/sklearn/utils/tests/test_stats.py#L297-L301

when all weights are 0, and all array values are np.nan, the result is np.nan (i.e. the last item in array), which I guess kind of makes sense. I still think we should really error when weights are all 0. You can't take a quantile of a non-existant cdf.

cc @StefanieSenger

Labels: Bug
`weighted_percentile` should error/warn when all sample weights 0

### Describe the bug

Noticed while working on #29431









### Steps/Code to Reproduce

See the following test:

https://github.com/scikit-learn/scikit-learn/blob/cd0478f42b2c873853e6317e3c4f2793dc149636/sklearn/utils/tests/test_stats.py#L67-L73

### Expected Results

Error or warning should probably be given. You're effectively asking for a quantile of a empty array.

### Actual Results

When all sample weights are 0, what happens is that `percentile_in_sorted` (as in the index of desired observation in array is the) is `101` (the last item). We should probably add a check and give a warning when `sample_weights` is all zero

cc @ogrisel @glemaitre 

### Versions

```shell
n/a
```

COMMENT:
I can see the problem, @lucyleeow, but I wonder if there is a better way to deal with it than raising an error or a warning.

> You can't take a quantile of a non-existant cdf.

I also think that returning the last value as a result when all `sample_weight` is 0 is somehow unexpected behaviour (the nan-case would be the only case where this maybe makes a bit of sense, but it's not special or an edge case).

But since `_weighted_percentile` is just a helper function, I wonder if it should really be concerned with raising error or wanings? There are two cases:

- If the user passes `sample_weight=np.zeros(X.shape[0])` or so to an estimator, I would expect the estimator's `fit` to raise an error, because that input doesn't make any sense from the beginning.

- But if `_weighted_percentile` was called from inside a cv-splitter or from inside `partial_fit` and the selected samples would just happen to all have a `sample_weight` of 0 each, then I would not want the program to stop running or even to raise a warning (that I would then be concerned about) just because part of the data that is handled in the same batch or split all happened to have `sample_weight` 0? 

It's just odd and wrong that `_weighted_percentile` returns exactly the last value then. I'm just not sure what else it should return instead? NaN?

Labels: Bug
`weighted_percentile` should error/warn when all sample weights 0

### Describe the bug

Noticed while working on #29431









### Steps/Code to Reproduce

See the following test:

https://github.com/scikit-learn/scikit-learn/blob/cd0478f42b2c873853e6317e3c4f2793dc149636/sklearn/utils/tests/test_stats.py#L67-L73

### Expected Results

Error or warning should probably be given. You're effectively asking for a quantile of a empty array.

### Actual Results

When all sample weights are 0, what happens is that `percentile_in_sorted` (as in the index of desired observation in array is the) is `101` (the last item). We should probably add a check and give a warning when `sample_weights` is all zero

cc @ogrisel @glemaitre 

### Versions

```shell
n/a
```

COMMENT:
> I would expect the estimator's fit to raise an error, because that input doesn't make any sense from the beginning

Maybe it is better to catch it earlier, potentially by `_check_sample_weight`, if it does not make sense to have all weights being 0 (can't think of a case where it does but you never know).
I would be nice to have a consistent error message and single source for such an error, though.

> It's just odd and wrong that _weighted_percentile returns exactly the last value then. 

I think that's exactly the problem. If it does return NaN, would the result for that `partial_fit` or split inside cv-splitting, essentially be not informative? On one hand, it may cause problems downstream and it may be better to error early, on the other, if you are e.g., calculating score over cv splits, maybe it's okay to have one that is nan? You couldn't average it without removing this value but it still may be useful.

Labels: Bug
`weighted_percentile` should error/warn when all sample weights 0

### Describe the bug

Noticed while working on #29431









### Steps/Code to Reproduce

See the following test:

https://github.com/scikit-learn/scikit-learn/blob/cd0478f42b2c873853e6317e3c4f2793dc149636/sklearn/utils/tests/test_stats.py#L67-L73

### Expected Results

Error or warning should probably be given. You're effectively asking for a quantile of a empty array.

### Actual Results

When all sample weights are 0, what happens is that `percentile_in_sorted` (as in the index of desired observation in array is the) is `101` (the last item). We should probably add a check and give a warning when `sample_weights` is all zero

cc @ogrisel @glemaitre 

### Versions

```shell
n/a
```

COMMENT:
> Maybe it is better to catch it earlier, potentially by _check_sample_weight, if it does not make sense to have all weights being 0 (can't think of a case where it does but you never know).
I would be nice to have a consistent error message and single source for such an error, though.

Yes, sounds like it makes sense to have it there. 
I also can't think of a good reason to pass all 0s as `sample_weight`. Maybe you would want to switch examples on and off on condition and then you have part of your data come with `sample_weight` of 0, but that's not all data.

> On one hand, it may cause problems downstream and it may be better to error early, on the other, if you are e.g., calculating score over cv splits, maybe it's okay to have one that is nan? You couldn't average it without removing this value but it still may be useful.

I would also think that returning nan in this case is better than returning the last value, because it's more neutral. Also, it is probably a seldom case.

Labels: Bug
`weighted_percentile` should error/warn when all sample weights 0

### Describe the bug

Noticed while working on #29431









### Steps/Code to Reproduce

See the following test:

https://github.com/scikit-learn/scikit-learn/blob/cd0478f42b2c873853e6317e3c4f2793dc149636/sklearn/utils/tests/test_stats.py#L67-L73

### Expected Results

Error or warning should probably be given. You're effectively asking for a quantile of a empty array.

### Actual Results

When all sample weights are 0, what happens is that `percentile_in_sorted` (as in the index of desired observation in array is the) is `101` (the last item). We should probably add a check and give a warning when `sample_weights` is all zero

cc @ogrisel @glemaitre 

### Versions

```shell
n/a
```

COMMENT:
How about:

* Amend `_weighted_percentile` to return `xp.nan` when `sample_weight` are all zeros
* Add option to error when `sample_weight` is all zeros in `_check_sample_weight` (e.g., add `allow_zero_weights` parameter), and use this where it does not make sense to have all zero sample weights

What do we think @StefanieSenger @ogrisel @betatim @antoinebaker ?

Labels: Bug
`weighted_percentile` should error/warn when all sample weights 0

### Describe the bug

Noticed while working on #29431









### Steps/Code to Reproduce

See the following test:

https://github.com/scikit-learn/scikit-learn/blob/cd0478f42b2c873853e6317e3c4f2793dc149636/sklearn/utils/tests/test_stats.py#L67-L73

### Expected Results

Error or warning should probably be given. You're effectively asking for a quantile of a empty array.

### Actual Results

When all sample weights are 0, what happens is that `percentile_in_sorted` (as in the index of desired observation in array is the) is `101` (the last item). We should probably add a check and give a warning when `sample_weights` is all zero

cc @ogrisel @glemaitre 

### Versions

```shell
n/a
```

COMMENT:
Sounds like a good plan to me.

Labels: Bug
`weighted_percentile` should error/warn when all sample weights 0

### Describe the bug

Noticed while working on #29431









### Steps/Code to Reproduce

See the following test:

https://github.com/scikit-learn/scikit-learn/blob/cd0478f42b2c873853e6317e3c4f2793dc149636/sklearn/utils/tests/test_stats.py#L67-L73

### Expected Results

Error or warning should probably be given. You're effectively asking for a quantile of a empty array.

### Actual Results

When all sample weights are 0, what happens is that `percentile_in_sorted` (as in the index of desired observation in array is the) is `101` (the last item). We should probably add a check and give a warning when `sample_weights` is all zero

cc @ogrisel @glemaitre 

### Versions

```shell
n/a
```

COMMENT:
BTW, I tried to fit `LogisticRegression` and `Ridge` on non-empty datasets with 0 weights, and we get low-level, solver-specific error message that make it hard for the users to find out that the root cause is 0-sum weight training sets. So raising a meaning `ValueError` in `_check_sample_weight` would definitely be a nice UX improvement to help debug problems related to this edge-cases.

Labels: Bug
`weighted_percentile` should error/warn when all sample weights 0

### Describe the bug

Noticed while working on #29431









### Steps/Code to Reproduce

See the following test:

https://github.com/scikit-learn/scikit-learn/blob/cd0478f42b2c873853e6317e3c4f2793dc149636/sklearn/utils/tests/test_stats.py#L67-L73

### Expected Results

Error or warning should probably be given. You're effectively asking for a quantile of a empty array.

### Actual Results

When all sample weights are 0, what happens is that `percentile_in_sorted` (as in the index of desired observation in array is the) is `101` (the last item). We should probably add a check and give a warning when `sample_weights` is all zero

cc @ogrisel @glemaitre 

### Versions

```shell
n/a
```

COMMENT:
/take

Labels: Bug
`weighted_percentile` should error/warn when all sample weights 0

### Describe the bug

Noticed while working on #29431









### Steps/Code to Reproduce

See the following test:

https://github.com/scikit-learn/scikit-learn/blob/cd0478f42b2c873853e6317e3c4f2793dc149636/sklearn/utils/tests/test_stats.py#L67-L73

### Expected Results

Error or warning should probably be given. You're effectively asking for a quantile of a empty array.

### Actual Results

When all sample weights are 0, what happens is that `percentile_in_sorted` (as in the index of desired observation in array is the) is `101` (the last item). We should probably add a check and give a warning when `sample_weights` is all zero

cc @ogrisel @glemaitre 

### Versions

```shell
n/a
```

COMMENT:
@j-hendricks it would be better to wait until is #31775 merged before opening a PR for this, though I see you did not add any tests so merge conflicts should not be too bad.

Labels: Bug
`weighted_percentile` should error/warn when all sample weights 0

### Describe the bug

Noticed while working on #29431









### Steps/Code to Reproduce

See the following test:

https://github.com/scikit-learn/scikit-learn/blob/cd0478f42b2c873853e6317e3c4f2793dc149636/sklearn/utils/tests/test_stats.py#L67-L73

### Expected Results

Error or warning should probably be given. You're effectively asking for a quantile of a empty array.

### Actual Results

When all sample weights are 0, what happens is that `percentile_in_sorted` (as in the index of desired observation in array is the) is `101` (the last item). We should probably add a check and give a warning when `sample_weights` is all zero

cc @ogrisel @glemaitre 

### Versions

```shell
n/a
```

COMMENT:
@lucyleeow thanks for the heads up! I just got excited about contributing again haha. I've converted this to a draft PR and will mark it ready for review once #31775 is merged :)

Labels: Bug