Talip7/scikit-learn-issues-multilabel huggingface dataset:
RFC switch to Polars as the default dataframe lib in our examples

We now support polars as an output for our transformers and `ColumnTransformer`, but our examples use `pd.DataFrame`.

Our `datasets` module also returnes either a numpy array or a pandas DataFrame.

I'm suggesting that we enable datasets to return a polars dataframe, and to switch our examples from pandas to polars.

This has a few benefits:
- Polars on users' systems takes advantage of multi-core CPUs which is the case for pretty much all users these days. So it's quite faster in most cases.
- Pandas is dealing with issues related to Arrow, and even if they don't require Arrow as a required dependency, there will be behavior changes whether Arrow is installed or not at least on String Dtype.

Another thing is the (in)stability issues related to Pandas API where we need to deal with deprecation warnings very often. Although I'm not sure how stable the API is on the places where we touch the API on the polars side (maybe cc @MarcoGorelli )

WDYT @scikit-learn/core-devs @scikit-learn/documentation-team @scikit-learn/contributor-experience-team 

This is not really a core part of our library, but what we put in our examples and our default choices affect people since many learn from our examples.

COMMENT:
One addendum on the copy from/to numpy. In the new Polars release `0.20.7`. 

`F-order` numpy data can now be moved in and out of a Polars `DataFrame` zero-copy.

```python
import polars as pl
import numpy as np

rows = int(1e8)
cols = 5
x = np.ones((rows, cols), order='F')
# no-copy
df = pl.DataFrame(x)
# no-copy
df.to_numpy()

# But completely safe. We will never mutate
# underlying data. If you do, we will simply
# trigger a copy when you move data out into numpy
```


## Why only F-order?
Because Polars is a columnar query engine, we only store `F-contiguous` data. Otherwise we would need to skip rows on column traversal. This would duplicate all code, or add an indirection for traversal which would be much slower.

Aside from that, you now have `row` values in your cache line, making all columnar algorithms much much slower.

## C-contiguous
In c-contiguous, row values are back to back. If we need `column_value` we must skip `n` `row_x` slots, where `n` is proportional on the number of columns.
```
[column_value_1, row_x, row_x, ...,  row_x, column_value_2]
```

## F-contiguous
In f-contiguous, column slots are back to back, leading to fast cache efficient reading
```
[column_value_1, column_value_2, ..., column_value_n]
```

