Skip to content

TST: Add test to verify 'dropna' behaviour on SparseArray #34879

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.

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

Merged
merged 3 commits into from
Jun 20, 2020
Merged

TST: Add test to verify 'dropna' behaviour on SparseArray #34879

merged 3 commits into from
Jun 20, 2020

Conversation

MBrouns
Copy link
Contributor

@MBrouns MBrouns commented Jun 20, 2020

verify that calling dropna on a pd.SparseArray does not inadvertently drop non-na records on both DataFrames and the SparseArray itself.

@charlesdong1991 charlesdong1991 added the Testing pandas testing functions or related to the test suite label Jun 20, 2020
Copy link
Member

@charlesdong1991 charlesdong1991 left a comment

Choose a reason for hiding this comment

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

thanks! couple nitpicks, and CI failure is irrelevant.

verify that calling `dropna` on a pd.SparseArray does not
inadvertently drop non-na records on both DataFrames
and the SparseArray itself.
@MBrouns
Copy link
Contributor Author

MBrouns commented Jun 20, 2020

Thanks, incorporated the feedback!

@jorisvandenbossche jorisvandenbossche changed the title Add test to verify dropna behaviour on pd.SparseArray TST: Add test to verify dropna behaviour on pd.SparseArray Jun 20, 2020
@jorisvandenbossche jorisvandenbossche changed the title TST: Add test to verify dropna behaviour on pd.SparseArray TST: Add test to verify 'dropna' behaviour on SparseArray Jun 20, 2020
Copy link
Member

@jorisvandenbossche jorisvandenbossche left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!

The test you added looks good, but I think it would be nice to directly include a test for "invert" as well. See the comment from the original issues: #28287 (comment)

@jorisvandenbossche jorisvandenbossche added the Sparse Sparse Data Type label Jun 20, 2020
@MBrouns
Copy link
Contributor Author

MBrouns commented Jun 20, 2020

The test you added looks good, but I think it would be nice to directly include a test for "invert" as well. See the comment from the original issues: #28287 (comment)

Isn't that already covered by

@pytest.mark.parametrize("fill_value", [True, False])
def test_invert(fill_value):
arr = np.array([True, False, False, True])
sparray = SparseArray(arr, fill_value=fill_value)
result = ~sparray
expected = SparseArray(~arr, fill_value=not fill_value)
tm.assert_sp_array_equal(result, expected)
result = ~pd.Series(sparray)
expected = pd.Series(expected)
tm.assert_series_equal(result, expected)
result = ~pd.DataFrame({"A": sparray})
expected = pd.DataFrame({"A": expected})
tm.assert_frame_equal(result, expected)
?

@jorisvandenbossche
Copy link
Member

Isn't that already covered by

That indeed looks to be the case, thanks for checking!

Copy link
Member

@jorisvandenbossche jorisvandenbossche left a comment

Choose a reason for hiding this comment

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

Thanks for the update!

@jorisvandenbossche
Copy link
Member

(failures are all the html failures)

@jorisvandenbossche jorisvandenbossche merged commit b970ceb into pandas-dev:master Jun 20, 2020
@jorisvandenbossche
Copy link
Member

Thanks!

@jorisvandenbossche jorisvandenbossche added this to the 1.1 milestone Jun 20, 2020
@MBrouns MBrouns deleted the 28287_dropna_sparse_test branch June 20, 2020 15:08
wesbarnett pushed a commit to wesbarnett/pandas that referenced this pull request Jun 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Sparse Sparse Data Type Testing pandas testing functions or related to the test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DataFrame.dropna() not working with sparse columns
3 participants