Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
17 changes: 14 additions & 3 deletions python/ray/dataframe/dataframe.py
Original file line number Diff line number Diff line change
Expand Up @@ -776,9 +776,20 @@ def dropna(self, axis=0, how='any', thresh=None, subset=None,
DataFrame with the dropna applied.
"""
if is_list_like(axis):
raise NotImplementedError(
"To contribute to Pandas on Ray, please visit "
"github.com/ray-project/ray.")
axis = set([pd.DataFrame()._get_axis_number(ax) for ax in axis])
Copy link
Owner

Choose a reason for hiding this comment

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

Don't put it in a set here.

result = self
Copy link
Owner

Choose a reason for hiding this comment

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

unnecessary

Copy link
Author

Choose a reason for hiding this comment

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

I think it is necessary in this case b/c the for loop makes reference to result. this is consistent with the pandas source code.

for ax in axis:
Copy link
Owner

Choose a reason for hiding this comment

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

Add a comment here that this is inefficient since it forces the DataFrame to be built as an intermediate and should be fixed later.

result = result.dropna(
axis=ax, how=how, thresh=thresh, subset=subset)
if not inplace:
return result

return self._update_inplace(
Copy link
Owner

Choose a reason for hiding this comment

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

Pass _block_partitions in instead, it's more efficient.

Copy link
Author

Choose a reason for hiding this comment

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

done!

row_partitions=result._row_partitions,
col_partitions=result._col_partitions,
columns=result._col_metadata.index,
Copy link
Owner

Choose a reason for hiding this comment

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

self.columns here

index=result._row_metadata.index
Copy link
Owner

Choose a reason for hiding this comment

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

self.index here

)

axis = pd.DataFrame()._get_axis_number(axis)
inplace = validate_bool_kwarg(inplace, "inplace")
Expand Down
41 changes: 32 additions & 9 deletions python/ray/dataframe/test/test_dataframe.py
Original file line number Diff line number Diff line change
Expand Up @@ -839,6 +839,7 @@ def test_dense_nan_df():

test_dropna(ray_df, pd_df)
test_dropna_inplace(ray_df, pd_df)
test_dropna_multiple_axes(ray_df, pd_df)


@pytest.fixture
Expand Down Expand Up @@ -1297,16 +1298,17 @@ def test_drop_duplicates():

@pytest.fixture
def test_dropna(ray_df, pd_df):
ray_df_equals_pandas(ray_df.dropna(axis=1, how='all'),
pd_df.dropna(axis=1, how='all'))
assert ray_df_equals_pandas(ray_df.dropna(axis=1, how='all'),
pd_df.dropna(axis=1, how='all'))

ray_df_equals_pandas(ray_df.dropna(axis=1, how='any'),
pd_df.dropna(axis=1, how='any'))
assert ray_df_equals_pandas(ray_df.dropna(axis=1, how='any'),
Copy link
Owner

Choose a reason for hiding this comment

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

Great catch, thanks!

pd_df.dropna(axis=1, how='any'))

ray_df_equals_pandas(ray_df.dropna(axis=0, how='all'),
pd_df.dropna(axis=0, how='all'))
assert ray_df_equals_pandas(ray_df.dropna(axis=0, how='all'),
pd_df.dropna(axis=0, how='all'))

ray_df_equals_pandas(ray_df.dropna(thresh=2), pd_df.dropna(thresh=2))
assert ray_df_equals_pandas(ray_df.dropna(thresh=2),
pd_df.dropna(thresh=2))


@pytest.fixture
Expand All @@ -1317,12 +1319,33 @@ def test_dropna_inplace(ray_df, pd_df):
ray_df.dropna(thresh=2, inplace=True)
pd_df.dropna(thresh=2, inplace=True)

ray_df_equals_pandas(ray_df, pd_df)
assert ray_df_equals_pandas(ray_df, pd_df)

ray_df.dropna(axis=1, how='any', inplace=True)
pd_df.dropna(axis=1, how='any', inplace=True)

ray_df_equals_pandas(ray_df, pd_df)
assert ray_df_equals_pandas(ray_df, pd_df)


@pytest.fixture
def test_dropna_multiple_axes(ray_df, pd_df):
ray_df = ray_df.copy()
Copy link
Owner

Choose a reason for hiding this comment

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

change the name here to ray_df_copy

pd_df = pd_df.copy()
cp = ray_df.copy()
result = ray_df.dropna(how='all', axis=[0, 1])
result2 = ray_df.dropna(how='all', axis=(0, 1))
expected = pd_df.dropna(how='all').dropna(how='all', axis=1)
Copy link
Owner

Choose a reason for hiding this comment

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

You don't need this. For now, just comparing ray and pandas is fine.


assert ray_df_equals_pandas(result, expected)
assert ray_df_equals_pandas(result2, expected)

assert ray_df_equals_pandas(result, expected)
Copy link
Owner

Choose a reason for hiding this comment

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

Why is this code duplicated?

assert ray_df_equals_pandas(result2, expected)
assert ray_df_equals(ray_df, cp)

inp = ray_df.copy()
Copy link
Owner

Choose a reason for hiding this comment

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

You already make a copy above, use it since none of those operations mutated the copy.

inp.dropna(how='all', axis=(0, 1), inplace=True)
assert ray_df_equals_pandas(inp, expected)
Copy link
Owner

Choose a reason for hiding this comment

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

Split inplace test from other test and model after the other dropna tests.



def test_duplicated():
Expand Down