Skip to content

Commit

Permalink
SNOW-1573367 Fix not raise NotImplementedError for seting cell with l…
Browse files Browse the repository at this point in the history
…ist (#2027)
  • Loading branch information
sfc-gh-azhan authored Aug 5, 2024
1 parent 5aa08c8 commit 6e7b721
Show file tree
Hide file tree
Showing 6 changed files with 66 additions and 15 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
- Fixed a bug in `DataFrame.to_pandas_batches` where the iterator could throw an error if certain transformation is made to the pandas dataframe due to wrong isolation level.
- Fixed a bug in `DataFrame.lineage.trace` to split the quoted feature view's name and version correctly.
- Fixed a bug in `Column.isin` that caused invalid sql generation when passed an empty list.
- Fixed a bug that fails to raise NotImplementedError while setting cell with list like item.

### Snowpark Local Testing Updates

Expand Down
35 changes: 24 additions & 11 deletions src/snowflake/snowpark/modin/pandas/indexing.py
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,25 @@ def validate_key_for_at_iat(
)


def raise_set_cell_with_list_like_value_error(
df: BasePandasDataset,
item: INDEXING_ITEM_TYPE,
row_loc: INDEXING_LOCATOR_TYPE,
col_loc: INDEXING_LOCATOR_TYPE,
) -> None:
"""
Raise NotImplementedError when setting cell with list like item
"""
if is_list_like(item) or isinstance(item, pd.Series):
# item is list like or a series
if is_scalar(row_loc) and (
isinstance(df, pd.Series)
or (isinstance(df, pd.DataFrame) and is_scalar(col_loc))
):
# locators indicate setting a cell
ErrorMessage.not_implemented(SET_CELL_WITH_LIST_LIKE_VALUE_ERROR_MESSAGE)


class _LocationIndexerBase:
"""
Base class for location indexer like loc and iloc.
Expand Down Expand Up @@ -949,7 +968,7 @@ def __setitem__(
f".{self.api_name} set for multiindex is not yet implemented"
)

self._validate_item_type(item, row_loc)
self._validate_item_type(item, row_loc, col_loc)

# If the row key is list-like (Index, list, np.ndarray, etc.), convert it to Series.
if not isinstance(row_loc, pd.Series) and is_list_like(row_loc):
Expand Down Expand Up @@ -1000,6 +1019,7 @@ def _validate_item_type(
self,
item: INDEXING_ITEM_TYPE,
row_loc: Union[Scalar, list, slice, tuple, AnyArrayLike],
col_loc: Union[Scalar, list, slice, tuple, AnyArrayLike],
) -> None:
"""
Validate item data type for loc set. Raise error if the type is invalid.
Expand All @@ -1016,12 +1036,6 @@ def _validate_item_type(
if isinstance(self.df, pd.Series):
if isinstance(item, pd.DataFrame):
raise ValueError(LOC_SET_INCOMPATIBLE_INDEXER_WITH_DF_ERROR_MESSAGE)
elif is_scalar(row_loc) and (
isinstance(item, pd.Series) or is_list_like(item)
):
ErrorMessage.not_implemented(
SET_CELL_WITH_LIST_LIKE_VALUE_ERROR_MESSAGE
)
else:
if is_scalar(row_loc) and (
isinstance(item, pd.DataFrame) or is_2d_array(item)
Expand All @@ -1032,6 +1046,8 @@ def _validate_item_type(
)
)

raise_set_cell_with_list_like_value_error(self.df, item, row_loc, col_loc)

if (isinstance(row_loc, pd.Series) or is_list_like(row_loc)) and (
isinstance(item, range)
):
Expand Down Expand Up @@ -1245,10 +1261,7 @@ def __setitem__(
is_item_series = isinstance(item, pd.Series)

if not isinstance(item, BasePandasDataset) and is_list_like(item):
if isinstance(self.df, pd.Series) and is_scalar(row_loc):
ErrorMessage.not_implemented(
SET_CELL_WITH_LIST_LIKE_VALUE_ERROR_MESSAGE
)
raise_set_cell_with_list_like_value_error(self.df, item, row_loc, col_loc)

if isinstance(item, pd.Index):
item = np.array(item.tolist()).transpose()
Expand Down
9 changes: 9 additions & 0 deletions tests/integ/modin/frame/test_at.py
Original file line number Diff line number Diff line change
Expand Up @@ -201,3 +201,12 @@ def test_at_multiindex_neg(
snowpark_df = pd.DataFrame(native_df)
with pytest.raises(error):
snowpark_df.at[key]


@sql_count_checker(query_count=0)
def test_raise_set_cell_with_list_like_value_error():
s = pd.Series([[1, 2], [3, 4]])
with pytest.raises(NotImplementedError):
s.at[0] = [0, 0]
with pytest.raises(NotImplementedError):
s.to_frame().at[0, 0] = [0, 0]
9 changes: 9 additions & 0 deletions tests/integ/modin/frame/test_iat.py
Original file line number Diff line number Diff line change
Expand Up @@ -290,3 +290,12 @@ def test_iat_neg(
):
with pytest.raises(error):
default_index_snowpark_pandas_df.iat[key]


@sql_count_checker(query_count=0)
def test_raise_set_cell_with_list_like_value_error():
s = pd.Series([[1, 2], [3, 4]])
with pytest.raises(NotImplementedError):
s.iat[0] = [0, 0]
with pytest.raises(NotImplementedError):
s.to_frame().iat[0, 0] = [0, 0]
9 changes: 9 additions & 0 deletions tests/integ/modin/frame/test_iloc.py
Original file line number Diff line number Diff line change
Expand Up @@ -3196,3 +3196,12 @@ def iloc_helper(df):
iloc_helper,
inplace=True,
)


@sql_count_checker(query_count=0)
def test_raise_set_cell_with_list_like_value_error():
s = pd.Series([[1, 2], [3, 4]])
with pytest.raises(NotImplementedError):
s.iloc[0] = [0, 0]
with pytest.raises(NotImplementedError):
s.to_frame().iloc[0, 0] = [0, 0]
18 changes: 14 additions & 4 deletions tests/integ/modin/frame/test_loc.py
Original file line number Diff line number Diff line change
Expand Up @@ -2547,10 +2547,11 @@ def set_loc_helper(df):
# these cases
("a", [1], True, False),
("a", (1,), True, False),
("w", [1], False, False),
("w", (1,), False, False),
("a", np.array([1]), False, False),
("a", native_pd.Index([1]), False, False),
# Snowpark pandas does not support set cell with list like item
("w", [1], False, True),
("w", (1,), False, True),
("a", np.array([1]), False, True),
("a", native_pd.Index([1]), False, True),
],
)
def test_df_loc_set_scalar_with_item_negative(
Expand Down Expand Up @@ -3945,3 +3946,12 @@ def loc_set_helper(df):
}
)
eval_snowpark_pandas_result(snow_df, native_df, loc_set_helper, inplace=True)


@sql_count_checker(query_count=0)
def test_raise_set_cell_with_list_like_value_error():
s = pd.Series([[1, 2], [3, 4]])
with pytest.raises(NotImplementedError):
s.loc[0] = [0, 0]
with pytest.raises(NotImplementedError):
s.to_frame().loc[0, 0] = [0, 0]

0 comments on commit 6e7b721

Please sign in to comment.