Talip7/scikit-learn-issues-multilabel huggingface dataset:
RFC New parameters for penalties in LogisticRegression

Based on the comment https://github.com/scikit-learn/scikit-learn/pull/28706#discussion_r1541184840:

Currently, `LogisticRegression` uses `C` as inverse penalization strength, `penalty` to select the type of penalty and `l1_ratio` to control the ration between l1 and l2 penalties.
I propose the following:
1. Add `alpha` (as in `Ridge`, `ElasticNet`, `PoissonRegressor` ...) instead of `C`.
  Fail if both are given at the same time.
2. Deprecate `C`.
3. Deprecate `penalty` which is redundant. `alpha` and `l1_ratio` are enough.

COMMENT:
If we are to do such a big renaming, wouldn't it make sense to use a more explicit name such as `l2_regularization` or `l2_reg`?

Also note: the alpha in lasso / GLMs and the one in ridge do not have the same meaning. One is scaled by the sum of sample weights (or n_samples) while the other is not. If we are to use a more explicit name such as `l2_reg(ularization)` we could use that opportunity to also make that more uniform across all linear models.

We had a discussion in the past about whether this is intentional or not because it could lead to make efficient parametrization for parameter tuning but I think we could decouple the choice of the default parameter value (which can stay estimator specific) from the choice of the parametrization which we could choose to make uniform across all linear models to simplify the message in the narrative doc, the reference doc and the various examples.

We could also decide to move progressively and first rename C by `l2_reg` and defer the decision to do a renaming/uniformization for `alpha` to a later time (if ever).

Labels: API, RFC, module:linear_model
RFC New parameters for penalties in LogisticRegression

Based on the comment https://github.com/scikit-learn/scikit-learn/pull/28706#discussion_r1541184840:

Currently, `LogisticRegression` uses `C` as inverse penalization strength, `penalty` to select the type of penalty and `l1_ratio` to control the ration between l1 and l2 penalties.
I propose the following:
1. Add `alpha` (as in `Ridge`, `ElasticNet`, `PoissonRegressor` ...) instead of `C`.
  Fail if both are given at the same time.
2. Deprecate `C`.
3. Deprecate `penalty` which is redundant. `alpha` and `l1_ratio` are enough.

COMMENT:
But ``C`` controls both l1 and l2 regularization (as alpha does for ElasticNet), their relative strength being controlled by ``l1_ratio``. I'd find it confusing to rename it ``l2_reg`` and then enable l1 regularization by setting ``l2_reg=<some value>`` and ``l1_ratio=1``.

Labels: API, RFC, module:linear_model
RFC New parameters for penalties in LogisticRegression

Based on the comment https://github.com/scikit-learn/scikit-learn/pull/28706#discussion_r1541184840:

Currently, `LogisticRegression` uses `C` as inverse penalization strength, `penalty` to select the type of penalty and `l1_ratio` to control the ration between l1 and l2 penalties.
I propose the following:
1. Add `alpha` (as in `Ridge`, `ElasticNet`, `PoissonRegressor` ...) instead of `C`.
  Fail if both are given at the same time.
2. Deprecate `C`.
3. Deprecate `penalty` which is redundant. `alpha` and `l1_ratio` are enough.

COMMENT:
Indeed `l2_reg(ularization)` would be a bad name: `reg(ularization)_strength` or just `regularization` might be a better name.

Labels: API, RFC, module:linear_model
RFC New parameters for penalties in LogisticRegression

Based on the comment https://github.com/scikit-learn/scikit-learn/pull/28706#discussion_r1541184840:

Currently, `LogisticRegression` uses `C` as inverse penalization strength, `penalty` to select the type of penalty and `l1_ratio` to control the ration between l1 and l2 penalties.
I propose the following:
1. Add `alpha` (as in `Ridge`, `ElasticNet`, `PoissonRegressor` ...) instead of `C`.
  Fail if both are given at the same time.
2. Deprecate `C`.
3. Deprecate `penalty` which is redundant. `alpha` and `l1_ratio` are enough.