So zero-copy `C-contiguous` data `DataFrames` comes with a performance cost for most operations (which are columnar) and might even trigger a copy if underlying columnar algorithms expect slices with data back to back (which isn't far fetched).

Labels: RFC
RFC switch to Polars as the default dataframe lib in our examples

We now support polars as an output for our transformers and `ColumnTransformer`, but our examples use `pd.DataFrame`.

Our `datasets` module also returnes either a numpy array or a pandas DataFrame.

I'm suggesting that we enable datasets to return a polars dataframe, and to switch our examples from pandas to polars.

This has a few benefits:
- Polars on users' systems takes advantage of multi-core CPUs which is the case for pretty much all users these days. So it's quite faster in most cases.
- Pandas is dealing with issues related to Arrow, and even if they don't require Arrow as a required dependency, there will be behavior changes whether Arrow is installed or not at least on String Dtype.

Another thing is the (in)stability issues related to Pandas API where we need to deal with deprecation warnings very often. Although I'm not sure how stable the API is on the places where we touch the API on the polars side (maybe cc @MarcoGorelli )

WDYT @scikit-learn/core-devs @scikit-learn/documentation-team @scikit-learn/contributor-experience-team 

This is not really a core part of our library, but what we put in our examples and our default choices affect people since many learn from our examples.

COMMENT:
> Now even if that statement is true, a fundamental question would be: should we as sklearn care about that? Since users can independently choose what to do before and after sklearn, a jerk reaction answer might be no, but at the same time, it would feel weird if I go to teach people how to deal with data, and use polars, and then have only pandas on our examples. I really don't know.
> 
> One thing that I personally feel, is that we have a position to have an influence in ML engineers' choices, and that means we should think in the context of a whole workflow rather than sklearn only. But I'm also not completely set on whether that's the right position to have.

I found this point interesting and worth bringing back up. My impression is that is sklearn goes `X` first, then people will rally around `X`, slowly but surely. For that reason, if core sklearn devs think that a certain technology would benefit users at large (including those that have no idea of memory/memmapping/\_\_dataframe\_\_ api/dtypes, then it would make sense to push the community in that direction.

I'm unsure how closely core pandas development interacts with sklearn's needs but it seems since @ritchie46 (polars dev) is interested in participating, it seems like the option could be quite good.

As for my perspective with writing tools on top of sklearn, it's pandas all the way right now. If there's any nuances with respect to user input into sklearn between pandas and polars, I would likely just stick with pandas only. Supporting two code paths with different levels of support is not something that I can maintained trivially.

Labels: RFC
RFC switch to Polars as the default dataframe lib in our examples

We now support polars as an output for our transformers and `ColumnTransformer`, but our examples use `pd.DataFrame`.

Our `datasets` module also returnes either a numpy array or a pandas DataFrame.

I'm suggesting that we enable datasets to return a polars dataframe, and to switch our examples from pandas to polars.

This has a few benefits:
- Polars on users' systems takes advantage of multi-core CPUs which is the case for pretty much all users these days. So it's quite faster in most cases.
- Pandas is dealing with issues related to Arrow, and even if they don't require Arrow as a required dependency, there will be behavior changes whether Arrow is installed or not at least on String Dtype.

Another thing is the (in)stability issues related to Pandas API where we need to deal with deprecation warnings very often. Although I'm not sure how stable the API is on the places where we touch the API on the polars side (maybe cc @MarcoGorelli )

WDYT @scikit-learn/core-devs @scikit-learn/documentation-team @scikit-learn/contributor-experience-team 

This is not really a core part of our library, but what we put in our examples and our default choices affect people since many learn from our examples.

COMMENT:
> As for my perspective with writing tools on top of sklearn, it's pandas all the way right now. If there's any nuances with respect to user input into sklearn between pandas and polars, I would likely just stick with pandas only.

To clarify, scikit-learn will continue to support pandas. An end user that only focuses on pandas will not need to worry about other dataframe libraries. With https://github.com/scikit-learn/scikit-learn/issues/28341#issuecomment-1925458235, polars can be optional, so we would not impact any pandas-only workflows.

> Lazy frames are a key feature of polars. I don't see how that would work out in scikit-learn pipelines where the full data is passed from each step to the next.

The scikit-learn transformer can also output lazy dataframes.[^1] In scikit-learn's `Pipeline`, the data is materialized when it reaches a step that requires the data in memory. For example, our trees will most likely require the data in memory, so it will be responsible to call `df.collect()`.

[^1]: I am leaving out details on how to manage stateful transformers with lazy DataFrames, but it's doable.

Labels: RFC
RFC switch to Polars as the default dataframe lib in our examples

We now support polars as an output for our transformers and `ColumnTransformer`, but our examples use `pd.DataFrame`.

Our `datasets` module also returnes either a numpy array or a pandas DataFrame.

I'm suggesting that we enable datasets to return a polars dataframe, and to switch our examples from pandas to polars.

This has a few benefits:
- Polars on users' systems takes advantage of multi-core CPUs which is the case for pretty much all users these days. So it's quite faster in most cases.
- Pandas is dealing with issues related to Arrow, and even if they don't require Arrow as a required dependency, there will be behavior changes whether Arrow is installed or not at least on String Dtype.

Another thing is the (in)stability issues related to Pandas API where we need to deal with deprecation warnings very often. Although I'm not sure how stable the API is on the places where we touch the API on the polars side (maybe cc @MarcoGorelli )

WDYT @scikit-learn/core-devs @scikit-learn/documentation-team @scikit-learn/contributor-experience-team 

This is not really a core part of our library, but what we put in our examples and our default choices affect people since many learn from our examples.

COMMENT:
Coming back to the original question of what to use in the examples: I think a lot of data science is of the form of operations that polars is good at, but I don't think that's captured well in our examples (at least it wasn't when I last checked them in detail). Maybe that changed, but using polars just to use polars seems strange. If we actually do significant preprocessing in any example, that would be a great way to showcase polars as input I think?

Labels: RFC
Enforce `feature_names_in_` and `n_features_in_` in `check_estimator` post SLEP007 implementation

### Describe the workflow you want to enable

I would like to propose an enhancement to the `check_estimator` function, particularly in light of the implementation of SLEP007. As per SLEP007, which was integrated into scikit-learn starting from version 1.1.0, estimators are expected to support `feature_names_in_` and `n_features_in_` attributes. However, it appears that the `check_estimator` utility does not currently enforce the presence of these attributes.

### Describe your proposed solution

I propose that the `check_estimator` function be updated to include checks for the existence of the `feature_names_in_` and `n_features_in_` attributes.

### Additional context

- SLEP007:[ https://scikit-learn-enhancement-proposals.readthedocs.io/en/latest/slep007/proposal.html](https://scikit-learn-enhancement-proposals.readthedocs.io/en/latest/slep007/proposal.html)
- Version of scikit-learn where SLEP007 was implemented: 1.1.0
- Example of `check_estimator` not enforcing these attributes: https://github.com/microsoft/LightGBM/issues/6279

COMMENT:
I think we need to have a kind of tag because this feature come for free if you inherite from `BaseEstimator` but should not be require for a "minimalist" implementation.

Maybe we should have a tag to trigger the test.

Labels: New Feature, Developer API
Enforce `feature_names_in_` and `n_features_in_` in `check_estimator` post SLEP007 implementation

### Describe the workflow you want to enable

I would like to propose an enhancement to the `check_estimator` function, particularly in light of the implementation of SLEP007. As per SLEP007, which was integrated into scikit-learn starting from version 1.1.0, estimators are expected to support `feature_names_in_` and `n_features_in_` attributes. However, it appears that the `check_estimator` utility does not currently enforce the presence of these attributes.

### Describe your proposed solution

I propose that the `check_estimator` function be updated to include checks for the existence of the `feature_names_in_` and `n_features_in_` attributes.

### Additional context

- SLEP007:[ https://scikit-learn-enhancement-proposals.readthedocs.io/en/latest/slep007/proposal.html](https://scikit-learn-enhancement-proposals.readthedocs.io/en/latest/slep007/proposal.html)
- Version of scikit-learn where SLEP007 was implemented: 1.1.0
- Example of `check_estimator` not enforcing these attributes: https://github.com/microsoft/LightGBM/issues/6279

COMMENT:
I think it's a good idea to have this in the common tests, for people who don't want to inherit from BaseEstimator (although I think they should 😁 ). But the correct implementation is really not trivial.

Labels: New Feature, Developer API
Enforce `feature_names_in_` and `n_features_in_` in `check_estimator` post SLEP007 implementation

### Describe the workflow you want to enable

I would like to propose an enhancement to the `check_estimator` function, particularly in light of the implementation of SLEP007. As per SLEP007, which was integrated into scikit-learn starting from version 1.1.0, estimators are expected to support `feature_names_in_` and `n_features_in_` attributes. However, it appears that the `check_estimator` utility does not currently enforce the presence of these attributes.

### Describe your proposed solution

I propose that the `check_estimator` function be updated to include checks for the existence of the `feature_names_in_` and `n_features_in_` attributes.

### Additional context

- SLEP007:[ https://scikit-learn-enhancement-proposals.readthedocs.io/en/latest/slep007/proposal.html](https://scikit-learn-enhancement-proposals.readthedocs.io/en/latest/slep007/proposal.html)
- Version of scikit-learn where SLEP007 was implemented: 1.1.0
- Example of `check_estimator` not enforcing these attributes: https://github.com/microsoft/LightGBM/issues/6279

COMMENT:
It looks like even when inheriting from `BaseEstimator` the `feature_names_in_` attribute is added after fitting the model only if the input data has feature names.

A few different behaviors that may be worth considering w.r.t. adding the `feature_name_in_` attribute after model fitting when input data doesn't have feature names:
- not adding the attribute at all (current approach)
- adding the attribute but making it `None`
- adding artificial feature names to the data ([LightGBM does this for example](https://github.com/microsoft/LightGBM/pull/6310#issuecomment-2025915162))

Should we stick with the current approach or would one of these alternatives be favorable?

<details><summary>Example showing current approach</summary>
<p>

```python
import numpy as np
from sklearn.linear_model import LogisticRegression


np.random.seed(123)
X = np.random.normal(size=(50, 2))
y = np.random.randint(0, 2, 50)

est = LogisticRegression()
est.fit(X, y)
assert not hasattr(est, "feature_names_in_")
``` 
</p>
</details>

Labels: New Feature, Developer API
Enforce `feature_names_in_` and `n_features_in_` in `check_estimator` post SLEP007 implementation

### Describe the workflow you want to enable

I would like to propose an enhancement to the `check_estimator` function, particularly in light of the implementation of SLEP007. As per SLEP007, which was integrated into scikit-learn starting from version 1.1.0, estimators are expected to support `feature_names_in_` and `n_features_in_` attributes. However, it appears that the `check_estimator` utility does not currently enforce the presence of these attributes.

### Describe your proposed solution

I propose that the `check_estimator` function be updated to include checks for the existence of the `feature_names_in_` and `n_features_in_` attributes.

### Additional context

- SLEP007:[ https://scikit-learn-enhancement-proposals.readthedocs.io/en/latest/slep007/proposal.html](https://scikit-learn-enhancement-proposals.readthedocs.io/en/latest/slep007/proposal.html)
- Version of scikit-learn where SLEP007 was implemented: 1.1.0
- Example of `check_estimator` not enforcing these attributes: https://github.com/microsoft/LightGBM/issues/6279

COMMENT:
>  this feature come for free if you inherite from `BaseEstimator`

Are you sure that is working as expected? `lightgbm`'s scikit-learn estimators inherit from `BaseEstimator` and don't seem to be getting `feature_names_in_` defined.

Consider this example with Python 3.11, `lightgbm==4.3.0`, `pandas==2.2.0`, and `scikit-learn==1.4.1`.

```python
import lightgbm as lgb
from sklearn.datasets import load_digits

X, y = load_digits(n_class=2, return_X_y=True, as_frame=True)

# prove that this dataframe has column names
print(X.columns[:5])
# 'pixel_0_0', 'pixel_0_1', 'pixel_0_2', 'pixel_0_3', ...

# construct the estimator + fit the model
est = lgb.LGBMModel(n_estimators=5, objective="binary")
est.fit(X, y)

# try to get 'feature_names_in_'
est.feature_names_in_
# AttributeError: 'LGBMModel' object has no attribute 'feature_names_in_'.

# but it does inherit from BaseEstimator!
est.__class__.__mro__
# (<class 'lightgbm.sklearn.LGBMModel'>, <class 'sklearn.base.BaseEstimator'>, <class 'sklearn.utils._estimator_html_repr._HTMLDocumentationLinkMixin'>, <class 'sklearn.utils._metadata_requests._MetadataRequester'>, <class 'object'>)
```

What am I misunderstanding? Does every estimator need to explicitly implement this as `xgboost` did in https://github.com/dmlc/xgboost/pull/7526?

Labels: New Feature, Developer API
Enforce `feature_names_in_` and `n_features_in_` in `check_estimator` post SLEP007 implementation

### Describe the workflow you want to enable

I would like to propose an enhancement to the `check_estimator` function, particularly in light of the implementation of SLEP007. As per SLEP007, which was integrated into scikit-learn starting from version 1.1.0, estimators are expected to support `feature_names_in_` and `n_features_in_` attributes. However, it appears that the `check_estimator` utility does not currently enforce the presence of these attributes.

### Describe your proposed solution

I propose that the `check_estimator` function be updated to include checks for the existence of the `feature_names_in_` and `n_features_in_` attributes.

### Additional context

- SLEP007:[ https://scikit-learn-enhancement-proposals.readthedocs.io/en/latest/slep007/proposal.html](https://scikit-learn-enhancement-proposals.readthedocs.io/en/latest/slep007/proposal.html)
- Version of scikit-learn where SLEP007 was implemented: 1.1.0
- Example of `check_estimator` not enforcing these attributes: https://github.com/microsoft/LightGBM/issues/6279

COMMENT:
@jameslamb It looks like the `feature_names_in_` and `n_features_in_` attributes are only added by `BaseEstimator` when `_validate_data` is called, which for many sklearn estimators happens within their `fit` method.

Since `_validate_data` isn't currently called in the LGBM estimators, that would explain why they're currently not accessible (though `n_features_in_` is manually set in `LGBMModel`, so is currently accessible in the example you posted above).

Labels: New Feature, Developer API
Enforce `feature_names_in_` and `n_features_in_` in `check_estimator` post SLEP007 implementation

### Describe the workflow you want to enable

I would like to propose an enhancement to the `check_estimator` function, particularly in light of the implementation of SLEP007. As per SLEP007, which was integrated into scikit-learn starting from version 1.1.0, estimators are expected to support `feature_names_in_` and `n_features_in_` attributes. However, it appears that the `check_estimator` utility does not currently enforce the presence of these attributes.

### Describe your proposed solution

I propose that the `check_estimator` function be updated to include checks for the existence of the `feature_names_in_` and `n_features_in_` attributes.

### Additional context

- SLEP007:[ https://scikit-learn-enhancement-proposals.readthedocs.io/en/latest/slep007/proposal.html](https://scikit-learn-enhancement-proposals.readthedocs.io/en/latest/slep007/proposal.html)
- Version of scikit-learn where SLEP007 was implemented: 1.1.0
- Example of `check_estimator` not enforcing these attributes: https://github.com/microsoft/LightGBM/issues/6279

COMMENT:
Thanks @nicklamiller . Do you have link(s) to share for that claim?

Before we make changes in `lightgbm` (and maybe go propose them in other projects like XGBoost), I'd really like to hear an authoritative answer from a `scikit-learn` maintainer about what the expectation is for estimators like `lightgbm.LGBMClassifier` which inherit from `BaseEstimator` and which implement `fit()` but not `transform()`.

* should such estimators implement `feature_names_in_`?
* should they implement `_validate_data()`?
* is there some documentation besides https://scikit-learn-enhancement-proposals.readthedocs.io/en/latest/slep007/proposal.html that could be used to confirm an implementation complies with `scikit-learn`'s expectations?

I'm happy to help with the work to keep `lightgbm`'s scikit-learn estimators compliant with changes here but I'm finding it difficult to understand the precise expectations.

Labels: New Feature, Developer API
Enforce `feature_names_in_` and `n_features_in_` in `check_estimator` post SLEP007 implementation

### Describe the workflow you want to enable

I would like to propose an enhancement to the `check_estimator` function, particularly in light of the implementation of SLEP007. As per SLEP007, which was integrated into scikit-learn starting from version 1.1.0, estimators are expected to support `feature_names_in_` and `n_features_in_` attributes. However, it appears that the `check_estimator` utility does not currently enforce the presence of these attributes.

### Describe your proposed solution

I propose that the `check_estimator` function be updated to include checks for the existence of the `feature_names_in_` and `n_features_in_` attributes.

### Additional context

- SLEP007:[ https://scikit-learn-enhancement-proposals.readthedocs.io/en/latest/slep007/proposal.html](https://scikit-learn-enhancement-proposals.readthedocs.io/en/latest/slep007/proposal.html)
- Version of scikit-learn where SLEP007 was implemented: 1.1.0
- Example of `check_estimator` not enforcing these attributes: https://github.com/microsoft/LightGBM/issues/6279

COMMENT:
@jameslamb sure thing, I was unable to find documentation specifically mentioning when to use `_validate_data`, but there appear to be many instances within the code base of using this method within `fit`, some examples:

- [`IncrementalPCA`](https://github.com/scikit-learn/scikit-learn/blob/3af257e51f0a213211ac18cbde9175f7ae21579e/sklearn/decomposition/_incremental_pca.py#L227)
- [`LinearRegression`](https://github.com/scikit-learn/scikit-learn/blob/3af257e51f0a213211ac18cbde9175f7ae21579e/sklearn/linear_model/_base.py#L578)
- [`NearestCentroid`](https://github.com/scikit-learn/scikit-learn/blob/3af257e51f0a213211ac18cbde9175f7ae21579e/sklearn/neighbors/_nearest_centroid.py#L162)

with that said there are places where `fit` doesn't seem to invoke `_validate_data`:

- [`CalibratedClassifierCV`](https://github.com/scikit-learn/scikit-learn/blob/3af257e51f0a213211ac18cbde9175f7ae21579e/sklearn/calibration.py#L295)
- [`IsotonicRegression`](https://github.com/scikit-learn/scikit-learn/blob/3af257e51f0a213211ac18cbde9175f7ae21579e/sklearn/isotonic.py#L348)

also noting a few `examples` that show an example `fit` method without `_validate_data`:
- [`InductiveClusterer`](https://github.com/scikit-learn/scikit-learn/blob/3af257e51f0a213211ac18cbde9175f7ae21579e/examples/cluster/plot_inductive_clustering.py#L58)
- [`CustomEstimator`](https://github.com/scikit-learn/scikit-learn/blob/3af257e51f0a213211ac18cbde9175f7ae21579e/examples/developing_estimators/sklearn_is_fitted.py#L42)

So I agree, clarification on expectations to keep an estimator compliant would be great. @glemaitre @adrinjalali if either of you have insight here, it would be much appreciated 🙏.

Labels: New Feature, Developer API