Talip7/scikit-learn-issues-multilabel huggingface dataset:
Suggesting updates on the doc of `sklearn.neighbors.NeighborhoodComponentsAnalysis`

### Describe the issue linked to the documentation

Hi,

We are an academic team of software engineering researchers from a university working on automated program analysis techniques to improve API documentation quality, ultimately contributing to improving data science software development practices. we would like to keep anonymity for the purpose of double-blind paper reviewing.

We discover an inconsistency issue between documentation and code in the class [`sklearn.neighbors.NeighborhoodComponentsAnalysis`](https://scikit-learn.org/dev/modules/generated/sklearn.neighbors.NeighborhoodComponentsAnalysis.html#neighborhoodcomponentsanalysis). As mentioned in the description of the value `auto` of parameter `init`:

> -  **'auto'**
Depending on `n_components`, the most reasonable initialization will be chosen. **If `n_components <= n_classes` we use `'lda'`**, as it uses labels information. If not, but `n_components < min(n_features, n_samples)`, we use `'pca'`, as it projects data in meaningful directions (those of higher variance). Otherwise, we just use `'identity'`.

However, the most relevant source code snippet looks like this:

```python
if init == "auto":
    n_classes = len(np.unique(y))
    if n_components <= min(n_features, n_classes - 1):
        init = "lda"
    elif n_components < min(n_features, n_samples):
        init = "pca"
    else:
        init = "identity"
```

Apparently, the condition (**_n_components <= n_classes_**) shown in the document are different from the condition (**_n_components <= min(n_features, n_classes - 1)_**) actually executed in the code.


### Suggest a potential alternative/fix

Maybe you can update the documentation to avoid unnecessary misunderstanding.

COMMENT:
@adam2392 @ojoaomorais didn't see your comments, feel free to fix the issue, but the user reporting the issue is blocked from the org now.

Labels: Documentation, spam
Suggesting updates on the doc of `sklearn.neighbors.NeighborhoodComponentsAnalysis`

### Describe the issue linked to the documentation

Hi,

We are an academic team of software engineering researchers from a university working on automated program analysis techniques to improve API documentation quality, ultimately contributing to improving data science software development practices. we would like to keep anonymity for the purpose of double-blind paper reviewing.

We discover an inconsistency issue between documentation and code in the class [`sklearn.neighbors.NeighborhoodComponentsAnalysis`](https://scikit-learn.org/dev/modules/generated/sklearn.neighbors.NeighborhoodComponentsAnalysis.html#neighborhoodcomponentsanalysis). As mentioned in the description of the value `auto` of parameter `init`:

> -  **'auto'**
Depending on `n_components`, the most reasonable initialization will be chosen. **If `n_components <= n_classes` we use `'lda'`**, as it uses labels information. If not, but `n_components < min(n_features, n_samples)`, we use `'pca'`, as it projects data in meaningful directions (those of higher variance). Otherwise, we just use `'identity'`.

However, the most relevant source code snippet looks like this:

```python
if init == "auto":
    n_classes = len(np.unique(y))
    if n_components <= min(n_features, n_classes - 1):
        init = "lda"
    elif n_components < min(n_features, n_samples):
        init = "pca"
    else:
        init = "identity"
```

Apparently, the condition (**_n_components <= n_classes_**) shown in the document are different from the condition (**_n_components <= min(n_features, n_classes - 1)_**) actually executed in the code.


### Suggest a potential alternative/fix

Maybe you can update the documentation to avoid unnecessary misunderstanding.

COMMENT:
> @adam2392 @ojoaomorais didn't see your comments, feel free to fix the issue, but the user reporting the issue is blocked from the org now.

Submitted the PR #29441

Labels: Documentation, spam
Large errors when computing Euclidean pairwise distances

### Describe the bug

See the code below. Matrices `X` and `Y` here are the same. The function `cdist` here gives the correct answer. The "worst offender" is large diagonal entry at `incorrect_answer[2,2]`.

### Steps/Code to Reproduce

import numpy as np
from sklearn.metrics import pairwise_distances
from scipy.spatial.distance import cdist

X=np.array([[   1. ,    1. ,    1. ,    1.2,    1.4,    4.4,   -1. ,  -22. ],
       [   0. ,    1. ,    0. ,    3.2,    1.4,    4.4, -188. ,  -72. ],
       [   1. ,    1. ,    0. ,   -0.2,    1.1,    4.4,   -1. ,  -22. ],
       [   1. ,    1. ,    1. ,    1.2,    1.4,   14.4,   -1. ,  -42. ]])
Y=X.copy()
correct_answer = cdist(X,Y,metric='euclidean')
incorrect_answer=pairwise_distances(X, Y, metric='euclidean')

### Expected Results

incorrect_answer[2,2] == 0.0 or close to numerical precision

### Actual Results

incorrect_answer[2,2]
np.float64(3.371747880871523e-07)

### Versions

```shell
System:
    python: 3.11.5 (main, Sep 11 2023, 08:31:25) [Clang 14.0.6 ]
   machine: macOS-14.5-arm64-arm-64bit
Python dependencies:
      sklearn: 1.5.0
          pip: 23.3.1
   setuptools: 68.2.2
        numpy: 2.0.0
        scipy: 1.13.1
       Cython: None
       pandas: 2.2.2
   matplotlib: 3.9.0
       joblib: 1.3.2
threadpoolctl: 3.4.0
Built with OpenMP: True
threadpoolctl info:
       user_api: blas
   internal_api: openblas
    num_threads: 12
         prefix: libopenblas
       filepath: /Users/oebuild/miniconda/miniconda3/envs/carnot1/lib/python3.11/site-packages/scipy/.dylibs/libopenblas.0.dylib
        version: 0.3.27
threading_layer: pthreads
   architecture: neoversen1
       user_api: openmp
   internal_api: openmp
    num_threads: 12
         prefix: libomp
       filepath: /Users/oebuild/miniconda/miniconda3/envs/carnot1/lib/python3.11/site-packages/sklearn/.dylibs/libomp.dylib
        version: None
```

COMMENT:
I am closing this issue. I think that this trade-off between accuracy and performance is desirable (unless there is an easy and performant workaround).

Labels: Bug, Needs Triage
Wrong number of features in documentation

### Describe the issue linked to the documentation

In the scikit learn web page, [ https://scikit-learn.org/stable/modules/decomposition.html#pca, section](https://scikit-learn.org/stable/modules/decomposition.html#exact-pca-and-probabilistic-interpretation), there is a wrong number of features.
![image](https://github.com/scikit-learn/scikit-learn/assets/3278406/75307d40-d0b0-49c7-b66a-1ec65beaeef3)


### Suggest a potential alternative/fix

It should be changed the number of features from 4 to 3 in the documentation.

COMMENT:
Nop, what you are pointing out are the classes, not the feature. I confirm that iris has 4 features and I assume that here, we use a PCA to project it to 2 dimensions.

Labels: Documentation, Needs Triage
CI Unpin matplotlib<3.9 in doc build

In https://github.com/scikit-learn/scikit-learn/pull/29388 we pinned `matplotlib<3.9` see in particular https://github.com/scikit-learn/scikit-learn/pull/29388#discussion_r1668574040.

This is a DeprecationWarning in matplotlib 3.9 turned into error in the CI:

```
matplotlib._api.deprecation.MatplotlibDeprecationWarning: The 'labels' parameter of boxplot() has been renamed 'tick_labels' since Matplotlib 3.9; support for the old name will be dropped in 3.11.
```

3 examples fail (DeprecationWarning turned into error): they have been fixed in #29471.
- [x] `examples/inspection/plot_permutation_importance_multicollinear.py`
- [x] `examples/ensemble/plot_gradient_boosting_regression.py`
- [x] `examples/release_highlights/plot_release_highlights_0_22_0.py`

See [build log](https://app.circleci.com/pipelines/github/scikit-learn/scikit-learn/59445/workflows/2fe8c71c-c157-4882-b386-59028fb62f15/jobs/278637) from one of the commit in this #29388.

The easiest work-around is likely to do some version comparison something like this (not tested):
```py
from sklearn.utils.fixes import parse_version
import matplotlib

tick_labels = ...
# labels has been renamed to tick_labels in matplotlib 3.9. This code can be simplified when scikit-learn minimum supported version is 3.9
tick_labels_parameter_name = "tick_labels" if parse_version(matplotlib.__version__) >= parse_version("3.9") else "labels"
tick_labels_dict = {tick_labels_parameter_name: tick_labels}

plt.boxplot(..., **tick_labels_dict)
```

COMMENT:
It looks like the `build-doc-and-deploy` workflow is running as expected: https://app.circleci.com/pipelines/github/scikit-learn/scikit-learn?branch=main. @lesteve, should we still keep this open?

Labels: Build / CI
CI Unpin matplotlib<3.9 in doc build

In https://github.com/scikit-learn/scikit-learn/pull/29388 we pinned `matplotlib<3.9` see in particular https://github.com/scikit-learn/scikit-learn/pull/29388#discussion_r1668574040.

This is a DeprecationWarning in matplotlib 3.9 turned into error in the CI:

```
matplotlib._api.deprecation.MatplotlibDeprecationWarning: The 'labels' parameter of boxplot() has been renamed 'tick_labels' since Matplotlib 3.9; support for the old name will be dropped in 3.11.
```

3 examples fail (DeprecationWarning turned into error): they have been fixed in #29471.
- [x] `examples/inspection/plot_permutation_importance_multicollinear.py`
- [x] `examples/ensemble/plot_gradient_boosting_regression.py`
- [x] `examples/release_highlights/plot_release_highlights_0_22_0.py`

See [build log](https://app.circleci.com/pipelines/github/scikit-learn/scikit-learn/59445/workflows/2fe8c71c-c157-4882-b386-59028fb62f15/jobs/278637) from one of the commit in this #29388.

The easiest work-around is likely to do some version comparison something like this (not tested):
```py
from sklearn.utils.fixes import parse_version
import matplotlib

tick_labels = ...
# labels has been renamed to tick_labels in matplotlib 3.9. This code can be simplified when scikit-learn minimum supported version is 3.9
tick_labels_parameter_name = "tick_labels" if parse_version(matplotlib.__version__) >= parse_version("3.9") else "labels"
tick_labels_dict = {tick_labels_parameter_name: tick_labels}

plt.boxplot(..., **tick_labels_dict)
```

COMMENT:
Yes we should keep it open :wink:.

In case you want to work on this, start with a single example and make sure there is no warning when you run it locally with matplotlib 3.9, for example `examples/ensemble/plot_gradient_boosting_regression.py` because it runs quite fast.

The doc build works because we are pinning matplotlib<3.9, as you can tell for example from this [build log](https://app.circleci.com/pipelines/github/scikit-learn/scikit-learn/59871/workflows/db7ba4e4-3895-48b7-becb-47bd802a46ef/jobs/280043?invite=true#step-105-84886_72).

Once all the examples listed in this issue are fixed (i.e. they don't produce warnings with matplotlib 3.9), we will be able to use matplotlib 3.9 in the doc build.

Labels: Build / CI
CI Unpin matplotlib<3.9 in doc build

In https://github.com/scikit-learn/scikit-learn/pull/29388 we pinned `matplotlib<3.9` see in particular https://github.com/scikit-learn/scikit-learn/pull/29388#discussion_r1668574040.

This is a DeprecationWarning in matplotlib 3.9 turned into error in the CI:

```
matplotlib._api.deprecation.MatplotlibDeprecationWarning: The 'labels' parameter of boxplot() has been renamed 'tick_labels' since Matplotlib 3.9; support for the old name will be dropped in 3.11.
```

3 examples fail (DeprecationWarning turned into error): they have been fixed in #29471.
- [x] `examples/inspection/plot_permutation_importance_multicollinear.py`
- [x] `examples/ensemble/plot_gradient_boosting_regression.py`
- [x] `examples/release_highlights/plot_release_highlights_0_22_0.py`

See [build log](https://app.circleci.com/pipelines/github/scikit-learn/scikit-learn/59445/workflows/2fe8c71c-c157-4882-b386-59028fb62f15/jobs/278637) from one of the commit in this #29388.

The easiest work-around is likely to do some version comparison something like this (not tested):
```py
from sklearn.utils.fixes import parse_version
import matplotlib

tick_labels = ...
# labels has been renamed to tick_labels in matplotlib 3.9. This code can be simplified when scikit-learn minimum supported version is 3.9
tick_labels_parameter_name = "tick_labels" if parse_version(matplotlib.__version__) >= parse_version("3.9") else "labels"
tick_labels_dict = {tick_labels_parameter_name: tick_labels}

plt.boxplot(..., **tick_labels_dict)
```

COMMENT:
I opened a PR here - https://github.com/scikit-learn/scikit-learn/pull/29471. Please review when you get a chance.

Labels: Build / CI
Add weighting example to Color Quantization using K-Means page

### Describe the issue linked to the documentation

This is not an issue, but a request for addition. 
The page I am referring to can be found at https://scikit-learn.org/stable/auto_examples/cluster/plot_color_quantization.html
I recently read tried out the example on color quantization on the scikit-learn example docs. I also tried adding weights to the KMeans model fitting procedure depending on the frequencies of each color in the original palette. It seemed to give me a better final image. I would request you to consider adding this to the page.

### Suggest a potential alternative/fix

_No response_

COMMENT:
I don't think that we need to make the example more complex here. We only want to bring the concept about quantization with seeking for optimal results.

Labels: Documentation, Needs Triage
⚠️ CI failed on Wheel builder (last failure: Jul 08, 2024) ⚠️

**CI is still failing on [Wheel builder](https://github.com/scikit-learn/scikit-learn/actions/runs/9833194137)** (Jul 08, 2024)

COMMENT:
Temporary issue with scipy-dev see https://github.com/scikit-learn/scikit-learn/pull/29428#issuecomment-2213522161

Labels: Needs Triage
⚠️ CI failed on Linux_Nightly.pylatest_pip_scipy_dev (last failure: Jul 08, 2024) ⚠️

**CI is still failing on [Linux_Nightly.pylatest_pip_scipy_dev](https://dev.azure.com/scikit-learn/scikit-learn/_build/results?buildId=68381&view=logs&j=dfe99b15-50db-5d7b-b1e9-4105c42527cf)** (Jul 08, 2024)
- test_search_cv[HalvingGridSearchCV(cv=2,error_score='raise',estimator=Pipeline(steps=[('pca',PCA()),('ridge',Ridge())]),min_resources='smallest',param_grid={'ridge__alpha':[0.1,1.0]},random_state=0)-check_estimator_sparse_array]
- test_search_cv[HalvingGridSearchCV(cv=2,error_score='raise',estimator=Pipeline(steps=[('pca',PCA()),('ridge',Ridge())]),min_resources='smallest',param_grid={'ridge__alpha':[0.1,1.0]},random_state=0)-check_estimator_sparse_matrix]
- test_search_cv[HalvingRandomSearchCV(cv=2,error_score='raise',estimator=Pipeline(steps=[('pca',PCA()),('ridge',Ridge())]),param_distributions={'ridge__alpha':[0.1,1.0]},random_state=0)-check_estimator_sparse_array]
- test_search_cv[HalvingRandomSearchCV(cv=2,error_score='raise',estimator=Pipeline(steps=[('pca',PCA()),('ridge',Ridge())]),param_distributions={'ridge__alpha':[0.1,1.0]},random_state=0)-check_estimator_sparse_matrix]

COMMENT:
Temporary issue with scipy-dev see https://github.com/scikit-learn/scikit-learn/pull/29428#issuecomment-2213522161

Labels: Needs Triage
⚠️ CI failed on Linux_free_threaded.pylatest_pip_free_threaded (last failure: Jul 08, 2024) ⚠️

**CI is still failing on [Linux_free_threaded.pylatest_pip_free_threaded](https://dev.azure.com/scikit-learn/scikit-learn/_build/results?buildId=68381&view=logs&j=8bc43b48-889f-54b9-cd8b-781ee8447bf2)** (Jul 08, 2024)
- test_search_cv[HalvingGridSearchCV(cv=2,error_score='raise',estimator=Pipeline(steps=[('pca',PCA()),('ridge',Ridge())]),min_resources='smallest',param_grid={'ridge__alpha':[0.1,1.0]},random_state=0)-check_estimator_sparse_array]
- test_search_cv[HalvingGridSearchCV(cv=2,error_score='raise',estimator=Pipeline(steps=[('pca',PCA()),('ridge',Ridge())]),min_resources='smallest',param_grid={'ridge__alpha':[0.1,1.0]},random_state=0)-check_estimator_sparse_matrix]
- test_search_cv[HalvingRandomSearchCV(cv=2,error_score='raise',estimator=Pipeline(steps=[('pca',PCA()),('ridge',Ridge())]),param_distributions={'ridge__alpha':[0.1,1.0]},random_state=0)-check_estimator_sparse_array]
- test_search_cv[HalvingRandomSearchCV(cv=2,error_score='raise',estimator=Pipeline(steps=[('pca',PCA()),('ridge',Ridge())]),param_distributions={'ridge__alpha':[0.1,1.0]},random_state=0)-check_estimator_sparse_matrix]

COMMENT:
Temporary issue with scipy-dev see https://github.com/scikit-learn/scikit-learn/pull/29428#issuecomment-2213522161

Labels: Needs Triage
provide a reconstruct method for `Transformer` classes

### Describe the workflow you want to enable

provide a reconstruct method for `Transformer` classes, for convenience.

### Describe your proposed solution

    def reconstruct(self, X):
        Y = self.transform(X)
        return self.inverse_transform(Y)

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

_No response_

### Additional context

_No response_

COMMENT:
This would make sense only for the decomposition estimator and thus the API is not general enough.

I hardly think that this change would accepted through a scikit-learn enhancement proposal. I prefer to close it now.

Thanks @Freakwill for the proposal.

Labels: New Feature, Needs Triage
Documentation section 3.4.1.1 has incorrect description that would be correct if the `max_loss` metric were to be tweaked and renamed

### Describe the issue linked to the documentation

(Very similar to issue #13887 which was reported and fixed 5 years ago, so I have borrowed much of the text.)

In the documentation, section 3.4.1.1. "Common cases: predefined values", the remark:

> All scorer objects follow the convention that higher return values are better than lower return values.

is not 100% correct, as the `max_error` metric used for regression is _not_ a "greater is better" metric, as far as I can tell.

If I may, I would love to implement the PR myself, as it would be my first time contributing to a large, well-known library.

### Suggest a potential alternative/fix

1. I suggest implementing a function named `neg_max_score` which simply returns the negative of the value of max_error; this is a direct analogy to what is done in the case of ‘neg_mean_absolute_error’ and others. A better model has a lower value of mean absolute error, therefore a larger value of the mean absolute error implies a better model. The same is true for maximum error, where it is also the case that a better model is assigned a lower loss.

2. Remove references to `max_error` from section 3.4.1.1 and replace them with `neg_max_error`.

COMMENT:
Thanks for your issue, if you are interested to open a PR, this would be more than welcome :pray:!

You can probably draw some inspiration from the PR https://github.com/scikit-learn/scikit-learn/pull/14898 that fixed the similar issue about brier_score you mentioned

Labels: Documentation
Documentation section 3.4.1.1 has incorrect description that would be correct if the `max_loss` metric were to be tweaked and renamed

### Describe the issue linked to the documentation

(Very similar to issue #13887 which was reported and fixed 5 years ago, so I have borrowed much of the text.)

In the documentation, section 3.4.1.1. "Common cases: predefined values", the remark:

> All scorer objects follow the convention that higher return values are better than lower return values.

is not 100% correct, as the `max_error` metric used for regression is _not_ a "greater is better" metric, as far as I can tell.

If I may, I would love to implement the PR myself, as it would be my first time contributing to a large, well-known library.

### Suggest a potential alternative/fix

1. I suggest implementing a function named `neg_max_score` which simply returns the negative of the value of max_error; this is a direct analogy to what is done in the case of ‘neg_mean_absolute_error’ and others. A better model has a lower value of mean absolute error, therefore a larger value of the mean absolute error implies a better model. The same is true for maximum error, where it is also the case that a better model is assigned a lower loss.

2. Remove references to `max_error` from section 3.4.1.1 and replace them with `neg_max_error`.

COMMENT:
can i also work on the documentation part of 3.4.1.1? @lesteve @artificialfintelligence

Labels: Documentation
Documentation section 3.4.1.1 has incorrect description that would be correct if the `max_loss` metric were to be tweaked and renamed

### Describe the issue linked to the documentation

(Very similar to issue #13887 which was reported and fixed 5 years ago, so I have borrowed much of the text.)

In the documentation, section 3.4.1.1. "Common cases: predefined values", the remark:

> All scorer objects follow the convention that higher return values are better than lower return values.

is not 100% correct, as the `max_error` metric used for regression is _not_ a "greater is better" metric, as far as I can tell.

If I may, I would love to implement the PR myself, as it would be my first time contributing to a large, well-known library.

### Suggest a potential alternative/fix

1. I suggest implementing a function named `neg_max_score` which simply returns the negative of the value of max_error; this is a direct analogy to what is done in the case of ‘neg_mean_absolute_error’ and others. A better model has a lower value of mean absolute error, therefore a larger value of the mean absolute error implies a better model. The same is true for maximum error, where it is also the case that a better model is assigned a lower loss.

2. Remove references to `max_error` from section 3.4.1.1 and replace them with `neg_max_error`.

COMMENT:
> can i also work on the documentation part of 3.4.1.1? @lesteve @artificialfintelligence 

Absolutely! Would love to work with you on this. I'm actually not that comfortable with ReStructure Text (.rst) files so you're a lifesaver!

P.S. Apologies for the delayed response. I've been feeling a bit under the weather since I opened this issue. I should have some time tonight to start working on it.

Labels: Documentation
Documentation section 3.4.1.1 has incorrect description that would be correct if the `max_loss` metric were to be tweaked and renamed

### Describe the issue linked to the documentation

(Very similar to issue #13887 which was reported and fixed 5 years ago, so I have borrowed much of the text.)

In the documentation, section 3.4.1.1. "Common cases: predefined values", the remark:

> All scorer objects follow the convention that higher return values are better than lower return values.

is not 100% correct, as the `max_error` metric used for regression is _not_ a "greater is better" metric, as far as I can tell.

If I may, I would love to implement the PR myself, as it would be my first time contributing to a large, well-known library.

### Suggest a potential alternative/fix

1. I suggest implementing a function named `neg_max_score` which simply returns the negative of the value of max_error; this is a direct analogy to what is done in the case of ‘neg_mean_absolute_error’ and others. A better model has a lower value of mean absolute error, therefore a larger value of the mean absolute error implies a better model. The same is true for maximum error, where it is also the case that a better model is assigned a lower loss.

2. Remove references to `max_error` from section 3.4.1.1 and replace them with `neg_max_error`.

COMMENT:
/take

Labels: Documentation