COMMENT:
> Indeed `l2_reg(ularization)` would be a bad name: `reg(ularization)_strength` or just `regularization` might be a better name.

I would prefer a consistent name among all linear models and `alpha` seems to be used most often as penalty strength.

As of version 1.4:

| estimator | type | penalty strength parameter | additional penalty parameter |
| --------- | ---- | -------------------------- | ---------------------------- |
| `LogisticRegression` | classifier | `C` | `penalty`, `l1_ratio` |
| ~`PassiveAggressiveClassifier`~ deprecated | ~classifier~ | ~`C`~ different meaning |
| `RidgeClassifier` | classifier | `alpha` |
| `SGDClassifier` | classifier | `alpha` | `penalty`, `l1_ratio` |
| `Ridge` | regressor | `alpha` |
| `SGDRegressor` | regressor | `alpha` | `penalty`, `l1_ratio`|
| `ElasticNet` | regressor | `alpha` | `l1_ratio` |
| `Lasso` | regressor | `alpha` |
| `LassoLars` | regressor | `alpha` |
| `OrthogonalMatchingPursuit` | regressor | `n_nonzero_coefs` |
| `HuberRegressor` | regressor | `alpha` |
| `QuantileRegressor` | regressor | `alpha` |
| `PoissonRegressor` | regressor | `alpha` |
| `TweedieRegressor` | regressor | `alpha` |
| `GammaRegressor` | regressor | `alpha` |
| ~`PassiveAggressiveRegressor`~ deprecated | ~regressor~ | ~`C`~ different meaning |

Labels: API, RFC, module:linear_model
Misleading OpenMP warning on MacOS when building with Meson

### Describe the bug

Compiling on MacOS with openmp works the old way, see https://scikit-learn.org/dev/developers/advanced_installation.html#macos:
- `brew install libomp`
- ```
  export CC=/usr/bin/clang
  export CXX=/usr/bin/clang++
  export CPPFLAGS="$CPPFLAGS -Xpreprocessor -fopenmp"
  export CFLAGS="$CFLAGS -I/usr/local/opt/libomp/include"
  export CXXFLAGS="$CXXFLAGS -I/usr/local/opt/libomp/include"
  export LDFLAGS="$LDFLAGS -Wl,-rpath,/usr/local/opt/libomp/lib -L/usr/local/opt/libomp/lib -lomp"
  ```
- Build `make in`

With the new meson build system, a warning is raised and scikit-learn is built without openmp:
- same as above, just the last commant is `make dev-meson` instead of `make in`.

### Steps/Code to Reproduce

```
% make dev-meson
```

### Expected Results

No warning and it compiles with openmp enabled.

### Actual Results

```
Run-time dependency OpenMP for c found: NO (tried system)
  ../../sklearn/meson.build:63: WARNING:
                  ***********
                  * WARNING *
                  ***********

  It seems that scikit-learn cannot be built with OpenMP.

  - Make sure you have followed the installation instructions:

      https://scikit-learn.org/dev/developers/advanced_installation.html

  - If your compiler supports OpenMP but you still see this
    message, please submit a bug report at:

      https://github.com/scikit-learn/scikit-learn/issues

  - The build will continue with OpenMP-based parallelism
    disabled. Note however that some estimators will run in
    sequential mode instead of leveraging thread-based
    parallelism.

                      ***
```

### Versions

