Adds triangular option to DataFrame.corr, closes #22840
State: closed

Adds the `tri` argument to `DataFrame.corr`.

- [ x ] closes #22840
- [ x ] tests added / passed
- [ x ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ x ] whatsnew entry


Labels:

Comments:
Hello @dsaxton! Thanks for submitting the PR.

 - There are no PEP8 issues in the file [`pandas/core/frame.py`](https://github.com/dsaxton/pandas/blob/9a95e8c1c26193398ad534540770eb89b5d85e7b/pandas/core/frame.py) !

 - There are no PEP8 issues in the file [`pandas/tests/frame/test_analytics.py`](https://github.com/dsaxton/pandas/blob/9a95e8c1c26193398ad534540770eb89b5d85e7b/pandas/tests/frame/test_analytics.py) !


Some general comments:

1. If we were to include a `tri` argument, I would prefer a `tri=None(default)/'upper'/'lower'` instead of `tri=True/False` for just the lower triangular so users can have the option of the upper triangular.
2. In your implementation, it appears fill value would be `NaN` as opposed to numpy (triu/trul) which the fill value is 0. I am not sure what the preferred fill value should be, but in general we try to stay consistent with numpy conventions. 
@mroeschke Good points, I can change the argument to "upper", "lower" or None rather than True or False.  It's interesting that `numpy` uses zeros for omitted entries, would be curious to hear how others feel.
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/22842?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`master@a03d953`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `90%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/22842/graphs/tree.svg?width=650&token=eZ4WkYLtcO&height=150&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/22842?src=pr&el=tree)

```diff
@@            Coverage Diff            @@
##             master   #22842   +/-   ##
=========================================
  Coverage          ?   90.61%           
=========================================
  Files             ?      169           
  Lines             ?    50835           
  Branches          ?        0           
=========================================
  Hits              ?    46062           
  Misses            ?     4773           
  Partials          ?        0
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.61% <90%> (?)` | |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/22842?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/22842/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `97.16% <90%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/22842?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/22842?src=pr&el=footer). Last update [a03d953...9df2bcb](https://codecov.io/gh/pandas-dev/pandas/pull/22842?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

Yes.

On Thu, Sep 27, 2018 at 8:37 AM Daniel Saxton <notifications@github.com>
wrote:

> *@dsaxton* commented on this pull request.
> ------------------------------
>
> In pandas/core/frame.py
> <https://github.com/pandas-dev/pandas/pull/22842#discussion_r220925527>:
>
> > @@ -6686,6 +6686,12 @@ def corr(self, method='pearson', min_periods=1):
>              to have a valid result. Currently only available for pearson
>              and spearman correlation
>
> +        tri : {'upper', 'lower'} or None, default : None
> +            Whether or not to return the upper / lower triangular
> +            correlation matrix
>
> Sure, you mean within the docstring itself right?
>
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/22842#discussion_r220925527>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ABQHIk8oEsMuV4-Al9Lqc3WGAyFg3c1jks5ufNSSgaJpZM4W7Um_>
> .
>

> 2\. In your implementation, it appears fill value would be `NaN` as opposed to numpy (triu/trul) which the fill value is 0. I am not sure what the preferred fill value should be, but in general we try to stay consistent with numpy conventions.

NumPy's tril/u are standard matrix algebra ops.  In the use case argued for this api change, the reason is to have simple ops on the correlation to find certain features.  Presumably you want to not double count.

I don't really see why a masked triangular is the right approach to the stated problem.  For example, if I wanted to find the variable with the highest correlation, I would be just as happy to have all elements except the diagonal.  If I want to find the variable with the highest average correlation to the others, I need a full matrix except the diagonal to make it easy to call sum()

`tri` is probably the wrong description.  `mask` with choices {lower,upper, diag} seems like it would be more general.
@bashtage It allows for simple one-liners like `corr_mat.where(corr_mat.abs() > 0.9).dropna(how="all", axis=1).columns` to extract the names of single columns from highly correlated pairs as mentioned in #22840.  This doesn't work with the full correlation matrix, or the matrix with only the diagonal removed.
@TomAugspurger Regarding the docstring example, should it be something as in the most recent commit, or perhaps just the full lower triangular form itself?
`corr_mat.where(corr_mat.abs() > 0.9).dropna(how="all", axis=1).columns` is a strnage operation since it is order dependent while a correlation matrix is not.  In particular, this method favors dropping early columns, and so the utility of the method depends on users knowing that order matters.  I would say this is why masking the diagonal is a more natural method.  

```python
np.random.seed(1)
x = np.random.standard_normal((20,5))+3*np.random.standard_normal((20,1))
df = pd.DataFrame(x,columns = ['c{0}'.format(i) for i in range(5)])
corr_mat = df.corr()
mask = ~np.triu(np.ones((5,5),dtype=np.bool))
c2 = corr_mat.where(mask)
c2
```

```
Out[40]: 
          c0        c1        c2        c3  c4
c0       NaN       NaN       NaN       NaN NaN
c1  0.828210       NaN       NaN       NaN NaN
c2  0.852960  0.938965       NaN       NaN NaN
c3  0.831917  0.919567  0.921551       NaN NaN
c4  0.917205  0.899102  0.916828  0.881752 NaN
```

```python
c2.where(c2.abs() > 0.9).dropna(how="all", axis=1).columns 
```

shows
```
Out[41]: Index(['c0', 'c1', 'c2'], dtype='object')
```

But c4 has a correlation > 0.9, as does c3.

Masking the diagonal

```python
mask = np.eye(5, dtype=np.bool)
c3 = corr_mat.where(mask)
c3.where(c3.abs() > 0.9).dropna(how="all", axis=1).columns 
```

returns the list of variables that have any correlation > 0.9 without inducing an order dependnce

```
Out[45]: Index(['c0', 'c1', 'c2', 'c3', 'c4'], dtype='object')
```


@bashtage The reason for `corr_mat.where(corr_mat.abs() > 0.9).dropna(how="all", axis=1).columns` is to (arbitrarily, so we don't care if the choice is affected by the order of columns) select _one_ column from each correlated pair (we don't want to select every column).

In your example we would select all but `c3` and `c4` because these do not have a correlation of at least 0.9 with each other.  The point is to select a minimal set of columns we'd need to remove to get rid of the high correlations.
Here is a better example of why this mask can be very misleading.  Suppose the correlation matrix is
```
1.000 0.880 0.880 0.914
0.880 1.000 0.880 0.914
0.880 0.880 1.000 0.914
0.914 0.914 0.914 1.000
```

Looking at a triangular view (lower) would delete all but the last. 

Pivoting this column to be in the middle makes the selection used either triangular method even more problematic.  

My main point is that this seems to be a non-general mask that is not even well suited to removing variables that are highly correlated . One algorithm used in some LASSO estimators does thie following -- ding the pair with the highest correlation and then remove the the element that has the highest correaltion with the remaining -- this isn't easy to implement with a lower triangular masked view.
I am not really in love with this option. I would prefer this simply be a cookbook example.
@jreback Ok, I will close the PR and do that instead.