issue_comments

thanks. Do you want to put in a PR fixing that?

Awesome @AndrewWilliams3142 ! Very excited we have this.

Thanks for the review @mathause

Hitting merge; any other feedback is welcome and we can iterate.

If you insist ;)

da_a -= da_a.mean(dim=dim) is indeed marginally faster. As they are already aligned, we don't have to worry about this.

Could you also add a test for the TypeError?

python with raises_regex(TypeError, "Only xr.DataArray is supported"): xr.corr(xr.Dataset(), xr.Dataset())

This looks great! I think it's fine to have a simple implementation like this, even if it's not perfectly efficient. Well done for getting something working @AndrewWilliams3142 .

For the future: let's move away from using np.random in docstrings, since they're not reproducible, and will never pass a doctest.

Anything remaining before merging?

no worries about hypothesis, that's something we can add in a new PR.

Also, I don't think there is a hypothesis.extra.xarray module, yet. Any comments on that, @Zac-HD?

Currently corr needs to sanitize the inputs twice, which will be inefficient. One way around this is to define an internal method which can do both, depending on a method keyword (no need to write extra tests for this IMHO):

```python

def corr(da_a, da_b, dim=None, ddof=0):

    return _cov_corr(da_a, da_b, dim=None, ddof=0, method="corr")

def cov(da_a, da_b, dim=None, ddof=0):

return _cov_corr(da_a, da_b, dim=None, ddof=0, method="cov")

def _cov_corr(da_a, da_b, dim=None, ddof=0, method=None):

# compute cov

if method = "cov":
    return cov

# compute corr

return corr

```

Maybe you could use xr.apply_ufunc instead of looping in the tests (might be overkill).

If you want to test individual values without reimplementing the function in the tests (which is what I suspect comparing with the result of np.cov would require), that might be the only way.

If not, you could also check properties of covariance / correlation matrices, e.g. that assert_allclose(xr.cov(a, b) / (a.std() * b.std()), xr.corr(a, b)) (I'm not sure if I remember that formula correctly) or that the diagonal of the auto-covariance matrix is the same as the variance of the array (with a 1D vector, not sure about more dimensions). If you decide to test using properties, you could also extend our small collection of tests using hypothesis (see #1846).