```shell
System:
    python: 3.11.7 (main, Dec  4 2023, 18:10:11) [Clang 15.0.0 (clang-1500.1.0.2.5)]
executable: /Users/lorentzen/github/python3_sklearn/bin/python
   machine: macOS-14.4-x86_64-i386-64bit

Python dependencies:
      sklearn: 1.5.dev0
          pip: 24.0
   setuptools: 68.0.0
        numpy: 1.26.0
        scipy: 1.11.3
       Cython: 3.0.9
       pandas: 2.1.1
   matplotlib: 3.8.0
       joblib: 1.3.2
threadpoolctl: 3.2.0

Built with OpenMP: True

threadpoolctl info:
       user_api: openmp
   internal_api: openmp
    num_threads: 8
         prefix: libomp
       filepath: /usr/local/Cellar/libomp/17.0.6/lib/libomp.dylib
        version: None

       user_api: blas
   internal_api: openblas
    num_threads: 4
         prefix: libopenblas
       filepath: /Users/lorentzen/github/python3_sklearn/lib/python3.11/site-packages/numpy/.dylibs/libopenblas64_.0.dylib
        version: 0.3.23.dev
threading_layer: pthreads
   architecture: Haswell

       user_api: blas
   internal_api: openblas
    num_threads: 4
         prefix: libopenblas
       filepath: /Users/lorentzen/github/python3_sklearn/lib/python3.11/site-packages/scipy/.dylibs/libopenblas.0.dylib
        version: 0.3.21.dev
threading_layer: pthreads
   architecture: Haswell
```

COMMENT:
> The Ci job finds it 

Yes, that was what I was going to say :wink:, which is reassuring.

Labels: Bug, Build / CI
Misleading OpenMP warning on MacOS when building with Meson

### Describe the bug

Compiling on MacOS with openmp works the old way, see https://scikit-learn.org/dev/developers/advanced_installation.html#macos:
- `brew install libomp`
- ```
  export CC=/usr/bin/clang
  export CXX=/usr/bin/clang++
  export CPPFLAGS="$CPPFLAGS -Xpreprocessor -fopenmp"
  export CFLAGS="$CFLAGS -I/usr/local/opt/libomp/include"
  export CXXFLAGS="$CXXFLAGS -I/usr/local/opt/libomp/include"
  export LDFLAGS="$LDFLAGS -Wl,-rpath,/usr/local/opt/libomp/lib -L/usr/local/opt/libomp/lib -lomp"
  ```
- Build `make in`

With the new meson build system, a warning is raised and scikit-learn is built without openmp:
- same as above, just the last commant is `make dev-meson` instead of `make in`.

### Steps/Code to Reproduce

```
% make dev-meson
```

### Expected Results

No warning and it compiles with openmp enabled.

### Actual Results

```
Run-time dependency OpenMP for c found: NO (tried system)
  ../../sklearn/meson.build:63: WARNING:
                  ***********
                  * WARNING *
                  ***********

  It seems that scikit-learn cannot be built with OpenMP.

  - Make sure you have followed the installation instructions:

      https://scikit-learn.org/dev/developers/advanced_installation.html

  - If your compiler supports OpenMP but you still see this
    message, please submit a bug report at:

      https://github.com/scikit-learn/scikit-learn/issues

  - The build will continue with OpenMP-based parallelism
    disabled. Note however that some estimators will run in
    sequential mode instead of leveraging thread-based
    parallelism.

                      ***
```

### Versions

```shell
System:
    python: 3.11.7 (main, Dec  4 2023, 18:10:11) [Clang 15.0.0 (clang-1500.1.0.2.5)]
executable: /Users/lorentzen/github/python3_sklearn/bin/python
   machine: macOS-14.4-x86_64-i386-64bit

Python dependencies:
      sklearn: 1.5.dev0
          pip: 24.0
   setuptools: 68.0.0
        numpy: 1.26.0
        scipy: 1.11.3
       Cython: 3.0.9
       pandas: 2.1.1
   matplotlib: 3.8.0
       joblib: 1.3.2
threadpoolctl: 3.2.0

Built with OpenMP: True

threadpoolctl info:
       user_api: openmp
   internal_api: openmp
    num_threads: 8
         prefix: libomp
       filepath: /usr/local/Cellar/libomp/17.0.6/lib/libomp.dylib
        version: None

       user_api: blas
   internal_api: openblas
    num_threads: 4
         prefix: libopenblas
       filepath: /Users/lorentzen/github/python3_sklearn/lib/python3.11/site-packages/numpy/.dylibs/libopenblas64_.0.dylib
        version: 0.3.23.dev
threading_layer: pthreads
   architecture: Haswell

       user_api: blas
   internal_api: openblas
    num_threads: 4
         prefix: libopenblas
       filepath: /Users/lorentzen/github/python3_sklearn/lib/python3.11/site-packages/scipy/.dylibs/libopenblas.0.dylib
        version: 0.3.21.dev
threading_layer: pthreads
   architecture: Haswell
```

