Talip7/scikit-learn-issues-multilabel huggingface dataset:
`_weighted_percentile` NaN handling with array API

There isn't *necessarily* anything to fix here, but I thought it would be useful to open this for documentation, at least.

---

`_weighted_percentile` added support for NaN in #29034 and support for array APIs in #29431.

Our implementation relys on `sort` putting NaN values at the end:

https://github.com/scikit-learn/scikit-learn/blob/8cfc72b81f7f19a03b5316440efc7d6bebd3c27c/sklearn/utils/stats.py#L70-L74

AFAICT (confirmed by @ev-br) array API specs do not specify how `sort` should handle NaN, which means it is left to individual packages to determine.

* torch seems to follow numpy and sort NaN to the end (tested manually with `float('nan')` and `torch.nan`) but this is not mentioned in the [docs](https://docs.pytorch.org/docs/stable/generated/torch.sort.html). There is some discussion of ordering NaN as the largest value here: https://github.com/pytorch/pytorch/issues/46544#issuecomment-883356705 and a related issue about negative NaN here: https://github.com/pytorch/pytorch/issues/116567
* CuPy seems to follow numpy behaviour as well (relevant issues: https://github.com/cupy/cupy/issues/3324, and they seem to have [tests](https://github.com/cupy/cupy/blob/66820586ee1c41013868a8de4977c84f29180bc8/tests/cupy_tests/sorting_tests/test_sort.py#L161) to check that their results are the same as numpy with nan sorting )

As everything works, I don't think we need to do anything here (especially as we ultimately want to drop maintaining our own quantile function), but just thought it would be useful to document.

cc @StefanieSenger @ogrisel

COMMENT:
I see now, and I have also missed that note.

I agree that since the currently supported array libraries in scikit-learn all sort NaNs to the end (that is with the `C` order(?)), we're fine for now, and in case a new array library would handle this differently, there will be many places where this problem would pop up and then this will be re-discussed.

Thanks for looking into this! ❤

Labels: Array API
`_weighted_percentile` NaN handling with array API

There isn't *necessarily* anything to fix here, but I thought it would be useful to open this for documentation, at least.

---

`_weighted_percentile` added support for NaN in #29034 and support for array APIs in #29431.

Our implementation relys on `sort` putting NaN values at the end:

https://github.com/scikit-learn/scikit-learn/blob/8cfc72b81f7f19a03b5316440efc7d6bebd3c27c/sklearn/utils/stats.py#L70-L74

AFAICT (confirmed by @ev-br) array API specs do not specify how `sort` should handle NaN, which means it is left to individual packages to determine.

* torch seems to follow numpy and sort NaN to the end (tested manually with `float('nan')` and `torch.nan`) but this is not mentioned in the [docs](https://docs.pytorch.org/docs/stable/generated/torch.sort.html). There is some discussion of ordering NaN as the largest value here: https://github.com/pytorch/pytorch/issues/46544#issuecomment-883356705 and a related issue about negative NaN here: https://github.com/pytorch/pytorch/issues/116567
* CuPy seems to follow numpy behaviour as well (relevant issues: https://github.com/cupy/cupy/issues/3324, and they seem to have [tests](https://github.com/cupy/cupy/blob/66820586ee1c41013868a8de4977c84f29180bc8/tests/cupy_tests/sorting_tests/test_sort.py#L161) to check that their results are the same as numpy with nan sorting )

As everything works, I don't think we need to do anything here (especially as we ultimately want to drop maintaining our own quantile function), but just thought it would be useful to document.

cc @StefanieSenger @ogrisel

COMMENT:
> that is with the C order(?)

we assume arrays are stored by default C-contiguous (vs F-contiguous) (I don't have a default reference explaining this but you should be able to find articles on this).
I vaguely recall a note somewhere in the code about us assuming arrays are C-contiguous but I can't find it now.

Labels: Array API
`_weighted_percentile` NaN handling with array API

There isn't *necessarily* anything to fix here, but I thought it would be useful to open this for documentation, at least.

---

`_weighted_percentile` added support for NaN in #29034 and support for array APIs in #29431.

Our implementation relys on `sort` putting NaN values at the end:

https://github.com/scikit-learn/scikit-learn/blob/8cfc72b81f7f19a03b5316440efc7d6bebd3c27c/sklearn/utils/stats.py#L70-L74

AFAICT (confirmed by @ev-br) array API specs do not specify how `sort` should handle NaN, which means it is left to individual packages to determine.

* torch seems to follow numpy and sort NaN to the end (tested manually with `float('nan')` and `torch.nan`) but this is not mentioned in the [docs](https://docs.pytorch.org/docs/stable/generated/torch.sort.html). There is some discussion of ordering NaN as the largest value here: https://github.com/pytorch/pytorch/issues/46544#issuecomment-883356705 and a related issue about negative NaN here: https://github.com/pytorch/pytorch/issues/116567
* CuPy seems to follow numpy behaviour as well (relevant issues: https://github.com/cupy/cupy/issues/3324, and they seem to have [tests](https://github.com/cupy/cupy/blob/66820586ee1c41013868a8de4977c84f29180bc8/tests/cupy_tests/sorting_tests/test_sort.py#L161) to check that their results are the same as numpy with nan sorting )

As everything works, I don't think we need to do anything here (especially as we ultimately want to drop maintaining our own quantile function), but just thought it would be useful to document.

cc @StefanieSenger @ogrisel

COMMENT:
Yea, I had read about this, but I think somewhere in numpy's docs. It's a question on what language (C or Fortran) the way of storing arrays is optimised for. I didn't find any directive on how scikit-learn deals with this. For the time being, I will just accept that we prefer C-ordered arrays then.

Labels: Array API
`_weighted_percentile` NaN handling with array API

There isn't *necessarily* anything to fix here, but I thought it would be useful to open this for documentation, at least.

---

`_weighted_percentile` added support for NaN in #29034 and support for array APIs in #29431.

Our implementation relys on `sort` putting NaN values at the end:

https://github.com/scikit-learn/scikit-learn/blob/8cfc72b81f7f19a03b5316440efc7d6bebd3c27c/sklearn/utils/stats.py#L70-L74

AFAICT (confirmed by @ev-br) array API specs do not specify how `sort` should handle NaN, which means it is left to individual packages to determine.

* torch seems to follow numpy and sort NaN to the end (tested manually with `float('nan')` and `torch.nan`) but this is not mentioned in the [docs](https://docs.pytorch.org/docs/stable/generated/torch.sort.html). There is some discussion of ordering NaN as the largest value here: https://github.com/pytorch/pytorch/issues/46544#issuecomment-883356705 and a related issue about negative NaN here: https://github.com/pytorch/pytorch/issues/116567
* CuPy seems to follow numpy behaviour as well (relevant issues: https://github.com/cupy/cupy/issues/3324, and they seem to have [tests](https://github.com/cupy/cupy/blob/66820586ee1c41013868a8de4977c84f29180bc8/tests/cupy_tests/sorting_tests/test_sort.py#L161) to check that their results are the same as numpy with nan sorting )

As everything works, I don't think we need to do anything here (especially as we ultimately want to drop maintaining our own quantile function), but just thought it would be useful to document.

cc @StefanieSenger @ogrisel

COMMENT:
Coming back to this discussion and just to clarify something: the nan-ordering (nan values pushed to start or end of the sorted array) and the default memory layout (rowwise or columnwise contiguity) are two distinct questions, but both impact how correct our array API support: our code makes assumptions at different places about one or the other: we assume that other array API namespaces behave like numpy does: allocate 2D arrays with rowwise contiguity (C-contiguous) and sort nan values in the end positions of the sorted array.

The array API spec does not guarantee that all namespaces will behave this way for either question. Still, according to our existing tests, this is the case for all namespaces we tested so far.

The correct fix would be to inspect that this actually the case and adapt the code accordingly if it is not, but this would introduce complex code branches for hypothetical namespaces we have never encountered yet and that therefore might not exist.

I think we can leave our code this way and rely on our tests to check if this will always be the case for other namespaces: it's quite easy to tweak the test suite to launch it against a new namespace if we want to make sure that it works as expected with scikit-learn.

Labels: Array API
Inconsistent `median`/`quantile` behaviour now `_weighted_percentile` ignores NaNs

As of https://github.com/scikit-learn/scikit-learn/pull/29034, `_weighted_percentile` handles NaNs by ignoring them when calculating `percentile`.
`np.median` and `np.percentile` on the other hand, will return NaN if a NaN is present in the input (`np.nanmedian` and `np.nanpercentile` will ignore nans).

There are many cases in the codebase where, if `sample_weight` is `None`, a `np` function is used (NaN returned), if `sample_weight` is given, `_weighted_percentile` used and NaNs ignored.

Summary of affected cases:

* `DummyRegressor.fit`
* `AbsoluteError`/`PinballLoss`/`HuberLoss`  - `fit_intercept_only` method
* `median_absolute_error`
* `d2_pinball_score`
* `SplineTransformer._get_base_knot_positions` - I think this was the original reason for https://github.com/scikit-learn/scikit-learn/pull/29034

Maybe we could assess on a case by case basis whether it makes sense to return NaN if present in the input? @ogrisel suggested that we may want to raise a warning in some cases as well.

cc @StefanieSenger

COMMENT:
\take

Labels: module:utils
Inconsistent `median`/`quantile` behaviour now `_weighted_percentile` ignores NaNs

As of https://github.com/scikit-learn/scikit-learn/pull/29034, `_weighted_percentile` handles NaNs by ignoring them when calculating `percentile`.
`np.median` and `np.percentile` on the other hand, will return NaN if a NaN is present in the input (`np.nanmedian` and `np.nanpercentile` will ignore nans).

There are many cases in the codebase where, if `sample_weight` is `None`, a `np` function is used (NaN returned), if `sample_weight` is given, `_weighted_percentile` used and NaNs ignored.

Summary of affected cases:

* `DummyRegressor.fit`
* `AbsoluteError`/`PinballLoss`/`HuberLoss`  - `fit_intercept_only` method
* `median_absolute_error`
* `d2_pinball_score`
* `SplineTransformer._get_base_knot_positions` - I think this was the original reason for https://github.com/scikit-learn/scikit-learn/pull/29034

Maybe we could assess on a case by case basis whether it makes sense to return NaN if present in the input? @ogrisel suggested that we may want to raise a warning in some cases as well.

cc @StefanieSenger

COMMENT:
/take

Labels: module:utils
Inconsistent `median`/`quantile` behaviour now `_weighted_percentile` ignores NaNs

As of https://github.com/scikit-learn/scikit-learn/pull/29034, `_weighted_percentile` handles NaNs by ignoring them when calculating `percentile`.
`np.median` and `np.percentile` on the other hand, will return NaN if a NaN is present in the input (`np.nanmedian` and `np.nanpercentile` will ignore nans).

There are many cases in the codebase where, if `sample_weight` is `None`, a `np` function is used (NaN returned), if `sample_weight` is given, `_weighted_percentile` used and NaNs ignored.

Summary of affected cases:

* `DummyRegressor.fit`
* `AbsoluteError`/`PinballLoss`/`HuberLoss`  - `fit_intercept_only` method
* `median_absolute_error`
* `d2_pinball_score`
* `SplineTransformer._get_base_knot_positions` - I think this was the original reason for https://github.com/scikit-learn/scikit-learn/pull/29034

Maybe we could assess on a case by case basis whether it makes sense to return NaN if present in the input? @ogrisel suggested that we may want to raise a warning in some cases as well.

cc @StefanieSenger

COMMENT:
For metric function we need to be consistent with the other regression methods. I have the feeling that most of them would raise a ValueError but i am not sure.

Labels: module:utils
Inconsistent `median`/`quantile` behaviour now `_weighted_percentile` ignores NaNs

As of https://github.com/scikit-learn/scikit-learn/pull/29034, `_weighted_percentile` handles NaNs by ignoring them when calculating `percentile`.
`np.median` and `np.percentile` on the other hand, will return NaN if a NaN is present in the input (`np.nanmedian` and `np.nanpercentile` will ignore nans).

There are many cases in the codebase where, if `sample_weight` is `None`, a `np` function is used (NaN returned), if `sample_weight` is given, `_weighted_percentile` used and NaNs ignored.

Summary of affected cases:

* `DummyRegressor.fit`
* `AbsoluteError`/`PinballLoss`/`HuberLoss`  - `fit_intercept_only` method
* `median_absolute_error`
* `d2_pinball_score`
* `SplineTransformer._get_base_knot_positions` - I think this was the original reason for https://github.com/scikit-learn/scikit-learn/pull/29034

Maybe we could assess on a case by case basis whether it makes sense to return NaN if present in the input? @ogrisel suggested that we may want to raise a warning in some cases as well.

cc @StefanieSenger

COMMENT:
@ogrisel good point. All the regression metrics give error as we do `check_array` on them (with `ensure_all_finite`), so it doesn't matter what `_weighted_percentile` does, it won't ever receive input with NaNs.

Similarly, `check_array` is done on input passed to `_weighted_percentile` in `DummyRegressor.fit`. `SplineTransformer` also does not but I see NaN support is being added in https://github.com/scikit-learn/scikit-learn/pull/28043, and it's handled correctly in the PR.

That leaves `AbsoluteError`/`PinballLoss`/`HuberLoss` which are used in `GradientBoosting*` - which does not support NaNs. `HistGradientBoostingRegressor` does support NaNs and allows `AbsoluteError` but NaNs are handled before the loss function sees it.

Closing this as a non-issue, sorry for the noise!

Labels: module:utils
TargetEncoder example code

### Describe the issue linked to the documentation

The example used in the [stable TargetEncoder documentation](https://scikit-learn.org/stable/modules/generated/sklearn.preprocessing.TargetEncoder.html) is confusing as the order of the label (that is: dog, cat, snake) is not coherent with the expected order of `enc_low_smooth.encodings_` (the 80 corresponds to 'dog' but is is in second order not first). 

Printing `TargetEncoder.categories_` reveal that the order is indeed coherent with `TargetEncoder.encodings_`. However, as I was trying to understand where this difference of order came from, I wasn't able to find in [TargetEncoder class definition](https://github.com/scikit-learn/scikit-learn/blob/main/sklearn/preprocessing/_target_encoder.py) where `self.categories_` was set.  

### Suggest a potential alternative/fix

- make it more explicit in documentation, such as adding a print of `enc_auto.categories_`
- make `TargetEncoder()` preserve the columns order found in the dataset

COMMENT:
I suspect it is because string classes are sorted lexicographically, and it has caused confusion before thus the related issues:
https://github.com/scikit-learn/scikit-learn/issues/14954, https://github.com/scikit-learn/scikit-learn/issues/13631

In the interest of housekeeping, I would be for closing this issue as duplicate.

(@mathisdrn feel free to add to existing issues, or re-open if you disagree)

Labels: Documentation, Needs Triage
Tfidf no genera los cluster correctos para oraciones con poco significado y palabras repetidas

### Describe the workflow you want to enable

Dado el sigueinte csv:
texto,categoria
"el gato el gato el gato el gato el gato","gato"
"el perro el perro el perro el perro el perro","perro"
"la casa la casa la casa la casa la casa","casa"
"el avión el avión el avión el avión el avión","avión"
"la playa la playa la playa la playa la playa","playa"
"el gato el perro el gato el perro el gato","mezcla"
"el perro el gato el perro el gato el perro","mezcla"
"la playa la casa la playa la casa la playa","mezcla"
Al usar tfidf con stop words y ngramas el cluster de la ultima oracion de nuestro csv no lo agrupa en el cluster correcto, que en este caso deberia estar con la oracion 6 y 7

### Describe your proposed solution

Podemos mencionar las limitaciones con textos repetidos en la documentacion o mejorar los calculos para poder manejar textos con poco significado semantico y palabras repetidas.

### Describe alternatives you've considered, if relevant

_No response_

### Additional context

_No response_

COMMENT:
I am afraid I am going to close this one. Feel free to reopen if you translate your question in English.

Labels: New Feature, Needs Triage
Describe `set_{method}_request()` API, expose `_MetadataRequester`, or expose `_BaseScorer`

### Describe the issue linked to the documentation

TL;DR: Metadata routing for scoring could either use a base class or documentation of how to write `set_score_request()`.

Currently the [Metadata Estimator Dev Guide](https://scikit-learn.org/stable/auto_examples/miscellaneous/plot_metadata_routing.html#metadata-routing) has examples of a metadata-consuming estimator and a metadata-routing estimator.  However, the metadata routing is also designed for scorers and CV splitters which may or may not be estimators.   Fortunately, `sklearn.model_selection` exposes `BaseCrossValidator`, which like `BaseEstimator`, subclasses `_MetadataRequester`.  Unfortunately, ~there's no base class for scorers.~ the base class for scorers, `_BaseScorer`, is not public.

I don't understand how to string together the relevant methods that should be a part of `set_score_params`, The current workaround is to simply subclass `BaseEstimator`, even if I'm not making an estimator, or to subclass `_MetadataRequester`, even though its not part of the public API.  ~Or use `make_scorer` to pin the kwargs when instantiating the meta-estimator, rather than in `fit()`~

My use case is for scoring a time-series model where the data generating mechanism is known to the experiment, but not the model, and I need to compare the fit model to the true data generating mechanism.  I understand how to use a custom scorer in `RandomizedSearchCV`, and the [metadata API](https://scikit-learn.org/stable/metadata_routing.html#api-interface) explains how meta estimators like `RandomizedSearchCV` can pass additional arguments to my custom scorer.

### Suggest a potential alternative/fix

* publicly exposing `_MetadataRequester`
* publicly exposing `_BaseScorer`
* Document how to write the `set_{method}_request()` methods.  It looks like `_MetadataRequester` uses a descriptor `RequestMethod`, which relies on an instance having a `_get_metadata_request` method and a `_metadata_request` attribute (which it doesn't really describe).

EDIT: Found out about `_BaseScorer`.  Also, removed `make_scorer` workaround, as it doesn't result in passing the fitted estimator to the `_score_func`

COMMENT:
/take

Labels: Documentation, Needs Investigation, Metadata Routing
Describe `set_{method}_request()` API, expose `_MetadataRequester`, or expose `_BaseScorer`

### Describe the issue linked to the documentation

TL;DR: Metadata routing for scoring could either use a base class or documentation of how to write `set_score_request()`.

Currently the [Metadata Estimator Dev Guide](https://scikit-learn.org/stable/auto_examples/miscellaneous/plot_metadata_routing.html#metadata-routing) has examples of a metadata-consuming estimator and a metadata-routing estimator.  However, the metadata routing is also designed for scorers and CV splitters which may or may not be estimators.   Fortunately, `sklearn.model_selection` exposes `BaseCrossValidator`, which like `BaseEstimator`, subclasses `_MetadataRequester`.  Unfortunately, ~there's no base class for scorers.~ the base class for scorers, `_BaseScorer`, is not public.

I don't understand how to string together the relevant methods that should be a part of `set_score_params`, The current workaround is to simply subclass `BaseEstimator`, even if I'm not making an estimator, or to subclass `_MetadataRequester`, even though its not part of the public API.  ~Or use `make_scorer` to pin the kwargs when instantiating the meta-estimator, rather than in `fit()`~

My use case is for scoring a time-series model where the data generating mechanism is known to the experiment, but not the model, and I need to compare the fit model to the true data generating mechanism.  I understand how to use a custom scorer in `RandomizedSearchCV`, and the [metadata API](https://scikit-learn.org/stable/metadata_routing.html#api-interface) explains how meta estimators like `RandomizedSearchCV` can pass additional arguments to my custom scorer.

### Suggest a potential alternative/fix

* publicly exposing `_MetadataRequester`
* publicly exposing `_BaseScorer`
* Document how to write the `set_{method}_request()` methods.  It looks like `_MetadataRequester` uses a descriptor `RequestMethod`, which relies on an instance having a `_get_metadata_request` method and a `_metadata_request` attribute (which it doesn't really describe).

EDIT: Found out about `_BaseScorer`.  Also, removed `make_scorer` workaround, as it doesn't result in passing the fitted estimator to the `_score_func`

COMMENT:
@MagicDake thanks for your enthusiasm! However I am not sure this issue is yet ready for someone to go implement a solution. The suggested actions read (to me) like a list of alternatives - so the first step here is to have a discussion about the options and form a consensus. One way to do that is for you to describe how you understand the issue and its solution(s).

Labels: Documentation, Needs Investigation, Metadata Routing