Repository: scikit-learn/scikit-learn
Pull request: FIX check X in LedoitWolf to not allow 0 samples array
#### Reference Issues/PRs

Fix #20362 

#### What does this implement/fix? Explain your changes.

This pull-request implements the array checks to the following functions:

- ledoit_wolf_shrinkage
- ledoit_wolf

#### Any other comments?

Should we extend this check to other covariance functions?
If yes, please tell me.

-- 

This is my first code contribution on Github, be patient! 

Problem statement:
`sklearn.covariance.ledoit_wolf` throws ZeroDivisionError


#### Describe the bug
`sklearn.covariance.ledoit_wolf` throws `ZeroDivisionError` when `X` is an empty array

#### Steps/Code to Reproduce


~~~python
import numpy as np
from sklearn.covariance import ledoit_wolf
ledoit_wolf(X=np.zeros((0,2)))
~~~

#### Expected Results
Expect handle the 0 gracefully.

#### Actual Results
~~~python
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/root/miniconda3/lib/python3.7/site-packages/sklearn/utils/validation.py", line 63, in inner_f
    return f(*args, **kwargs)
  File "/root/miniconda3/lib/python3.7/site-packages/sklearn/covariance/_shrunk_covariance.py", line 310, in ledoit_wolf
    X, assume_centered=assume_centered, block_size=block_size)
  File "/root/miniconda3/lib/python3.7/site-packages/sklearn/covariance/_shrunk_covariance.py", line 243, in ledoit_wolf_shrinkage
    beta = 1. / (n_features * n_samples) * (beta_ / n_samples - delta_)
ZeroDivisionError: float division by zero
~~~

#### Versions
Linux-5.4.104+-x86_64-with-Ubuntu-18.04-bionic
Python 3.7.10 (default, May 3 2021, 02:48:31)
[GCC 7.5.0]
NumPy 1.19.5
SciPy 1.4.1
Scikit-Learn 0.24.1

Hints from issue discussion:
Hi,

What should be the _ledoit_wolf_ behaviour when giving an empty array?
Do you except a personalized scikit-learn error or do you except having a zero return?

On my side, I think it should raise an error of the kind: _ValueError: Found array with 0 sample(s) (shape=(0, 2)) while a minimum of 1 is required._

Maybe we should pass the X parameter to check_array...

Commit message: add array checks to ledoit_wolf covariances

Patch to review:
diff --git a/sklearn/covariance/_shrunk_covariance.py b/sklearn/covariance/_shrunk_covariance.py
index 494c65d01186c..952ac777e947a 100644
--- a/sklearn/covariance/_shrunk_covariance.py
+++ b/sklearn/covariance/_shrunk_covariance.py
@@ -198,6 +198,7 @@ def ledoit_wolf_shrinkage(X, assume_centered=False, block_size=1000):
     where mu = trace(cov) / n_features
     """
     X = np.asarray(X)
+    X = check_array(X, accept_sparse=True)
     # for only one feature, the result is the same whatever the shrinkage
     if len(X.shape) == 2 and X.shape[1] == 1:
         return 0.0
@@ -297,6 +298,7 @@ def ledoit_wolf(X, *, assume_centered=False, block_size=1000):
     where mu = trace(cov) / n_features
     """
     X = np.asarray(X)
+    X = check_array(X, accept_sparse=True)
     # for only one feature, the result is the same whatever the shrinkage
     if len(X.shape) == 2 and X.shape[1] == 1:
         if not assume_centered:


Code review comments:
diff_hunk: @@ -198,6 +198,7 @@ def ledoit_wolf_shrinkage(X, assume_centered=False, block_size=1000):
     where mu = trace(cov) / n_features
     """
     X = np.asarray(X)
+    X = check_array(X, accept_sparse=True)
path: sklearn/covariance/_shrunk_covariance.py
text: @user1:
Does this really support sparse data? I don't believe so. I cannot see any test for running Ledoit Wolf covariance estimation from data stored in scipy sparse matrices.

@author:
From what I see [in the docs](https://scikit-learn.org/stable/modules/generated/sklearn.covariance.ledoit_wolf.html), there is an example of sparse inverse covariance estimation using Ledoit Wolf. You can find this example [here](https://scikit-learn.org/stable/auto_examples/covariance/plot_sparse_cov.html#sphx-glr-auto-examples-covariance-plot-sparse-cov-py).

However, you are right... the function is not directly feeded with a sparse matrix.

Should I create a commit removing the _accept_sparse_ parameter?





@user1:
This is example is about obtaining a precision matrix with many zeros (square matrix of shape `n_features` by `n_features`). It does not depend however on the physical memory layout of the (rectangular) input data matrix X of (shape `n_samples` by `n_features`).

> Should I create a commit removing the accept_sparse parameter?

I believe so.

@author:
@user1 Done. Thank you for your review!

diff_hunk: @@ -198,6 +198,7 @@ def ledoit_wolf_shrinkage(X, assume_centered=False, block_size=1000):
     where mu = trace(cov) / n_features
     """
     X = np.asarray(X)
path: sklearn/covariance/_shrunk_covariance.py
text: @user1:
The `asarray` call is now redundant.

```suggestion
```