COMMENT:
I didn’t check if multithreading works despite this warning. Will do…

Labels: Bug, Build / CI
Fetchers docstring examples trigger dataset fetch in CI

Docstring examples were recently added to the fetchers. This makes the doc tests executed by pytest actually fetch the datasets.
In the fetcher tests we took some precaution to not fetch the real datasets, see  https://github.com/scikit-learn/scikit-learn/blob/1bbb2289919935e077fe021df3d42e99beab09e0/sklearn/datasets/tests/test_lfw.py#L40

It has a significant impact on the duration of the test suite (and probably on memory usage as well)
```
27.75s call     ::__init__.py::_lfw.py::sklearn.datasets._lfw.fetch_lfw_pairs
20.98s call     ::__init__.py::_lfw.py::sklearn.datasets._lfw.fetch_lfw_people
```

COMMENT:
This is probably doable to skip doctests in `sklearn/conftest.py` `pytest_collection_modifyitems` based on `SKLEARN_SKIP_NETWORK_TESTS`.

Labels: Build / CI, Performance
Fetchers docstring examples trigger dataset fetch in CI

Docstring examples were recently added to the fetchers. This makes the doc tests executed by pytest actually fetch the datasets.
In the fetcher tests we took some precaution to not fetch the real datasets, see  https://github.com/scikit-learn/scikit-learn/blob/1bbb2289919935e077fe021df3d42e99beab09e0/sklearn/datasets/tests/test_lfw.py#L40

It has a significant impact on the duration of the test suite (and probably on memory usage as well)
```
27.75s call     ::__init__.py::_lfw.py::sklearn.datasets._lfw.fetch_lfw_pairs
20.98s call     ::__init__.py::_lfw.py::sklearn.datasets._lfw.fetch_lfw_people
```

COMMENT:
Uhm normally, I almost sure to have written some code to avoid this using some fixture.

Labels: Build / CI, Performance
Fetchers docstring examples trigger dataset fetch in CI

Docstring examples were recently added to the fetchers. This makes the doc tests executed by pytest actually fetch the datasets.
In the fetcher tests we took some precaution to not fetch the real datasets, see  https://github.com/scikit-learn/scikit-learn/blob/1bbb2289919935e077fe021df3d42e99beab09e0/sklearn/datasets/tests/test_lfw.py#L40

It has a significant impact on the duration of the test suite (and probably on memory usage as well)
```
27.75s call     ::__init__.py::_lfw.py::sklearn.datasets._lfw.fetch_lfw_pairs
20.98s call     ::__init__.py::_lfw.py::sklearn.datasets._lfw.fetch_lfw_people
```

COMMENT:
At list it was the intent behind this PR: 

https://github.com/scikit-learn/scikit-learn/pull/28027/files#diff-3cdc9913d34276436e69cd506f11cdc77ef286069bd5465aaeeb73ac2bdee318

Labels: Build / CI, Performance
Fetchers docstring examples trigger dataset fetch in CI

Docstring examples were recently added to the fetchers. This makes the doc tests executed by pytest actually fetch the datasets.
In the fetcher tests we took some precaution to not fetch the real datasets, see  https://github.com/scikit-learn/scikit-learn/blob/1bbb2289919935e077fe021df3d42e99beab09e0/sklearn/datasets/tests/test_lfw.py#L40

It has a significant impact on the duration of the test suite (and probably on memory usage as well)
```
27.75s call     ::__init__.py::_lfw.py::sklearn.datasets._lfw.fetch_lfw_pairs
20.98s call     ::__init__.py::_lfw.py::sklearn.datasets._lfw.fetch_lfw_people
```

COMMENT:
And indeed, the `fetch_lfw_pairs` is not added as a fixture here:

https://github.com/scikit-learn/scikit-learn/blob/1bbb2289919935e077fe021df3d42e99beab09e0/sklearn/conftest.py#L71-L80

