Talip7/scikit-learn-issues-multilabel huggingface dataset:
RFC feature subsampling for tree based models

### Background
Currently, our (mostly tree-based) models supporting feature subsampling via `max_features` are:
- `DecisionTreeClassifier`, `DecisionTreeRegressor`
- `ExtraTreeClassifier`, `ExtraTreeRegressor`
- `BaggingClassifier`, `BaggingRegressor` 
- `ExtraTreesClassifier`, `ExtraTreesRegressor`
- `GradientBoostingClassifier`, `GradientBoostingRegressor`
- `IsolationForest`
- `RandomForestClassifier`, `RandomForestRegressor`

Soon, `HistGradientBoostingClassifier` and `HistGradientBoostingRegressor` will have feature subsampling, too, see #27139.

### Problem Statement
`max_features` can be a float between 0 and 1. In this case, it means that a **fraction** of features is randomly selected in each split. It can also be an integer, in which case it specifies the **number** of features selected in each split.
This means that `1` and `1.` have very different effects! See https://github.com/scikit-learn/scikit-learn/pull/27139#pullrequestreview-1619443083.

Also note that for most estimators the default is `max_features=None` or `1.0` meaning no subsampling. The notable exception is `RandomForestClassifier` and `ExtraTreesClassifier` with default `max_features="sqrt"` (which makes sense), but the corresponding regressors still use all features (so no rf but bagged trees). This was discussed in detail in #20111.

Additionally, for most estimator it means subsampling per tree split/node. For BaggingC/R models and IsolationForest it means per model/iteration.

### Proposal
Accompanied with a proper deprecation strategy, we could add different arguments, one for specifying fractions, one for integer numbers.

### Decision Options
#### A Do we want to change the current situation?
#### B Which names shall the new arguments have?
##### B.1
We could keep `max_features` for number of features and add only one new argument for fractions. Or we can actually add 2 new arguments and deprecate max_features completely.


##### B.2
Possible names for fractions, see also https://github.com/scikit-learn/scikit-learn/pull/27139#issuecomment-1715216957:
- `colsample_bynode`, same name as in XGBoost and LightGBM
- `feature_(sub)sample_per_split`
- `feature_fraction_per_split`

@scikit-learn/core-devs

COMMENT:
Moving this discussion to 1.5 as I don't think we will reach a consensus for 1.4.

Labels: RFC
RFC feature subsampling for tree based models

### Background
Currently, our (mostly tree-based) models supporting feature subsampling via `max_features` are:
- `DecisionTreeClassifier`, `DecisionTreeRegressor`
- `ExtraTreeClassifier`, `ExtraTreeRegressor`
- `BaggingClassifier`, `BaggingRegressor` 
- `ExtraTreesClassifier`, `ExtraTreesRegressor`
- `GradientBoostingClassifier`, `GradientBoostingRegressor`
- `IsolationForest`
- `RandomForestClassifier`, `RandomForestRegressor`

Soon, `HistGradientBoostingClassifier` and `HistGradientBoostingRegressor` will have feature subsampling, too, see #27139.

### Problem Statement
`max_features` can be a float between 0 and 1. In this case, it means that a **fraction** of features is randomly selected in each split. It can also be an integer, in which case it specifies the **number** of features selected in each split.
This means that `1` and `1.` have very different effects! See https://github.com/scikit-learn/scikit-learn/pull/27139#pullrequestreview-1619443083.

Also note that for most estimators the default is `max_features=None` or `1.0` meaning no subsampling. The notable exception is `RandomForestClassifier` and `ExtraTreesClassifier` with default `max_features="sqrt"` (which makes sense), but the corresponding regressors still use all features (so no rf but bagged trees). This was discussed in detail in #20111.

Additionally, for most estimator it means subsampling per tree split/node. For BaggingC/R models and IsolationForest it means per model/iteration.

### Proposal
Accompanied with a proper deprecation strategy, we could add different arguments, one for specifying fractions, one for integer numbers.

### Decision Options
#### A Do we want to change the current situation?
#### B Which names shall the new arguments have?
##### B.1
We could keep `max_features` for number of features and add only one new argument for fractions. Or we can actually add 2 new arguments and deprecate max_features completely.


##### B.2
Possible names for fractions, see also https://github.com/scikit-learn/scikit-learn/pull/27139#issuecomment-1715216957:
- `colsample_bynode`, same name as in XGBoost and LightGBM
- `feature_(sub)sample_per_split`
- `feature_fraction_per_split`

