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

### Describe the issue linked to the documentation

Hi,

We discover an inconsistency issue between documentation and code in the class [`sklearn.cluster.SpectralClustering`](https://scikit-learn.org/dev/modules/generated/sklearn.cluster.SpectralClustering.html#sklearn-cluster-spectralclustering). As mentioned in the description of parameter `gamma`:

> **gamma: float, default=10**
Kernel coefficient for rbf, poly, sigmoid, laplacian and chi2 kernels. Ignored for _**affinity='nearest_neighbors'.**_

The most relevant piece of source code looks like this:
```
if self.affinity == "nearest_neighbors":
    connectivity = kneighbors_graph(
        X, n_neighbors=self.n_neighbors, include_self=True, n_jobs=self.n_jobs
    )
    self.affinity_matrix_ = 0.5 * (connectivity + connectivity.T)
elif self.affinity == "precomputed_nearest_neighbors":
    estimator = NearestNeighbors(
        n_neighbors=self.n_neighbors, n_jobs=self.n_jobs, metric="precomputed"
    ).fit(X)
    connectivity = estimator.kneighbors_graph(X=X, mode="connectivity")
    self.affinity_matrix_ = 0.5 * (connectivity + connectivity.T)
elif self.affinity == "precomputed":
    self.affinity_matrix_ = X
else:
    params = self.kernel_params
    if params is None:
        params = {}
    if not callable(self.affinity):
        params["gamma"] = self.gamma
        params["degree"] = self.degree
        params["coef0"] = self.coef0
    self.affinity_matrix_ = pairwise_kernels(
        X, metric=self.affinity, filter_params=True, **params
    )
```
It seems that `gamma` will be ignored not only when `affinity` is nearest_neighbors but also when `affinity` is nearest_neighbors, precomputed.

Could you please check it?



### Suggest a potential alternative/fix

Maybe you can reconstruct the if-elif-else statement to cover the situation or update the doc to make it clear.

COMMENT:
This is a valid finding. I will open a PR to add this to the docstring.

Labels: Documentation, Needs Triage
Suggesting updates on the doc of `sklearn.cluster.SpectralClustering`

### Describe the issue linked to the documentation

Hi,

We discover an inconsistency issue between documentation and code in the class [`sklearn.cluster.SpectralClustering`](https://scikit-learn.org/dev/modules/generated/sklearn.cluster.SpectralClustering.html#sklearn-cluster-spectralclustering). As mentioned in the description of parameter `gamma`:

> **gamma: float, default=10**
Kernel coefficient for rbf, poly, sigmoid, laplacian and chi2 kernels. Ignored for _**affinity='nearest_neighbors'.**_

The most relevant piece of source code looks like this:
```
if self.affinity == "nearest_neighbors":
    connectivity = kneighbors_graph(
        X, n_neighbors=self.n_neighbors, include_self=True, n_jobs=self.n_jobs
    )
    self.affinity_matrix_ = 0.5 * (connectivity + connectivity.T)
elif self.affinity == "precomputed_nearest_neighbors":
    estimator = NearestNeighbors(
        n_neighbors=self.n_neighbors, n_jobs=self.n_jobs, metric="precomputed"
    ).fit(X)
    connectivity = estimator.kneighbors_graph(X=X, mode="connectivity")
    self.affinity_matrix_ = 0.5 * (connectivity + connectivity.T)
elif self.affinity == "precomputed":
    self.affinity_matrix_ = X
else:
    params = self.kernel_params
    if params is None:
        params = {}
    if not callable(self.affinity):
        params["gamma"] = self.gamma
        params["degree"] = self.degree
        params["coef0"] = self.coef0
    self.affinity_matrix_ = pairwise_kernels(
        X, metric=self.affinity, filter_params=True, **params
    )
```
It seems that `gamma` will be ignored not only when `affinity` is nearest_neighbors but also when `affinity` is nearest_neighbors, precomputed.

Could you please check it?



### Suggest a potential alternative/fix

Maybe you can reconstruct the if-elif-else statement to cover the situation or update the doc to make it clear.

COMMENT:
Hi @StefanieSenger ,

Thank you for your timely and detailed responses for each issue. Hope we can make this project better together.

Labels: Documentation, Needs Triage
Suggesting updates on the doc of `sklearn.cluster.SpectralClustering`

### Describe the issue linked to the documentation

Hi,

We discover an inconsistency issue between documentation and code in the class [`sklearn.cluster.SpectralClustering`](https://scikit-learn.org/dev/modules/generated/sklearn.cluster.SpectralClustering.html#sklearn-cluster-spectralclustering). As mentioned in the description of parameter `gamma`:

> **gamma: float, default=10**
Kernel coefficient for rbf, poly, sigmoid, laplacian and chi2 kernels. Ignored for _**affinity='nearest_neighbors'.**_

The most relevant piece of source code looks like this:
```
if self.affinity == "nearest_neighbors":
    connectivity = kneighbors_graph(
        X, n_neighbors=self.n_neighbors, include_self=True, n_jobs=self.n_jobs
    )
    self.affinity_matrix_ = 0.5 * (connectivity + connectivity.T)
elif self.affinity == "precomputed_nearest_neighbors":
    estimator = NearestNeighbors(
        n_neighbors=self.n_neighbors, n_jobs=self.n_jobs, metric="precomputed"
    ).fit(X)
    connectivity = estimator.kneighbors_graph(X=X, mode="connectivity")
    self.affinity_matrix_ = 0.5 * (connectivity + connectivity.T)
elif self.affinity == "precomputed":
    self.affinity_matrix_ = X
else:
    params = self.kernel_params
    if params is None:
        params = {}
    if not callable(self.affinity):
        params["gamma"] = self.gamma
        params["degree"] = self.degree
        params["coef0"] = self.coef0
    self.affinity_matrix_ = pairwise_kernels(
        X, metric=self.affinity, filter_params=True, **params
    )
```
It seems that `gamma` will be ignored not only when `affinity` is nearest_neighbors but also when `affinity` is nearest_neighbors, precomputed.

Could you please check it?



### Suggest a potential alternative/fix

Maybe you can reconstruct the if-elif-else statement to cover the situation or update the doc to make it clear.

COMMENT:
Hi @DocInspector,
thanks for your work. Would you mind revealing some details of your bot? Source code, goal, planned scale, especially.

Labels: Documentation, Needs Triage
Suggesting updates on the doc of `sklearn.cluster.SpectralClustering`

### Describe the issue linked to the documentation

Hi,

We discover an inconsistency issue between documentation and code in the class [`sklearn.cluster.SpectralClustering`](https://scikit-learn.org/dev/modules/generated/sklearn.cluster.SpectralClustering.html#sklearn-cluster-spectralclustering). As mentioned in the description of parameter `gamma`:

> **gamma: float, default=10**
Kernel coefficient for rbf, poly, sigmoid, laplacian and chi2 kernels. Ignored for _**affinity='nearest_neighbors'.**_

The most relevant piece of source code looks like this:
```
if self.affinity == "nearest_neighbors":
    connectivity = kneighbors_graph(
        X, n_neighbors=self.n_neighbors, include_self=True, n_jobs=self.n_jobs
    )
    self.affinity_matrix_ = 0.5 * (connectivity + connectivity.T)
elif self.affinity == "precomputed_nearest_neighbors":
    estimator = NearestNeighbors(
        n_neighbors=self.n_neighbors, n_jobs=self.n_jobs, metric="precomputed"
    ).fit(X)
    connectivity = estimator.kneighbors_graph(X=X, mode="connectivity")
    self.affinity_matrix_ = 0.5 * (connectivity + connectivity.T)
elif self.affinity == "precomputed":
    self.affinity_matrix_ = X
else:
    params = self.kernel_params
    if params is None:
        params = {}
    if not callable(self.affinity):
        params["gamma"] = self.gamma
        params["degree"] = self.degree
        params["coef0"] = self.coef0
    self.affinity_matrix_ = pairwise_kernels(
        X, metric=self.affinity, filter_params=True, **params
    )
```
It seems that `gamma` will be ignored not only when `affinity` is nearest_neighbors but also when `affinity` is nearest_neighbors, precomputed.

Could you please check it?



### Suggest a potential alternative/fix

Maybe you can reconstruct the if-elif-else statement to cover the situation or update the doc to make it clear.

COMMENT:
Based on the list of past issues: https://github.com/scikit-learn/scikit-learn/issues?q=is%3Aissue+author%3A%22DocInspector%22+ it seems that 3 out 5 reported issues were deemed false positives by reviewers. Please consider that we already have way too many valid user feedback to triage and address in the issue tracker. Therefore pushing (semi-)automated reports with such a high level of false positives is not a fruitful way to contribute to the project.

@DocInspector here is what I recommend for the future (speaking to the human operator behind the bot, assuming your contributions leverage some form of automation):

- please state explicitly in your reports and your github profile how you operate such an automation tool(s) for maximal transparency as @StefanieSenger suggested above,
- do not automatically submit an issue to our repo via your automation tools unless you first privately reviewed it to confirm it describes a valid documentation problem based on your own human judgement, acting as a potential user of the library;
- if you are not 100% convinced that this is a valid report, please refrain from opening the issue at all.

Labels: Documentation, Needs Triage
Suggesting updates on the doc of `sklearn.linear_model.OrthogonalMatchingPursuit`

### Describe the issue linked to the documentation

Hi,

We discover an inconsistency issue between documentation and code in the class [`sklearn.linear_model.OrthogonalMatchingPursuit`](https://scikit-learn.org/dev/modules/generated/sklearn.linear_model.OrthogonalMatchingPursuit.html#sklearn-linear-model-orthogonalmatchingpursuit). As mentioned in the description of parameter `n_nonzero_coefs` and `tol`.

> **n_nonzero_coefs: int, default=None**
Desired number of non-zero entries in the solution. _**If None (by default) this value is set to 10% of n_features.**_

> **tol: float, default=None**
Maximum squared norm of the residual. _**If not None, overrides n_nonzero_coefs.**_

The most relevant piece of source code looks like this:
```
if self.n_nonzero_coefs is None and self.tol is None:
    self.n_nonzero_coefs_ = max(int(0.1 * n_features), 1)
else:
    self.n_nonzero_coefs_ = self.n_nonzero_coefs
```

This piece of code does not logically cover the description in the documentation perfectly. For example, when `n_nonzero_coefs` is None and `tol` is not None, `n_nonzero_coefs` will be still overridden. However as the rule in `n_nonzero_coefs`, `n_nonzero_coefs` should be set to 10% of n_features.

Could you please check it?

### Suggest a potential alternative/fix

Maybe you can reconstruct the if-else branch to cover the situation or update the doc to make it clear.

COMMENT:
This is another false positive.

The distinction which max to use happens in `_gram_omp` with `max_features = len(Gram) if tol is not None else n_nonzero_coefs`. This issue can be closed.

Labels: Documentation
Suggesting updates on the doc of `sklearn.linear_model.OrthogonalMatchingPursuit`

### Describe the issue linked to the documentation

Hi,

We discover an inconsistency issue between documentation and code in the class [`sklearn.linear_model.OrthogonalMatchingPursuit`](https://scikit-learn.org/dev/modules/generated/sklearn.linear_model.OrthogonalMatchingPursuit.html#sklearn-linear-model-orthogonalmatchingpursuit). As mentioned in the description of parameter `n_nonzero_coefs` and `tol`.

> **n_nonzero_coefs: int, default=None**
Desired number of non-zero entries in the solution. _**If None (by default) this value is set to 10% of n_features.**_

> **tol: float, default=None**
Maximum squared norm of the residual. _**If not None, overrides n_nonzero_coefs.**_

The most relevant piece of source code looks like this:
```
if self.n_nonzero_coefs is None and self.tol is None:
    self.n_nonzero_coefs_ = max(int(0.1 * n_features), 1)
else:
    self.n_nonzero_coefs_ = self.n_nonzero_coefs
```

This piece of code does not logically cover the description in the documentation perfectly. For example, when `n_nonzero_coefs` is None and `tol` is not None, `n_nonzero_coefs` will be still overridden. However as the rule in `n_nonzero_coefs`, `n_nonzero_coefs` should be set to 10% of n_features.

Could you please check it?

### Suggest a potential alternative/fix

Maybe you can reconstruct the if-else branch to cover the situation or update the doc to make it clear.

COMMENT:
@StefanieSenger you're right about the max but we could be more explicit and say it is set to 10% of n_features when BOTH `n_nonzero_coefs` and `tol` are set to `None`. This is actually done in the `n_nonzero_coefs_` attribute:

https://github.com/scikit-learn/scikit-learn/blob/4e8253703013b38da503e2354d82fb7fa43dd4ec/sklearn/linear_model/_omp.py#L660-L663

And while looking into this, I wonder if `n_nonzero_coefs_` attr should be 'NA' or something to that effect when `tol` is not `None` as `tol` overrides `n_nonzero_coefs` when it is set.

Labels: Documentation
Suggesting updates on the doc of `sklearn.linear_model.OrthogonalMatchingPursuit`

### Describe the issue linked to the documentation

Hi,

We discover an inconsistency issue between documentation and code in the class [`sklearn.linear_model.OrthogonalMatchingPursuit`](https://scikit-learn.org/dev/modules/generated/sklearn.linear_model.OrthogonalMatchingPursuit.html#sklearn-linear-model-orthogonalmatchingpursuit). As mentioned in the description of parameter `n_nonzero_coefs` and `tol`.

> **n_nonzero_coefs: int, default=None**
Desired number of non-zero entries in the solution. _**If None (by default) this value is set to 10% of n_features.**_

> **tol: float, default=None**
Maximum squared norm of the residual. _**If not None, overrides n_nonzero_coefs.**_

The most relevant piece of source code looks like this:
```
if self.n_nonzero_coefs is None and self.tol is None:
    self.n_nonzero_coefs_ = max(int(0.1 * n_features), 1)
else:
    self.n_nonzero_coefs_ = self.n_nonzero_coefs
```

This piece of code does not logically cover the description in the documentation perfectly. For example, when `n_nonzero_coefs` is None and `tol` is not None, `n_nonzero_coefs` will be still overridden. However as the rule in `n_nonzero_coefs`, `n_nonzero_coefs` should be set to 10% of n_features.

Could you please check it?

### Suggest a potential alternative/fix

Maybe you can reconstruct the if-else branch to cover the situation or update the doc to make it clear.

COMMENT:
Yes, I agree with you, @lucyleeow, that there are still things to improve on the documentation. 

It seems that `tol` and `n_nonzero_coefs` interact differently depending on whether `n_nonzero_coefs_` or `max_features` are calculated.

> we could be more explicit and say it is set to 10% of n_features when BOTH `n_nonzero_coefs` and `tol` are set to `None`

Yes, that's for sure, and we should also hint that `n_nonzero_coefs_` is affected here.

> I wonder if `n_nonzero_coefs_` attr should be 'NA' or something to that effect when `tol` is not `None` as `tol` overrides `n_nonzero_coefs` when it is set.

I think that `tol` doesn't overwrite `n_nonzero_coefs_`, since `n_nonzero_coefs_` would be `None` (`n_nonzero_coefs`'s default value) if `tol` is not `None`. So, it's already the way you wrote. 

However, I wonder if we should specify in the documentation that `tol` overwrites `n_nonzero_coefs` _when `max_features` are calculated_, or not, because `max_features` is only a local variable used to calculate the helper matrix `L` and `gamma`, which are also only local variables and I wonder if we should mention these effects in the documentation at all, or leave it away entirely.

What do you think?

Labels: Documentation
Suggesting updates on the doc of `sklearn.linear_model.OrthogonalMatchingPursuit`

### Describe the issue linked to the documentation

Hi,

We discover an inconsistency issue between documentation and code in the class [`sklearn.linear_model.OrthogonalMatchingPursuit`](https://scikit-learn.org/dev/modules/generated/sklearn.linear_model.OrthogonalMatchingPursuit.html#sklearn-linear-model-orthogonalmatchingpursuit). As mentioned in the description of parameter `n_nonzero_coefs` and `tol`.

> **n_nonzero_coefs: int, default=None**
Desired number of non-zero entries in the solution. _**If None (by default) this value is set to 10% of n_features.**_

> **tol: float, default=None**
Maximum squared norm of the residual. _**If not None, overrides n_nonzero_coefs.**_

The most relevant piece of source code looks like this:
```
if self.n_nonzero_coefs is None and self.tol is None:
    self.n_nonzero_coefs_ = max(int(0.1 * n_features), 1)
else:
    self.n_nonzero_coefs_ = self.n_nonzero_coefs
```

This piece of code does not logically cover the description in the documentation perfectly. For example, when `n_nonzero_coefs` is None and `tol` is not None, `n_nonzero_coefs` will be still overridden. However as the rule in `n_nonzero_coefs`, `n_nonzero_coefs` should be set to 10% of n_features.

Could you please check it?

### Suggest a potential alternative/fix

Maybe you can reconstruct the if-else branch to cover the situation or update the doc to make it clear.

COMMENT:
I'm not very familiar with this module, this is just from reading the code.

> since n_nonzero_coefs_ would be None (n_nonzero_coefs's default value) if tol is not None

from:

https://github.com/scikit-learn/scikit-learn/blob/d16f0495feb7b781015262b1ef407ddc0e737136/sklearn/linear_model/_omp.py#L760-L765

If `n_nonzero_coefs` was set and `tol` is not `None`, then `n_nonzero_coefs_` attr would not be `None` in this case? For `n_nonzero_coefs_` to always be `None` if `tol` is not `None`, I think the code would need to look something like:

```python
        if self.n_nonzero_coefs is None and self.tol is None:
            # default for n_nonzero_coefs is 0.1 * n_features
            # but at least one.
            self.n_nonzero_coefs_ = max(int(0.1 * n_features), 1)
        elif self.tol is not None:
            self.n_nonzero_coefs_ = None
        else:
            self.n_nonzero_coefs_ = self.n_nonzero_coefs

```

> However, I wonder if we should specify in the documentation that tol overwrites n_nonzero_coefs when max_features are calculated,

What if we just add that `n_nonzero_coefs` is ignored if `tol` is set (in the `n_nonzero_coefs` parameter docstring)? Because that's effectively what (I think) is happening, from at a high level.

Labels: Documentation
Suggesting updates on the doc of `sklearn.linear_model.OrthogonalMatchingPursuit`

### Describe the issue linked to the documentation

Hi,

We discover an inconsistency issue between documentation and code in the class [`sklearn.linear_model.OrthogonalMatchingPursuit`](https://scikit-learn.org/dev/modules/generated/sklearn.linear_model.OrthogonalMatchingPursuit.html#sklearn-linear-model-orthogonalmatchingpursuit). As mentioned in the description of parameter `n_nonzero_coefs` and `tol`.

> **n_nonzero_coefs: int, default=None**
Desired number of non-zero entries in the solution. _**If None (by default) this value is set to 10% of n_features.**_

> **tol: float, default=None**
Maximum squared norm of the residual. _**If not None, overrides n_nonzero_coefs.**_

The most relevant piece of source code looks like this:
```
if self.n_nonzero_coefs is None and self.tol is None:
    self.n_nonzero_coefs_ = max(int(0.1 * n_features), 1)
else:
    self.n_nonzero_coefs_ = self.n_nonzero_coefs
```

This piece of code does not logically cover the description in the documentation perfectly. For example, when `n_nonzero_coefs` is None and `tol` is not None, `n_nonzero_coefs` will be still overridden. However as the rule in `n_nonzero_coefs`, `n_nonzero_coefs` should be set to 10% of n_features.

Could you please check it?

### Suggest a potential alternative/fix

Maybe you can reconstruct the if-else branch to cover the situation or update the doc to make it clear.

COMMENT:
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0 Transitional//EN">

<html>
<head>
So, now I've drawn myself a truth table, considering the default values:
</head>

<body>

  | coefs None | coefs value |  
-- | -- | -- | --
tol None | 10,00 % | value of coef |  
tol value | None | value of coef |  


</body>

</html>

If `n_nonzero_coefs` is set, `n_nonzero_coefs_` will be that value, otherwise it will be either 10% of the `n_features` (if tol is `None`) or `None`. And I've traced `n_nonzero_coefs_` alias `n_nonzero_coefs` down until `max_features = len(Gram) if tol is not None else n_nonzero_coefs`. 

And yes: `n_nonzero_coefs` is totally ignored if `tol` is set. I'd agree with this and I like your suggestion.

Labels: Documentation
Suggesting updates on the doc of `sklearn.linear_model.OrthogonalMatchingPursuit`

### Describe the issue linked to the documentation

Hi,

We discover an inconsistency issue between documentation and code in the class [`sklearn.linear_model.OrthogonalMatchingPursuit`](https://scikit-learn.org/dev/modules/generated/sklearn.linear_model.OrthogonalMatchingPursuit.html#sklearn-linear-model-orthogonalmatchingpursuit). As mentioned in the description of parameter `n_nonzero_coefs` and `tol`.

> **n_nonzero_coefs: int, default=None**
Desired number of non-zero entries in the solution. _**If None (by default) this value is set to 10% of n_features.**_

> **tol: float, default=None**
Maximum squared norm of the residual. _**If not None, overrides n_nonzero_coefs.**_

The most relevant piece of source code looks like this:
```
if self.n_nonzero_coefs is None and self.tol is None:
    self.n_nonzero_coefs_ = max(int(0.1 * n_features), 1)
else:
    self.n_nonzero_coefs_ = self.n_nonzero_coefs
```

This piece of code does not logically cover the description in the documentation perfectly. For example, when `n_nonzero_coefs` is None and `tol` is not None, `n_nonzero_coefs` will be still overridden. However as the rule in `n_nonzero_coefs`, `n_nonzero_coefs` should be set to 10% of n_features.

Could you please check it?

### Suggest a potential alternative/fix

Maybe you can reconstruct the if-else branch to cover the situation or update the doc to make it clear.

COMMENT:
Great! I think we could:

* Amend the code to:

```python
        if self.n_nonzero_coefs is None and self.tol is None:
            # default for n_nonzero_coefs is 0.1 * n_features
            # but at least one.
            self.n_nonzero_coefs_ = max(int(0.1 * n_features), 1)
        elif self.tol is not None:
            self.n_nonzero_coefs_ = None
        else:
            self.n_nonzero_coefs_ = self.n_nonzero_coefs
```

so `n_nonzero_coefs_` is always `None` when it has been ignored

* amend `n_nonzero_coefs_` docstring to include `None` and explain that when `None` it means the value has been ignored, here:

https://github.com/scikit-learn/scikit-learn/blob/b27894e466e6433ddbd2bca821e263df25acc2c9/sklearn/linear_model/_omp.py#L660-L663

Labels: Documentation