ENH: adding covariance param to corrcoef - see ticket #19852 #21434
Conversation
There was a problem hiding this comment.
Thank you! I am not sure where the decision was standing on adding this, but it seems reasonable to me.
I added a few comments for things that need to be fixed. But the main task implementation-wise will be to add tests for the new feature and also for all the relevant error paths!
numpy/lib/function_base.py
Outdated
| @@ -2681,14 +2681,14 @@ def cov(m, y=None, rowvar=True, bias=False, ddof=None, fweights=None, | ||
| return c.squeeze() | ||
|
|
||
|
|
||
| def _corrcoef_dispatcher(x, y=None, rowvar=None, bias=None, ddof=None, *, | ||
| def _corrcoef_dispatcher(x=None, cov = None, y=None, rowvar=None, bias=None, ddof=None, *, |
There was a problem hiding this comment.
The new parameter must be added at the end (or after the *, since order is irrelevant after that). In principle we could add it last, but in practice we want users to write cov=... anyway.
numpy/lib/function_base.py
Outdated
| @@ -2728,6 +2728,9 @@ def corrcoef(x, y=None, rowvar=True, bias=np._NoValue, ddof=np._NoValue, *, | ||
|
|
||
| .. versionadded:: 1.20 | ||
|
|
||
| cov : _NoValue, optional |
There was a problem hiding this comment.
This should be the actual type, i.e. array_like probably
numpy/lib/function_base.py
Outdated
| @@ -2818,7 +2821,12 @@ def corrcoef(x, y=None, rowvar=True, bias=np._NoValue, ddof=np._NoValue, *, | ||
| # 2015-03-15, 1.10 | ||
| warnings.warn('bias and ddof have no effect and are deprecated', | ||
| DeprecationWarning, stacklevel=3) | ||
| c = cov(x, y, rowvar, dtype=dtype) | ||
| if x is None and cov is None: | ||
| raise ValueError("'x' or 'cov' must be passed as a parameter") |
There was a problem hiding this comment.
This sould be a TypeError, it should be similar to what you currently get for np.corrcoef() after all.
There was a problem hiding this comment.
I don't think you check for both being passed. We need to check that exactly one of them is passed only.
numpy/lib/function_base.py
Outdated
| @@ -2839,6 +2847,9 @@ def corrcoef(x, y=None, rowvar=True, bias=np._NoValue, ddof=np._NoValue, *, | ||
| return c | ||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
Please remove these additional blank lines again.
Param cov was added to corrcoef func in order to accept a pre-calculated covariance matrix