@scikit-learn/core-devs

COMMENT:
Should we be closing this one @lorentzenchr ?

Labels: RFC
RFC feature subsampling for tree based models

### Background
Currently, our (mostly tree-based) models supporting feature subsampling via `max_features` are:
- `DecisionTreeClassifier`, `DecisionTreeRegressor`
- `ExtraTreeClassifier`, `ExtraTreeRegressor`
- `BaggingClassifier`, `BaggingRegressor` 
- `ExtraTreesClassifier`, `ExtraTreesRegressor`
- `GradientBoostingClassifier`, `GradientBoostingRegressor`
- `IsolationForest`
- `RandomForestClassifier`, `RandomForestRegressor`

Soon, `HistGradientBoostingClassifier` and `HistGradientBoostingRegressor` will have feature subsampling, too, see #27139.

### Problem Statement
`max_features` can be a float between 0 and 1. In this case, it means that a **fraction** of features is randomly selected in each split. It can also be an integer, in which case it specifies the **number** of features selected in each split.
This means that `1` and `1.` have very different effects! See https://github.com/scikit-learn/scikit-learn/pull/27139#pullrequestreview-1619443083.

Also note that for most estimators the default is `max_features=None` or `1.0` meaning no subsampling. The notable exception is `RandomForestClassifier` and `ExtraTreesClassifier` with default `max_features="sqrt"` (which makes sense), but the corresponding regressors still use all features (so no rf but bagged trees). This was discussed in detail in #20111.

Additionally, for most estimator it means subsampling per tree split/node. For BaggingC/R models and IsolationForest it means per model/iteration.

### Proposal
Accompanied with a proper deprecation strategy, we could add different arguments, one for specifying fractions, one for integer numbers.

### Decision Options
#### A Do we want to change the current situation?
#### B Which names shall the new arguments have?
##### B.1
We could keep `max_features` for number of features and add only one new argument for fractions. Or we can actually add 2 new arguments and deprecate max_features completely.


##### B.2
Possible names for fractions, see also https://github.com/scikit-learn/scikit-learn/pull/27139#issuecomment-1715216957:
- `colsample_bynode`, same name as in XGBoost and LightGBM
- `feature_(sub)sample_per_split`
- `feature_fraction_per_split`

@scikit-learn/core-devs

COMMENT:
> **A Do we want to change the current situation?**

The answer seems to be NO.

Labels: RFC
RFC feature subsampling for tree based models

### Background
Currently, our (mostly tree-based) models supporting feature subsampling via `max_features` are:
- `DecisionTreeClassifier`, `DecisionTreeRegressor`
- `ExtraTreeClassifier`, `ExtraTreeRegressor`
- `BaggingClassifier`, `BaggingRegressor` 
- `ExtraTreesClassifier`, `ExtraTreesRegressor`
- `GradientBoostingClassifier`, `GradientBoostingRegressor`
- `IsolationForest`
- `RandomForestClassifier`, `RandomForestRegressor`

Soon, `HistGradientBoostingClassifier` and `HistGradientBoostingRegressor` will have feature subsampling, too, see #27139.

### Problem Statement
`max_features` can be a float between 0 and 1. In this case, it means that a **fraction** of features is randomly selected in each split. It can also be an integer, in which case it specifies the **number** of features selected in each split.
This means that `1` and `1.` have very different effects! See https://github.com/scikit-learn/scikit-learn/pull/27139#pullrequestreview-1619443083.

Also note that for most estimators the default is `max_features=None` or `1.0` meaning no subsampling. The notable exception is `RandomForestClassifier` and `ExtraTreesClassifier` with default `max_features="sqrt"` (which makes sense), but the corresponding regressors still use all features (so no rf but bagged trees). This was discussed in detail in #20111.

Additionally, for most estimator it means subsampling per tree split/node. For BaggingC/R models and IsolationForest it means per model/iteration.

### Proposal
Accompanied with a proper deprecation strategy, we could add different arguments, one for specifying fractions, one for integer numbers.

### Decision Options
#### A Do we want to change the current situation?
#### B Which names shall the new arguments have?
##### B.1
We could keep `max_features` for number of features and add only one new argument for fractions. Or we can actually add 2 new arguments and deprecate max_features completely.


