Talip7/scikit-learn-issues-multilabel huggingface dataset:
Erroneous use of chi2 in the documentation

### Describe the issue linked to the documentation

The [documentation page](https://scikit-learn.org/stable/modules/feature_selection.html#univariate-feature-selection) for univariate feature selection suggests that the [`sklearn.feature_selection.chi2`](https://github.com/scikit-learn/scikit-learn/blob/36958fb24/sklearn/feature_selection/_univariate_selection.py#L168) function could be used to rank (and possibly select) features. The problem is that the example given in the documentation is the [Iris data set](https://scikit-learn.org/stable/auto_examples/datasets/plot_iris_dataset.html) where the features are continuous. The `chi2` function is however supposed to work with contingency matrices of discrete features (i.e., typically counts, frequencies, or booleans). 

The use of `chi2` is therefore likely to be misleading in this context. See [this discussion](https://stats.stackexchange.com/questions/591202/can-the-χ²-test-be-used-without-a-contingency-table/) on StackExchange for further details.

### Suggest a potential alternative/fix

Either find a different use of `chi2` that is more aligned with the fact that it works with contingency matrices or explain more clearly how it could be used with continuous features.

COMMENT:
I recall a similar issue some time ago: https://github.com/scikit-learn/scikit-learn/issues/21455#issuecomment-977968841

Labels: Documentation
Migrating from LGTM, or removng it

### Discussed in https://github.com/scikit-learn/scikit-learn/discussions/24578

<div type='discussions-op-text'>

<sup>Originally posted by **haiatn** October  4, 2022</sup>
Hi,
I read LGTM will shut down and we have all PRs scanned by it,
It could be replaced with Github Code Scanning:
https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/managing-code-scanning-alerts-for-your-repository

What do you think? Are there any limitations?</div>

COMMENT:
Seems like they've moved everything to github: https://github.blog/2022-08-15-the-next-step-for-lgtm-com-github-code-scanning/

But I'm not sure how much we've actually used the results of LGTM, I don't remember a single instance myself. Should we just remove them?

Labels: Needs Triage
Migrating from LGTM, or removng it

### Discussed in https://github.com/scikit-learn/scikit-learn/discussions/24578

<div type='discussions-op-text'>

<sup>Originally posted by **haiatn** October  4, 2022</sup>
Hi,
I read LGTM will shut down and we have all PRs scanned by it,
It could be replaced with Github Code Scanning:
https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/managing-code-scanning-alerts-for-your-repository

What do you think? Are there any limitations?</div>

COMMENT:
According to this issue https://github.com/scikit-learn/scikit-learn/issues/12167 it was ignored for several months when we didn't know that it stopped working.
But later when it started again it should've alerted on errors before merging and also people did open PRs to fix existing errors.

Labels: Needs Triage
Migrating from LGTM, or removng it

### Discussed in https://github.com/scikit-learn/scikit-learn/discussions/24578

<div type='discussions-op-text'>

<sup>Originally posted by **haiatn** October  4, 2022</sup>
Hi,
I read LGTM will shut down and we have all PRs scanned by it,
It could be replaced with Github Code Scanning:
https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/managing-code-scanning-alerts-for-your-repository

What do you think? Are there any limitations?</div>

COMMENT:
cc @thomasjpfan @jjerphan 

Thomas thinks they're useful, I'm curious as when such an incident has happened :P

Labels: Needs Triage
Migrating from LGTM, or removng it

### Discussed in https://github.com/scikit-learn/scikit-learn/discussions/24578

<div type='discussions-op-text'>

<sup>Originally posted by **haiatn** October  4, 2022</sup>
Hi,
I read LGTM will shut down and we have all PRs scanned by it,
It could be replaced with Github Code Scanning:
https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/managing-code-scanning-alerts-for-your-repository

What do you think? Are there any limitations?</div>

COMMENT:
From the pass 3-4 years, I think LGTM has not helped too much. A lot of the warnings or errors are patterns we are okay with. For example, this recommendation is something we are okay with:

![Screen Shot 2022-10-05 at 9 37 42 AM](https://user-images.githubusercontent.com/5402633/194074180-0c427cdf-c388-4444-af06-4b6eeba6b51f.jpg)

From looking through all the current [errors and warnings on LGTM](https://lgtm.com/projects/g/scikit-learn/scikit-learn/alerts/?mode=list), I think a lot of them are false positives. The hard part for contributors is to filter out all the noise.

From time to time, LGTM causes the PR's CI to fail that is unrelated to the PR.

For now, I am in favor of removing LGTM without a replacement. We can evaluate GitHub code scanning as a follow up.

Labels: Needs Triage
Migrating from LGTM, or removng it

### Discussed in https://github.com/scikit-learn/scikit-learn/discussions/24578

<div type='discussions-op-text'>

<sup>Originally posted by **haiatn** October  4, 2022</sup>
Hi,
I read LGTM will shut down and we have all PRs scanned by it,
It could be replaced with Github Code Scanning:
https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/managing-code-scanning-alerts-for-your-repository

What do you think? Are there any limitations?</div>

COMMENT:
If so, I suggest removing it and if a need comes up we can open an new issue with alternatives

Labels: Needs Triage
Migrating from LGTM, or removng it

### Discussed in https://github.com/scikit-learn/scikit-learn/discussions/24578

<div type='discussions-op-text'>

<sup>Originally posted by **haiatn** October  4, 2022</sup>
Hi,
I read LGTM will shut down and we have all PRs scanned by it,
It could be replaced with Github Code Scanning:
https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/managing-code-scanning-alerts-for-your-repository

What do you think? Are there any limitations?</div>

COMMENT:
Thank you all

Labels: Needs Triage
[RFC] Modularize 'Criterion' class inside tree submodule to allow 3rd party extensions for data-based criterion

### Summary

There exists other criterions, where additional arguments would need to be passed in. The current `Criterion` class is not actually a generic Criterion class since it explicitly assumes that the Criterion is supervised-learning based and does NOT require the data `X` itself. The `init(...)` function explicitly passes in `y`.

I am trying to implement and subclass `Criterion` to implement Unsupervised Random Forests (https://arxiv.org/abs/1907.02844), which use the data within the Criterion. I would like to subclass the `Criterion` class for my own `UnsupervisedCriterion`. However, the `init` function assumes access to a `y` label, but everything else about it is fine and correct.

Instead I would like to define my own `init` function, but keep the rest of the Criterion API. This way, I can define an unsupervised criterion with the `init` function:

```python
    cdef int init(self, const DTYPE_T[:, ::1] X, DOUBLE_t* sample_weight,
                  double weighted_n_samples, SIZE_t* samples, SIZE_t start,
                  SIZE_t end) nogil except -1
```

and I could then define the rest of the functions, while making sure I am in strict compliance with sklearn's Tree submodule.

### Proposed Solution

Add a class called `BaseCriterion` and migrate all functions/properties of the current `Criterion` class, EXCEPT for `init` function to `BaseCriterion`.

This is completely backwards-compatable and ensures 3rd party API can leverage the existing API contract for Criterions using the `BaseCriterion` class.

### Alternatives

Alternatively, I can do what `scikit-survival` does and pass in this additional data to `__cinit__`, but this is weird because you basically need to pass in data to a Criterion class before it's used with Tree/Splitter. Rn `init` is called within the Tree/Splitter, which ensures the `y` that `Tree/Splitter` have access to is the same as `Criterion`. If you are forced to pass things through `__cinit__` it is quite possible to introduce runtime bugs because your data changes after you have already instantiated a criterion class.

### Alternatives2

Another possibility that I'm not 100% sure on is that `Criterion.init(...)` is passed `y` and just resets it to the passed in `y` data. However, it never modifies `y`. Instead, it should only init the 'y' variable once.... Perhaps something like `Criterion.init_data(y)`, which separates initialization of the data compared to initialization of different variables.

COMMENT:
cc @Vincent-Maladiere and @ogrisel who potentially might be interested in this regarding extension `Criterion` in 3rd party scikit-learn-like packages.

https://github.com/scikit-learn/scikit-learn/issues/24000 might also be relevant but has a broader scope.

Labels: RFC, module:ensemble, module:tree, cython, Refactor
[RFC] Modularize 'Criterion' class inside tree submodule to allow 3rd party extensions for data-based criterion

### Summary

There exists other criterions, where additional arguments would need to be passed in. The current `Criterion` class is not actually a generic Criterion class since it explicitly assumes that the Criterion is supervised-learning based and does NOT require the data `X` itself. The `init(...)` function explicitly passes in `y`.

I am trying to implement and subclass `Criterion` to implement Unsupervised Random Forests (https://arxiv.org/abs/1907.02844), which use the data within the Criterion. I would like to subclass the `Criterion` class for my own `UnsupervisedCriterion`. However, the `init` function assumes access to a `y` label, but everything else about it is fine and correct.

Instead I would like to define my own `init` function, but keep the rest of the Criterion API. This way, I can define an unsupervised criterion with the `init` function:

```python
    cdef int init(self, const DTYPE_T[:, ::1] X, DOUBLE_t* sample_weight,
                  double weighted_n_samples, SIZE_t* samples, SIZE_t start,
                  SIZE_t end) nogil except -1
```

and I could then define the rest of the functions, while making sure I am in strict compliance with sklearn's Tree submodule.

### Proposed Solution

Add a class called `BaseCriterion` and migrate all functions/properties of the current `Criterion` class, EXCEPT for `init` function to `BaseCriterion`.

This is completely backwards-compatable and ensures 3rd party API can leverage the existing API contract for Criterions using the `BaseCriterion` class.

### Alternatives

Alternatively, I can do what `scikit-survival` does and pass in this additional data to `__cinit__`, but this is weird because you basically need to pass in data to a Criterion class before it's used with Tree/Splitter. Rn `init` is called within the Tree/Splitter, which ensures the `y` that `Tree/Splitter` have access to is the same as `Criterion`. If you are forced to pass things through `__cinit__` it is quite possible to introduce runtime bugs because your data changes after you have already instantiated a criterion class.

### Alternatives2

Another possibility that I'm not 100% sure on is that `Criterion.init(...)` is passed `y` and just resets it to the passed in `y` data. However, it never modifies `y`. Instead, it should only init the 'y' variable once.... Perhaps something like `Criterion.init_data(y)`, which separates initialization of the data compared to initialization of different variables.

COMMENT:
FYI, I have also written up a more lengthy proposal on improving tree submodule. See https://github.com/scikit-learn/scikit-learn/issues/24000#issuecomment-1270611258

Labels: RFC, module:ensemble, module:tree, cython, Refactor
[RFC] Modularize 'Criterion' class inside tree submodule to allow 3rd party extensions for data-based criterion

### Summary

There exists other criterions, where additional arguments would need to be passed in. The current `Criterion` class is not actually a generic Criterion class since it explicitly assumes that the Criterion is supervised-learning based and does NOT require the data `X` itself. The `init(...)` function explicitly passes in `y`.

I am trying to implement and subclass `Criterion` to implement Unsupervised Random Forests (https://arxiv.org/abs/1907.02844), which use the data within the Criterion. I would like to subclass the `Criterion` class for my own `UnsupervisedCriterion`. However, the `init` function assumes access to a `y` label, but everything else about it is fine and correct.

Instead I would like to define my own `init` function, but keep the rest of the Criterion API. This way, I can define an unsupervised criterion with the `init` function:

```python
    cdef int init(self, const DTYPE_T[:, ::1] X, DOUBLE_t* sample_weight,
                  double weighted_n_samples, SIZE_t* samples, SIZE_t start,
                  SIZE_t end) nogil except -1
```

and I could then define the rest of the functions, while making sure I am in strict compliance with sklearn's Tree submodule.

### Proposed Solution

Add a class called `BaseCriterion` and migrate all functions/properties of the current `Criterion` class, EXCEPT for `init` function to `BaseCriterion`.

This is completely backwards-compatable and ensures 3rd party API can leverage the existing API contract for Criterions using the `BaseCriterion` class.

### Alternatives

Alternatively, I can do what `scikit-survival` does and pass in this additional data to `__cinit__`, but this is weird because you basically need to pass in data to a Criterion class before it's used with Tree/Splitter. Rn `init` is called within the Tree/Splitter, which ensures the `y` that `Tree/Splitter` have access to is the same as `Criterion`. If you are forced to pass things through `__cinit__` it is quite possible to introduce runtime bugs because your data changes after you have already instantiated a criterion class.

### Alternatives2

Another possibility that I'm not 100% sure on is that `Criterion.init(...)` is passed `y` and just resets it to the passed in `y` data. However, it never modifies `y`. Instead, it should only init the 'y' variable once.... Perhaps something like `Criterion.init_data(y)`, which separates initialization of the data compared to initialization of different variables.

COMMENT:
Thanks for proposing a solutions with alternatives. The proposed solution makes sense to me.

Even if it modifies the current Cython API used by third party libraries, I think performing this change will help them in the long-term.

Labels: RFC, module:ensemble, module:tree, cython, Refactor
[RFC] Modularize 'Criterion' class inside tree submodule to allow 3rd party extensions for data-based criterion

### Summary

There exists other criterions, where additional arguments would need to be passed in. The current `Criterion` class is not actually a generic Criterion class since it explicitly assumes that the Criterion is supervised-learning based and does NOT require the data `X` itself. The `init(...)` function explicitly passes in `y`.

I am trying to implement and subclass `Criterion` to implement Unsupervised Random Forests (https://arxiv.org/abs/1907.02844), which use the data within the Criterion. I would like to subclass the `Criterion` class for my own `UnsupervisedCriterion`. However, the `init` function assumes access to a `y` label, but everything else about it is fine and correct.

Instead I would like to define my own `init` function, but keep the rest of the Criterion API. This way, I can define an unsupervised criterion with the `init` function:

```python
    cdef int init(self, const DTYPE_T[:, ::1] X, DOUBLE_t* sample_weight,
                  double weighted_n_samples, SIZE_t* samples, SIZE_t start,
                  SIZE_t end) nogil except -1
```

and I could then define the rest of the functions, while making sure I am in strict compliance with sklearn's Tree submodule.

### Proposed Solution

Add a class called `BaseCriterion` and migrate all functions/properties of the current `Criterion` class, EXCEPT for `init` function to `BaseCriterion`.

This is completely backwards-compatable and ensures 3rd party API can leverage the existing API contract for Criterions using the `BaseCriterion` class.

### Alternatives

Alternatively, I can do what `scikit-survival` does and pass in this additional data to `__cinit__`, but this is weird because you basically need to pass in data to a Criterion class before it's used with Tree/Splitter. Rn `init` is called within the Tree/Splitter, which ensures the `y` that `Tree/Splitter` have access to is the same as `Criterion`. If you are forced to pass things through `__cinit__` it is quite possible to introduce runtime bugs because your data changes after you have already instantiated a criterion class.

### Alternatives2

Another possibility that I'm not 100% sure on is that `Criterion.init(...)` is passed `y` and just resets it to the passed in `y` data. However, it never modifies `y`. Instead, it should only init the 'y' variable once.... Perhaps something like `Criterion.init_data(y)`, which separates initialization of the data compared to initialization of different variables.

COMMENT:
@scikit-learn/core-devs This needs a decision.

@adam2392 What a about passing a fake y and just never use it?

In total, I‘m a bit concerned about building an unofficial API for a 3rd party library (where are the unsupervised trees supposed to live?) which thereby becomes official making development in scikit-learn harder.

Labels: RFC, module:ensemble, module:tree, cython, Refactor
[RFC] Modularize 'Criterion' class inside tree submodule to allow 3rd party extensions for data-based criterion

### Summary

There exists other criterions, where additional arguments would need to be passed in. The current `Criterion` class is not actually a generic Criterion class since it explicitly assumes that the Criterion is supervised-learning based and does NOT require the data `X` itself. The `init(...)` function explicitly passes in `y`.

I am trying to implement and subclass `Criterion` to implement Unsupervised Random Forests (https://arxiv.org/abs/1907.02844), which use the data within the Criterion. I would like to subclass the `Criterion` class for my own `UnsupervisedCriterion`. However, the `init` function assumes access to a `y` label, but everything else about it is fine and correct.

Instead I would like to define my own `init` function, but keep the rest of the Criterion API. This way, I can define an unsupervised criterion with the `init` function:

```python
    cdef int init(self, const DTYPE_T[:, ::1] X, DOUBLE_t* sample_weight,
                  double weighted_n_samples, SIZE_t* samples, SIZE_t start,
                  SIZE_t end) nogil except -1
```

and I could then define the rest of the functions, while making sure I am in strict compliance with sklearn's Tree submodule.

### Proposed Solution

Add a class called `BaseCriterion` and migrate all functions/properties of the current `Criterion` class, EXCEPT for `init` function to `BaseCriterion`.

This is completely backwards-compatable and ensures 3rd party API can leverage the existing API contract for Criterions using the `BaseCriterion` class.

### Alternatives

Alternatively, I can do what `scikit-survival` does and pass in this additional data to `__cinit__`, but this is weird because you basically need to pass in data to a Criterion class before it's used with Tree/Splitter. Rn `init` is called within the Tree/Splitter, which ensures the `y` that `Tree/Splitter` have access to is the same as `Criterion`. If you are forced to pass things through `__cinit__` it is quite possible to introduce runtime bugs because your data changes after you have already instantiated a criterion class.

### Alternatives2

Another possibility that I'm not 100% sure on is that `Criterion.init(...)` is passed `y` and just resets it to the passed in `y` data. However, it never modifies `y`. Instead, it should only init the 'y' variable once.... Perhaps something like `Criterion.init_data(y)`, which separates initialization of the data compared to initialization of different variables.

COMMENT:
Can't find it now, but a couple of years ago I was working on criterion, and finishing that would mean changing the API on this base criterion class.

I'm happy to have a much better developer API here, but we need to be very clear about the fact that it won't follow the usual stability / deprecation cycle as other parts of the library.

Labels: RFC, module:ensemble, module:tree, cython, Refactor
Remove parsar parameter from fetch_openml()

### Describe the workflow you want to enable

data = fetch_openml("mnist_784", as_frame=True, parser="pandas") It cause error while running the 
sklearn/benchmarks/bench_mnist.py file 

### Describe your proposed solution

data = fetch_openml("mnist_784", as_frame=True) It is running smoothly.

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

_No response_

### Additional context

_No response_

COMMENT:
check my pull request #24576

Labels: New Feature, Needs Triage
Remove parsar parameter from fetch_openml()

### Describe the workflow you want to enable

data = fetch_openml("mnist_784", as_frame=True, parser="pandas") It cause error while running the 
sklearn/benchmarks/bench_mnist.py file 

### Describe your proposed solution

data = fetch_openml("mnist_784", as_frame=True) It is running smoothly.

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

_No response_

### Additional context

_No response_

COMMENT:
I'm not sure what error you're referring to. Closing.

Labels: New Feature, Needs Triage