So this is a simple fix.

/take

Labels: Build / CI, Performance
⚠️ CI failed on Linux_nogil.pylatest_pip_nogil ⚠️

**CI failed on [Linux_nogil.pylatest_pip_nogil](https://dev.azure.com/scikit-learn/scikit-learn/_build/results?buildId=65281&view=logs&j=67fbb25f-e417-50be-be55-3b1e9637fce5)** (Mar 26, 2024)
Unable to find junit file. Please see link for details.

COMMENT:
meson is missing from the requirements.

Labels: Bug, Build / CI
⚠️ CI failed on Check Manifest ⚠️

**CI is still failing on [Check Manifest](https://github.com/scikit-learn/scikit-learn/actions/runs/8608201085)** (Apr 09, 2024)

COMMENT:
Looks like check-manifest doesn't support alternative build backends (https://github.com/mgedmin/check-manifest/issues/163)

Labels: Build / CI
⚠️ CI failed on Check Manifest ⚠️

**CI is still failing on [Check Manifest](https://github.com/scikit-learn/scikit-learn/actions/runs/8608201085)** (Apr 09, 2024)

COMMENT:
It looks like `meson dist` doesn't support being run on something that isn't a git repository. Is that also how you read the error message? If yes, it makes me wonder if we can contribute something to meson to make this work (no need to change check manifest)?

Labels: Build / CI
⚠️ CI failed on Check Manifest ⚠️

**CI is still failing on [Check Manifest](https://github.com/scikit-learn/scikit-learn/actions/runs/8608201085)** (Apr 09, 2024)

COMMENT:
Another option would be to switch to [check-sdist](https://github.com/henryiii/check-sdist) as mentioned in https://github.com/mgedmin/check-manifest/issues/163#issuecomment-1494446684.

I quickly checked and it seems like `pipx run check-sdist` is happy and says:
```
Successfully built scikit_learn-1.5.dev0.tar.gz
SDist matches git
```

Full disclosure, I don't have a good grasp of what either `check-manifest` nor `check-sdist` is doing.

Probably a good thing to do would be something like https://github.com/numpy/numpy/issues/23981#issuecomment-1656241060 to make sure there are no major differences in the produced sdist between before and after switching to Meson as our main build backend.

Side-comment: to build a `sdist` it seems like people do:
```
python -m build --sdist
```
for example numpy:
https://github.com/numpy/numpy/blob/c3694335faffe35e3d961feffefdc0a4b9a665da/.github/workflows/wheels.yml#L220-L223

Labels: Build / CI
⚠️ CI failed on Check Manifest ⚠️

**CI is still failing on [Check Manifest](https://github.com/scikit-learn/scikit-learn/actions/runs/8608201085)** (Apr 09, 2024)

COMMENT:
Looking at the [check-sdist source code](https://github.com/henryiii/check-sdist/blob/main/src/check_sdist/__main__.py) I think it should be quite helpful. +1 for replacing check-manifest by check-sdist on my side.

Labels: Build / CI
⚠️ CI failed on Check Manifest ⚠️

**CI is still failing on [Check Manifest](https://github.com/scikit-learn/scikit-learn/actions/runs/8608201085)** (Apr 09, 2024)

COMMENT:
I opened https://github.com/scikit-learn/scikit-learn/pull/28757 to tackle this.

Labels: Build / CI
D2_pinball_score

Hello team, I’m currently utilizing RandomizedSearchCV . Specifically, I’m working on probabilistic forecasting. However, when I use D2_pinball_score as the scoring metric, I encounter an error despite using the supported version. Any insights or guidance would be greatly appreciated.

![image](https://github.com/scikit-learn/scikit-learn/assets/70128255/08f74449-85cc-487d-9833-70c1dcc00a06)

COMMENT:
`"d2_pinball_score"` is not a valid scorer name. You should instead pass a custom scorer object that wraps the `d2_pinball_score` callable with the value of `alpha` you are interested in:

```python
from sklearn.metrics import make_scorer
from sklearn.metrics import d2_pinball_score

pinball_95_scorer = make_scorer(d2_pinball_score, alpha=0.95) 
```

Then you should be able to pass `scoring=pinball_95_scorer` to the `RandomizedSearchCV` constructor.

More details:

https://scikit-learn.org/stable/modules/generated/sklearn.metrics.make_scorer.html

I agree that both the documentation for the scoring parameter, the `make_scorer` utility, the `d2_pinball_score` metric and the error message could be improved because this is not easy to discover the correct usage by yourself.

Labels: Documentation
D2_pinball_score

Hello team, I’m currently utilizing RandomizedSearchCV . Specifically, I’m working on probabilistic forecasting. However, when I use D2_pinball_score as the scoring metric, I encounter an error despite using the supported version. Any insights or guidance would be greatly appreciated.

![image](https://github.com/scikit-learn/scikit-learn/assets/70128255/08f74449-85cc-487d-9833-70c1dcc00a06)

COMMENT:
I'm working on `scikit-learn/sklearn/model_selection/_search.py`

Labels: Documentation
D2_pinball_score

Hello team, I’m currently utilizing RandomizedSearchCV . Specifically, I’m working on probabilistic forecasting. However, when I use D2_pinball_score as the scoring metric, I encounter an error despite using the supported version. Any insights or guidance would be greatly appreciated.

![image](https://github.com/scikit-learn/scikit-learn/assets/70128255/08f74449-85cc-487d-9833-70c1dcc00a06)

COMMENT:
/take

Labels: Documentation
Polars not mentioned as requirement to build documentation

### Describe the issue linked to the documentation

The developer documentation [here](https://scikit-learn.org/dev/developers/contributing.html#building-the-documentation) lists the dependencies required to build the documentation. However, `polars` is not mentioned as a required dependency leading to the following error:
```bash
    Traceback (most recent call last):
      File "/.../scikit-learn/examples/release_highlights/plot_release_highlights_1_4_0.py", line 58, in <module>
        import polars as pl
    ModuleNotFoundError: No module named 'polars'
```

It seems that since version 1.4.0, polars is now also supported.

### Suggest a potential alternative/fix

The easiest fix would be to add `polars` in the documentation [here](https://scikit-learn.org/dev/developers/contributing.html#building-the-documentation) as a needed dependency.
Alternatively, a `requirements.txt` could be created with all the needed dependencies for development - although sure I'm this must have already been discussed in the past and there should be reasons against it.

COMMENT:
Instead of a requirements.txt we could put the doc dependencies (without pinning) as part of our `pyproject.tml` with a specific `doc` subsection. This way we could `pip install -e ".[doc]"` to install the latest version of the doc dependencies.

But +1 for the fix in the doc itself anyways.

/cc @Charlie-XIAO to monitor this and reflect that to the branch with the new theme.

Labels: Documentation
Polars not mentioned as requirement to build documentation

### Describe the issue linked to the documentation

The developer documentation [here](https://scikit-learn.org/dev/developers/contributing.html#building-the-documentation) lists the dependencies required to build the documentation. However, `polars` is not mentioned as a required dependency leading to the following error:
```bash
    Traceback (most recent call last):
      File "/.../scikit-learn/examples/release_highlights/plot_release_highlights_1_4_0.py", line 58, in <module>
        import polars as pl
    ModuleNotFoundError: No module named 'polars'
```

It seems that since version 1.4.0, polars is now also supported.

### Suggest a potential alternative/fix

The easiest fix would be to add `polars` in the documentation [here](https://scikit-learn.org/dev/developers/contributing.html#building-the-documentation) as a needed dependency.
Alternatively, a `requirements.txt` could be created with all the needed dependencies for development - although sure I'm this must have already been discussed in the past and there should be reasons against it.

COMMENT:
Also if we do manage doc dependencies in pyproject.toml, we might want to find a way to use that as the source of truth (combined with `_min_dependencies.py`) to generate the lock files for our CI. I am not sure if conda-lock or similar can do that. /cc @lesteve.

Labels: Documentation