From e5fc074fd229c1ac6d74be1bbd5fffa550a8171f Mon Sep 17 00:00:00 2001 From: Daniel Saxton Date: Sun, 12 Aug 2018 18:47:20 -0500 Subject: [PATCH 1/7] more helpful error message for invalid correlation type --- pandas/core/frame.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pandas/core/frame.py b/pandas/core/frame.py index 638129291b495..236fb8508b26d 100644 --- a/pandas/core/frame.py +++ b/pandas/core/frame.py @@ -6759,7 +6759,7 @@ def corr(self, method='pearson', min_periods=1): elif method == 'spearman': correl = libalgos.nancorr_spearman(ensure_float64(mat), minp=min_periods) - else: + elif method == 'kendall': if min_periods is None: min_periods = 1 mat = ensure_float64(mat).T @@ -6783,6 +6783,8 @@ def corr(self, method='pearson', min_periods=1): c = corrf(ac, bc) correl[i, j] = c correl[j, i] = c + else: + raise ValueError("method must be either 'pearson', 'spearman' or 'kendall'") return self._constructor(correl, index=idx, columns=cols) From ddc0f1404c206fd561c0ae1f18090b4be92268b5 Mon Sep 17 00:00:00 2001 From: Daniel Saxton Date: Sun, 12 Aug 2018 19:01:17 -0500 Subject: [PATCH 2/7] placate pep8 --- pandas/core/frame.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pandas/core/frame.py b/pandas/core/frame.py index 236fb8508b26d..046ca66eda877 100644 --- a/pandas/core/frame.py +++ b/pandas/core/frame.py @@ -6784,7 +6784,8 @@ def corr(self, method='pearson', min_periods=1): correl[i, j] = c correl[j, i] = c else: - raise ValueError("method must be either 'pearson', 'spearman' or 'kendall'") + raise ValueError("method must be either 'pearson', " + "'spearman' or 'kendall'") return self._constructor(correl, index=idx, columns=cols) From 0fce19fbf571c74197266de96249a994ec34c563 Mon Sep 17 00:00:00 2001 From: daniel saxton Date: Mon, 13 Aug 2018 09:02:48 -0500 Subject: [PATCH 3/7] add invalid method test for DataFrame.corr --- pandas/tests/frame/test_analytics.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pandas/tests/frame/test_analytics.py b/pandas/tests/frame/test_analytics.py index f72cf8cdaafe9..8734be17a8644 100644 --- a/pandas/tests/frame/test_analytics.py +++ b/pandas/tests/frame/test_analytics.py @@ -130,6 +130,10 @@ def test_corr_cov_independent_index_column(self): assert result.index is not result.columns assert result.index.equals(result.columns) + def test_corr_invalid_method(self): + df = pd.DataFrame(np.random.normal(size=(10, 2))) + pytest.raises(ValueError, df.corr(method="____")) + def test_cov(self): # min_periods no NAs (corner case) expected = self.frame.cov() From 6490af3309e66c6ffd4c54b295b9c985e4397cfe Mon Sep 17 00:00:00 2001 From: daniel saxton Date: Mon, 13 Aug 2018 11:08:46 -0500 Subject: [PATCH 4/7] try to fix test --- pandas/tests/frame/test_analytics.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pandas/tests/frame/test_analytics.py b/pandas/tests/frame/test_analytics.py index 8734be17a8644..7d1342afe51ca 100644 --- a/pandas/tests/frame/test_analytics.py +++ b/pandas/tests/frame/test_analytics.py @@ -132,7 +132,8 @@ def test_corr_cov_independent_index_column(self): def test_corr_invalid_method(self): df = pd.DataFrame(np.random.normal(size=(10, 2))) - pytest.raises(ValueError, df.corr(method="____")) + with pytest.raises(ValueError): + df.corr(method="____") def test_cov(self): # min_periods no NAs (corner case) From 97da60ccbc38fa04e10018c3ee5f5f8b48bdfa9e Mon Sep 17 00:00:00 2001 From: daniel saxton Date: Mon, 13 Aug 2018 14:11:56 -0500 Subject: [PATCH 5/7] add to release notes for DataFrame.corr error type --- doc/source/whatsnew/v0.24.0.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/source/whatsnew/v0.24.0.txt b/doc/source/whatsnew/v0.24.0.txt index 2ddfba6d01a1b..8539dab77e176 100644 --- a/doc/source/whatsnew/v0.24.0.txt +++ b/doc/source/whatsnew/v0.24.0.txt @@ -468,6 +468,7 @@ Other API Changes - :meth:`PeriodIndex.tz_convert` and :meth:`PeriodIndex.tz_localize` have been removed (:issue:`21781`) - :class:`Index` subtraction will attempt to operate element-wise instead of raising ``TypeError`` (:issue:`19369`) - :class:`pandas.io.formats.style.Styler` supports a ``number-format`` property when using :meth:`~pandas.io.formats.style.Styler.to_excel` (:issue:`22015`) +- :meth:`DataFrame.corr` now raises a ``ValueError`` instead of a ``KeyError`` when supplied with an invalid method. .. _whatsnew_0240.deprecations: From e7d49a138ff12991e7f68c5e4b0a85a3a55284b7 Mon Sep 17 00:00:00 2001 From: Daniel Saxton Date: Mon, 13 Aug 2018 20:41:38 -0500 Subject: [PATCH 6/7] add error for Series, format error messages, edit tests --- pandas/core/frame.py | 3 ++- pandas/core/series.py | 10 ++++++++-- pandas/tests/frame/test_analytics.py | 4 +++- pandas/tests/series/test_analytics.py | 8 ++++++++ 4 files changed, 21 insertions(+), 4 deletions(-) diff --git a/pandas/core/frame.py b/pandas/core/frame.py index 82122a51144be..42a6795935cdc 100644 --- a/pandas/core/frame.py +++ b/pandas/core/frame.py @@ -6684,7 +6684,8 @@ def corr(self, method='pearson', min_periods=1): correl[j, i] = c else: raise ValueError("method must be either 'pearson', " - "'spearman' or 'kendall'") + "'spearman', or 'kendall', '{method}' " + "was supplied".format(method=method)) return self._constructor(correl, index=idx, columns=cols) diff --git a/pandas/core/series.py b/pandas/core/series.py index 4b4fccccda4a0..8d1c768f69cb1 100644 --- a/pandas/core/series.py +++ b/pandas/core/series.py @@ -1931,8 +1931,14 @@ def corr(self, other, method='pearson', min_periods=None): this, other = self.align(other, join='inner', copy=False) if len(this) == 0: return np.nan - return nanops.nancorr(this.values, other.values, method=method, - min_periods=min_periods) + + if method in ['pearson', 'spearman', 'kendall']: + return nanops.nancorr(this.values, other.values, method=method, + min_periods=min_periods) + else: + raise ValueError("method must be either 'pearson', " + "'spearman', or 'kendall', '{method}' " + "was supplied".format(method=method)) def cov(self, other, min_periods=None): """ diff --git a/pandas/tests/frame/test_analytics.py b/pandas/tests/frame/test_analytics.py index 7d1342afe51ca..eee9cea5b07ac 100644 --- a/pandas/tests/frame/test_analytics.py +++ b/pandas/tests/frame/test_analytics.py @@ -132,7 +132,9 @@ def test_corr_cov_independent_index_column(self): def test_corr_invalid_method(self): df = pd.DataFrame(np.random.normal(size=(10, 2))) - with pytest.raises(ValueError): + pttrn = ("method must be either 'pearson', 'spearman', " + "or 'kendall'") + with tm.assert_raises_regex(ValueError, pttrn): df.corr(method="____") def test_cov(self): diff --git a/pandas/tests/series/test_analytics.py b/pandas/tests/series/test_analytics.py index 09e89115e120e..2e60691ff639d 100644 --- a/pandas/tests/series/test_analytics.py +++ b/pandas/tests/series/test_analytics.py @@ -778,6 +778,14 @@ def test_corr_rank(self): tm.assert_almost_equal(A.corr(B, method='kendall'), kexp) tm.assert_almost_equal(A.corr(B, method='spearman'), sexp) + def test_corr_invalid_method(self): + s1 = pd.Series(np.random.randn(10)) + s2 = pd.Series(np.random.randn(10)) + pttrn = ("method must be either 'pearson', 'spearman', " + "or 'kendall'") + with tm.assert_raises_regex(ValueError, pttrn): + s1.corr(s2, method="____") + def test_cov(self): # full overlap tm.assert_almost_equal(self.ts.cov(self.ts), self.ts.std() ** 2) From 5ba0bc8793b4af80157685e55f3426b313bbedb8 Mon Sep 17 00:00:00 2001 From: daniel saxton Date: Tue, 14 Aug 2018 10:46:49 -0500 Subject: [PATCH 7/7] remove else from Series.corr, change pttrn to msg in tests, add PR number to tests and release notes --- doc/source/whatsnew/v0.24.0.txt | 2 +- pandas/core/series.py | 8 ++++---- pandas/tests/frame/test_analytics.py | 7 ++++--- pandas/tests/series/test_analytics.py | 7 ++++--- 4 files changed, 13 insertions(+), 11 deletions(-) diff --git a/doc/source/whatsnew/v0.24.0.txt b/doc/source/whatsnew/v0.24.0.txt index 6a3cfa5e5a7f1..b30b2c885be40 100644 --- a/doc/source/whatsnew/v0.24.0.txt +++ b/doc/source/whatsnew/v0.24.0.txt @@ -468,7 +468,7 @@ Other API Changes - :meth:`PeriodIndex.tz_convert` and :meth:`PeriodIndex.tz_localize` have been removed (:issue:`21781`) - :class:`Index` subtraction will attempt to operate element-wise instead of raising ``TypeError`` (:issue:`19369`) - :class:`pandas.io.formats.style.Styler` supports a ``number-format`` property when using :meth:`~pandas.io.formats.style.Styler.to_excel` (:issue:`22015`) -- :meth:`DataFrame.corr` now raises a ``ValueError`` instead of a ``KeyError`` when supplied with an invalid method. +- :meth:`DataFrame.corr` and :meth:`Series.corr` now raise a ``ValueError`` along with a helpful error message instead of a ``KeyError`` when supplied with an invalid method (:issue:`22298`) .. _whatsnew_0240.deprecations: diff --git a/pandas/core/series.py b/pandas/core/series.py index 8d1c768f69cb1..8bf87e0ede664 100644 --- a/pandas/core/series.py +++ b/pandas/core/series.py @@ -1935,10 +1935,10 @@ def corr(self, other, method='pearson', min_periods=None): if method in ['pearson', 'spearman', 'kendall']: return nanops.nancorr(this.values, other.values, method=method, min_periods=min_periods) - else: - raise ValueError("method must be either 'pearson', " - "'spearman', or 'kendall', '{method}' " - "was supplied".format(method=method)) + + raise ValueError("method must be either 'pearson', " + "'spearman', or 'kendall', '{method}' " + "was supplied".format(method=method)) def cov(self, other, min_periods=None): """ diff --git a/pandas/tests/frame/test_analytics.py b/pandas/tests/frame/test_analytics.py index eee9cea5b07ac..f06c8336373ca 100644 --- a/pandas/tests/frame/test_analytics.py +++ b/pandas/tests/frame/test_analytics.py @@ -131,10 +131,11 @@ def test_corr_cov_independent_index_column(self): assert result.index.equals(result.columns) def test_corr_invalid_method(self): + # GH PR #22298 df = pd.DataFrame(np.random.normal(size=(10, 2))) - pttrn = ("method must be either 'pearson', 'spearman', " - "or 'kendall'") - with tm.assert_raises_regex(ValueError, pttrn): + msg = ("method must be either 'pearson', 'spearman', " + "or 'kendall'") + with tm.assert_raises_regex(ValueError, msg): df.corr(method="____") def test_cov(self): diff --git a/pandas/tests/series/test_analytics.py b/pandas/tests/series/test_analytics.py index 2e60691ff639d..dccc12643a6ad 100644 --- a/pandas/tests/series/test_analytics.py +++ b/pandas/tests/series/test_analytics.py @@ -779,11 +779,12 @@ def test_corr_rank(self): tm.assert_almost_equal(A.corr(B, method='spearman'), sexp) def test_corr_invalid_method(self): + # GH PR #22298 s1 = pd.Series(np.random.randn(10)) s2 = pd.Series(np.random.randn(10)) - pttrn = ("method must be either 'pearson', 'spearman', " - "or 'kendall'") - with tm.assert_raises_regex(ValueError, pttrn): + msg = ("method must be either 'pearson', 'spearman', " + "or 'kendall'") + with tm.assert_raises_regex(ValueError, msg): s1.corr(s2, method="____") def test_cov(self):