##### B.2
Possible names for fractions, see also https://github.com/scikit-learn/scikit-learn/pull/27139#issuecomment-1715216957:
- `colsample_bynode`, same name as in XGBoost and LightGBM
- `feature_(sub)sample_per_split`
- `feature_fraction_per_split`

@scikit-learn/core-devs

COMMENT:
Given the conversation in https://github.com/scikit-learn/scikit-learn/issues/20111, I'm not clear why the default value of the RandomForestRegressor is set to use all features. 

Using all features would make it a bagged ensemble of decision trees so the function name is actively misleading. 

This seems liable to trip up beginners who may not be familiar with the details of the implementation or those coming from languages like R where the default value is something like sqrt(p) or p/3. 

I don't particularly care what the subsampling fraction is set as. As show in the discussion I posted, there are many candidates and it is ultimately problem dependent. I just think that that some subsampling should be performed by default as it is basically one of the two defining features of the RandomForest algorithm.

Labels: RFC
RFC feature subsampling for tree based models

### Background
Currently, our (mostly tree-based) models supporting feature subsampling via `max_features` are:
- `DecisionTreeClassifier`, `DecisionTreeRegressor`
- `ExtraTreeClassifier`, `ExtraTreeRegressor`
- `BaggingClassifier`, `BaggingRegressor` 
- `ExtraTreesClassifier`, `ExtraTreesRegressor`
- `GradientBoostingClassifier`, `GradientBoostingRegressor`
- `IsolationForest`
- `RandomForestClassifier`, `RandomForestRegressor`

Soon, `HistGradientBoostingClassifier` and `HistGradientBoostingRegressor` will have feature subsampling, too, see #27139.

### Problem Statement
`max_features` can be a float between 0 and 1. In this case, it means that a **fraction** of features is randomly selected in each split. It can also be an integer, in which case it specifies the **number** of features selected in each split.
This means that `1` and `1.` have very different effects! See https://github.com/scikit-learn/scikit-learn/pull/27139#pullrequestreview-1619443083.

Also note that for most estimators the default is `max_features=None` or `1.0` meaning no subsampling. The notable exception is `RandomForestClassifier` and `ExtraTreesClassifier` with default `max_features="sqrt"` (which makes sense), but the corresponding regressors still use all features (so no rf but bagged trees). This was discussed in detail in #20111.

Additionally, for most estimator it means subsampling per tree split/node. For BaggingC/R models and IsolationForest it means per model/iteration.

### Proposal
Accompanied with a proper deprecation strategy, we could add different arguments, one for specifying fractions, one for integer numbers.

### Decision Options
#### A Do we want to change the current situation?
#### B Which names shall the new arguments have?
##### B.1
We could keep `max_features` for number of features and add only one new argument for fractions. Or we can actually add 2 new arguments and deprecate max_features completely.


##### B.2
Possible names for fractions, see also https://github.com/scikit-learn/scikit-learn/pull/27139#issuecomment-1715216957:
- `colsample_bynode`, same name as in XGBoost and LightGBM
- `feature_(sub)sample_per_split`
- `feature_fraction_per_split`

@scikit-learn/core-devs

COMMENT:
> Given the conversation in https://github.com/scikit-learn/scikit-learn/issues/20111, I'm not clear why the default value of the RandomForestRegressor is set to use all features.

It is for historical reasons. I'm bothered by that situation myself.
@JohnKilbride It is best if you comment directly on #20111 and express your opinion, and then tell as many people as you can to do the same.

Labels: RFC
⚠️ CI failed on Linux.pylatest_pip_openblas_pandas ⚠️

