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

merged 9 commits into from
Apr 27, 2020
+39 −13

Conversation

dsaxton
Copy link
Member

@dsaxton dsaxton commented Apr 26, 2020 •
edited
Loading

dsaxton
dsaxton commented Apr 26, 2020
View reviewed changes
pandas/core/frame.py Outdated
Comment on lines 7837 to 7840
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?

@dsaxton dsaxton added NA - MaskedArrays Related to pd.NA and nullable extension arrays Numeric Operations Arithmetic, Comparison, and Logical operations labels Apr 26, 2020
jreback
jreback requested changes Apr 26, 2020
View reviewed changes
Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

can you also use .convert_types() (e.g. like the OP) in the tests

pandas/core/frame.py Outdated
Comment on lines 7837 to 7840
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
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

pandas/tests/frame/methods/test_cov_corr.py Show resolved Hide resolved
@jreback jreback added this to the 1.1 milestone Apr 26, 2020
@dsaxton
Copy link
Member Author

dsaxton commented Apr 27, 2020

can you also use .convert_types() (e.g. like the OP) in the tests

Hmm, I think we wouldn't want that in the test because the functionality we're testing is corr / cov rather than convert_dtypes (the original bug report wasn't minimal, convert_dtypes was working okay)?

jreback
jreback reviewed Apr 27, 2020
View reviewed changes
pandas/core/frame.py Outdated
@@ -7871,16 +7870,16 @@ 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.astype(float).to_numpy()
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add copy=False here

@jreback jreback merged commit 8aa7072 into pandas-dev:master Apr 27, 2020
@jreback
Copy link
Contributor

jreback commented Apr 27, 2020

thanks @dsaxton
when implement DataFrame.to_numpy(na_value=np.nan) let's update this (if you can note that in the issue).

@dsaxton dsaxton mentioned this pull request Apr 28, 2020
Closed
rhshadrach pushed a commit to rhshadrach/pandas that referenced this pull request May 10, 2020
@dsaxton @rhshadrach
BUG: Don't raise in DataFrame.corr with pd.NA (pandas-dev#33809)
1cac270
@dsaxton dsaxton deleted the corr-with-na branch September 27, 2020 03:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TomAugspurger TomAugspurger TomAugspurger left review comments

@jreback jreback jreback approved these changes

Assignees
No one assigned
Labels
NA - MaskedArrays Related to pd.NA and nullable extension arrays Numeric Operations Arithmetic, Comparison, and Logical operations
Projects
None yet
Milestone
1.1
Development

Successfully merging this pull request may close these issues.

BUG: .corr() fails when input is Int64Dtype()
3 participants
@dsaxton @jreback @TomAugspurger