Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

BUG: Don't raise in DataFrame.corr with pd.NA #33809

Merged
merged 9 commits into from
Apr 27, 2020
Merged

BUG: Don't raise in DataFrame.corr with pd.NA #33809

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
36be92c
BUG: Don't raise in DataFrame.corr with pd.NA
dsaxton Apr 26, 2020
30df6b6
whatsnew and style
dsaxton Apr 26, 2020
9dba02a
Skip scipy
dsaxton Apr 26, 2020
8e56a3f
Issue number
dsaxton Apr 27, 2020
1691065
Merge remote-tracking branch 'upstream/master' into corr-with-na
dsaxton Apr 27, 2020
8237cb1
astype to float
dsaxton Apr 27, 2020
0745ca8
Merge remote-tracking branch 'upstream/master' into corr-with-na
dsaxton Apr 27, 2020
c2dd554
Try without ensure_float
dsaxton Apr 27, 2020
c2c059b
Don't copy
dsaxton Apr 27, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion doc/source/whatsnew/v1.1.0.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,7 @@ Numeric
- Bug in :meth:`DataFrame.mean` with ``numeric_only=False`` and either ``datetime64`` dtype or ``PeriodDtype`` column incorrectly raising ``TypeError`` (:issue:`32426`)
- Bug in :meth:`DataFrame.count` with ``level="foo"`` and index level ``"foo"`` containing NaNs causes segmentation fault (:issue:`21824`)
- Bug in :meth:`DataFrame.diff` with ``axis=1`` returning incorrect results with mixed dtypes (:issue:`32995`)
-
- Bug in :meth:`DataFrame.corr` and :meth:`DataFrame.cov` raising when handling nullable integer columns with ``pandas.NA`` (:issue:`33803`)

Conversion
^^^^^^^^^^
Expand Down
22 changes: 14 additions & 8 deletions pandas/core/frame.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7834,7 +7834,10 @@ def corr(self, method="pearson", min_periods=1) -> "DataFrame":
numeric_df = self._get_numeric_data()
cols = numeric_df.columns
idx = cols.copy()
mat = numeric_df.values
mat = numeric_df.to_numpy()
if is_object_dtype(mat.dtype):
# We end up with an object array if pd.NA is present
mat[isna(mat)] = np.nan
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels like a hack, and it seems it should somehow be possible to cast this whole DataFrame to float using to_numpy but that doesn't seem to work

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pass na_value=np.nan

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was trying that originally but it turns out DataFrame.to_numpy doesn't have an na_value argument (seems only Series and EAs do)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm @TomAugspurger

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#33820

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll want to avoid going through object dtype at all. Can you specify dtype="float"? Or does that break other things?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That raises an error:

TypeError: float() argument must be a string or a number, not 'NAType'

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That matches the behavior of Series.to_numpy(). We need to have the NA value passed through as np.nan I think.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think as a workaround astyping to float then calling to_numpy works, then later could use na_value for DataFrame.to_numpy directly?


if method == "pearson":
correl = libalgos.nancorr(ensure_float64(mat), minp=min_periods)
Expand Down Expand Up @@ -7969,19 +7972,22 @@ def cov(self, min_periods=None) -> "DataFrame":
numeric_df = self._get_numeric_data()
cols = numeric_df.columns
idx = cols.copy()
mat = numeric_df.values
mat = numeric_df.to_numpy()
if is_object_dtype(mat.dtype):
# We end up with an object array if pd.NA is present
mat[isna(mat)] = np.nan

if notna(mat).all():
if min_periods is not None and min_periods > len(mat):
baseCov = np.empty((mat.shape[1], mat.shape[1]))
baseCov.fill(np.nan)
base_cov = np.empty((mat.shape[1], mat.shape[1]))
base_cov.fill(np.nan)
else:
baseCov = np.cov(mat.T)
baseCov = baseCov.reshape((len(cols), len(cols)))
base_cov = np.cov(mat.T)
base_cov = base_cov.reshape((len(cols), len(cols)))
else:
baseCov = libalgos.nancorr(ensure_float64(mat), cov=True, minp=min_periods)
base_cov = libalgos.nancorr(ensure_float64(mat), cov=True, minp=min_periods)

return self._constructor(baseCov, index=idx, columns=cols)
return self._constructor(base_cov, index=idx, columns=cols)

def corrwith(self, other, axis=0, drop=False, method="pearson") -> Series:
"""
Expand Down
24 changes: 24 additions & 0 deletions pandas/tests/frame/methods/test_cov_corr.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,16 @@ def test_cov(self, float_frame, float_string_frame):
)
tm.assert_frame_equal(result, expected)

@pytest.mark.parametrize(
"other_column", [pd.array([1, 2, 3]), np.array([1.0, 2.0, 3.0])]
)
def test_cov_nullable_integer(self, other_column):
data = pd.DataFrame({"a": pd.array([1, 2, None]), "b": other_column})
result = data.cov()
arr = np.array([[0.5, 0.5], [0.5, 1.0]])
expected = pd.DataFrame(arr, columns=["a", "b"], index=["a", "b"])
tm.assert_frame_equal(result, expected)


class TestDataFrameCorr:
# DataFrame.corr(), as opposed to DataFrame.corrwith
Expand Down Expand Up @@ -153,6 +163,20 @@ def test_corr_int(self):
df3.cov()
df3.corr()

@pytest.mark.parametrize(
"nullable_column", [pd.array([1, 2, 3]), pd.array([1, 2, None])]
)
@pytest.mark.parametrize(
"other_column",
[pd.array([1, 2, 3]), np.array([1.0, 2.0, 3.0]), np.array([1.0, 2.0, np.nan])],
)
@pytest.mark.parametrize("method", ["pearson", "spearman", "kendall"])
def test_corr_nullable_integer(self, nullable_column, other_column, method):
data = pd.DataFrame({"a": nullable_column, "b": other_column})
jreback marked this conversation as resolved.
Show resolved Hide resolved
result = data.corr(method=method)
expected = pd.DataFrame(np.ones((2, 2)), columns=["a", "b"], index=["a", "b"])
tm.assert_frame_equal(result, expected)


class TestDataFrameCorrWith:
def test_corrwith(self, datetime_frame):
Expand Down