**CI failed on [Linux.pylatest_pip_openblas_pandas](https://dev.azure.com/scikit-learn/scikit-learn/_build/results?buildId=58914&view=logs&j=78a0bf4f-79e5-5387-94ec-13e67d216d6e)** (Sep 12, 2023)
Unable to find junit file. Please see link for details.

COMMENT:
## CI is no longer failing! ✅

[Successful run](https://dev.azure.com/scikit-learn/scikit-learn/_build/results?buildId=59103&view=logs&j=78a0bf4f-79e5-5387-94ec-13e67d216d6e) on Sep 14, 2023

Labels: Needs Triage
ENH Add `pos_label` parameter to `TargetEncoder`

### Describe the workflow you want to enable

Add a `pos_label` parameter to `TargetEncoder` to enable the user to specify which label should be the positive class when the target is binary.

### Describe your proposed solution

Add a `pos_label` parameter that is passed to the `LabelBinarizer` `pos_label` parameter.

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

_No response_

### Additional context

Ref: https://github.com/scikit-learn/scikit-learn/pull/26674#discussion_r1317446826

cc @ogrisel

COMMENT:
Indeed, it makes sense. I am also thinking that the current validation of `LabelBinarizer` is not support UX friendly since it accepts only `Integral` values. I assume that we should be able to pass any type of classes.

Then we could use the `_check_pos_label_consistency` and improve it as well by passing `classes` if already computed (to avoid calling `np.unique`) and also add an extra check that `pos_label` is in `classes`.

Labels: Enhancement
ENH Add `pos_label` parameter to `TargetEncoder`

### Describe the workflow you want to enable

Add a `pos_label` parameter to `TargetEncoder` to enable the user to specify which label should be the positive class when the target is binary.

### Describe your proposed solution

Add a `pos_label` parameter that is passed to the `LabelBinarizer` `pos_label` parameter.

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

_No response_

### Additional context

Ref: https://github.com/scikit-learn/scikit-learn/pull/26674#discussion_r1317446826

cc @ogrisel

COMMENT:
/take

Labels: Enhancement
ENH Add `pos_label` parameter to `TargetEncoder`

### Describe the workflow you want to enable

Add a `pos_label` parameter to `TargetEncoder` to enable the user to specify which label should be the positive class when the target is binary.

### Describe your proposed solution

Add a `pos_label` parameter that is passed to the `LabelBinarizer` `pos_label` parameter.

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

_No response_

### Additional context

Ref: https://github.com/scikit-learn/scikit-learn/pull/26674#discussion_r1317446826

cc @ogrisel

COMMENT:
Hi @ivantishchenko, just checking in to see if you're still working on this issue. If not, I'd be happy to take it over or collaborate if you're open to it. Let me know what works best — totally understand if you're busy or it's still in progress!

Thanks!

Labels: Enhancement
ENH Add `pos_label` parameter to `TargetEncoder`

### Describe the workflow you want to enable

Add a `pos_label` parameter to `TargetEncoder` to enable the user to specify which label should be the positive class when the target is binary.

### Describe your proposed solution

Add a `pos_label` parameter that is passed to the `LabelBinarizer` `pos_label` parameter.

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

_No response_

### Additional context

Ref: https://github.com/scikit-learn/scikit-learn/pull/26674#discussion_r1317446826

cc @ogrisel

COMMENT:
@tvp590 I'm currently very busy with my main project and won't be able to finish the code for this issue in the near term.

Please feel free to take over. I'd be happy to review your code and help where I can.

Labels: Enhancement
ENH Add `pos_label` parameter to `TargetEncoder`

### Describe the workflow you want to enable

Add a `pos_label` parameter to `TargetEncoder` to enable the user to specify which label should be the positive class when the target is binary.

### Describe your proposed solution

Add a `pos_label` parameter that is passed to the `LabelBinarizer` `pos_label` parameter.

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

_No response_

### Additional context

Ref: https://github.com/scikit-learn/scikit-learn/pull/26674#discussion_r1317446826

cc @ogrisel

COMMENT:
Hi,just want to check if any one is working on this,if not i would like to work  on it

Labels: Enhancement
ENH Add `pos_label` parameter to `TargetEncoder`

### Describe the workflow you want to enable

Add a `pos_label` parameter to `TargetEncoder` to enable the user to specify which label should be the positive class when the target is binary.

### Describe your proposed solution

Add a `pos_label` parameter that is passed to the `LabelBinarizer` `pos_label` parameter.

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

_No response_

### Additional context

Ref: https://github.com/scikit-learn/scikit-learn/pull/26674#discussion_r1317446826

cc @ogrisel

COMMENT:
Hey , wanna now is any one working on this issue.
I would be happy to work if it's still open.

Labels: Enhancement
A welcome bot

Shall we add a welcome bot with a nice and pleasant message as:
![image](https://github.com/scikit-learn/scikit-learn/assets/208217/b0ba57aa-8010-420d-9712-3c832f076b9f)
https://github.com/scikit-learn/blog/pull/170

It gave me a cheesy feel-good moment, and I like these.

More seriously, I think that such a both can help
- Keep positive vibes
- Manage expectations / give early hints about how to move a PR forward

COMMENT:
The JupyterHub org uses one too

https://github.com/jupyterhub/binderhub/issues/1616#issuecomment-1374919808 - example welcome on a issue (only first time someone opens an issue)

https://github.com/jupyterhub/binderhub/pull/1164#issuecomment-709872147 - example welcome on a PR (only first time someone opens a PR)


I have no idea how much it improves things, despite the bot having existed for a few years. But I think once in a while you get reactions like Gael's.

A thing that I was worried about at first is basically doubling the number of notifications you get (each new issue also gets a welcome message) but that turns out to not really be a big deal as the two arrive at the same time.

Labels: Build / CI
A welcome bot

Shall we add a welcome bot with a nice and pleasant message as:
![image](https://github.com/scikit-learn/scikit-learn/assets/208217/b0ba57aa-8010-420d-9712-3c832f076b9f)
https://github.com/scikit-learn/blog/pull/170

It gave me a cheesy feel-good moment, and I like these.

More seriously, I think that such a both can help
- Keep positive vibes
- Manage expectations / give early hints about how to move a PR forward

COMMENT:
Was thinking of similar vibes, plus more that I had in mind. To explain, let me write my own workflow(ish):

- I usually only check notifications on things that I'm _involved_ with already. I only take new things if:
	- somebody directly tags me
	- somebody I'm supporting / mentoring needs my help
	- my existing "involved" list gets short enough for me to add more to it, given the time I have.

Now imagine a bunch of us would follow the same sort of workflow. Then:

In my mind, in an ideal world (although it's debatable if it's an ideal world or not, but I kinda like it):
- a bot tags all PRs by default as "low priority"
- we triage PRs quickly and give them priority, but that doesn't mean we review them or start reviewing them
- when we have bandwidth to pick new PRs to review, we pick some from each "high, medium, low priority" list, based on their priority and the time they take to review, in a FIFO manner
- this kinda gives an idea of how many PRs are in front of a specific PR, and a bot along the welcome message can also give information on how many PRs on in front of it. After a while we can even give estimates on when it might be picked for review.
	- The bot on a daily basis can update the comment with an updated info on the size of the queue in front of each PR.

Labels: Build / CI
A welcome bot

Shall we add a welcome bot with a nice and pleasant message as:
![image](https://github.com/scikit-learn/scikit-learn/assets/208217/b0ba57aa-8010-420d-9712-3c832f076b9f)
https://github.com/scikit-learn/blog/pull/170

It gave me a cheesy feel-good moment, and I like these.

More seriously, I think that such a both can help
- Keep positive vibes
- Manage expectations / give early hints about how to move a PR forward

COMMENT:
I have seen a welcome bot in other projects for opening issues or PRs on other projects. I have always ignored it and almost treat it like an ad on webpages (which I also ignore).

> a bot tags all PRs by default as "low priority"

I understand where you are coming from, but I would not feel great if I opened an issue and have it labeled as "low priority" by a bot. Although, I would be okay with a human did it.

> After a while we can even give estimates on when it might be picked for review.

In software development, it is hard for humans to give estimates. I do not see a bot doing better.

Labels: Build / CI
A welcome bot

Shall we add a welcome bot with a nice and pleasant message as:
![image](https://github.com/scikit-learn/scikit-learn/assets/208217/b0ba57aa-8010-420d-9712-3c832f076b9f)
https://github.com/scikit-learn/blog/pull/170

It gave me a cheesy feel-good moment, and I like these.

More seriously, I think that such a both can help
- Keep positive vibes
- Manage expectations / give early hints about how to move a PR forward

COMMENT:
As discussed in today's meeting, this could be used to warn new contributors that we frown upon AI-automated contribution and link to the relevant FAQ entry / contributor's guide to explain why.

Labels: Build / CI
A welcome bot

Shall we add a welcome bot with a nice and pleasant message as:
![image](https://github.com/scikit-learn/scikit-learn/assets/208217/b0ba57aa-8010-420d-9712-3c832f076b9f)
https://github.com/scikit-learn/blog/pull/170

It gave me a cheesy feel-good moment, and I like these.

More seriously, I think that such a both can help
- Keep positive vibes
- Manage expectations / give early hints about how to move a PR forward

COMMENT:
My thoughts on this are basically unchanged from two years ago. This means I'd support adding a welcome bot that posts a brief, welcoming message (with a cool picture). Two years later from my original comment I still don't know of any repos that have a welcome bot and decided to get rid of it, which I think means that at worst it is neutral, and probably it is a benefit.

Labels: Build / CI