Skip to content

Commit

Permalink
[SNOW-1320248]: Fix inplace=True on Series objects derived from Ser…
Browse files Browse the repository at this point in the history
…ies. (#2307)

<!---
Please answer these questions before creating your pull request. Thanks!
--->

1. Which Jira issue is this PR addressing? Make sure that there is an
accompanying issue to your PR.

   <!---
   In this section, please add a Snowflake Jira issue number.
   
Note that if a corresponding GitHub issue exists, you should still
include
   the Snowflake Jira issue number. For example, for GitHub issue
#1400, you should
   add "SNOW-1335071" here.
    --->

   Fixes SNOW-1320248

2. Fill out the following pre-review checklist:

- [ ] I am adding a new automated test(s) to verify correctness of my
new code
- [ ] If this test skips Local Testing mode, I'm requesting review from
@snowflakedb/local-testing
   - [ ] I am adding new logging messages
   - [ ] I am adding a new telemetry message
   - [ ] I am adding new credentials
   - [ ] I am adding a new dependency
- [ ] If this is a new feature/behavior, I'm adding the Local Testing
parity changes.

3. Please describe how your code solves the related issue.

This PR fixes the inplace argument for Series functions where the Series
is derived from another Series object (e.g. series = series.iloc[:4];
series.fillna(14, inplace=True))

---------

Co-authored-by: Hazem Elmeleegy <[email protected]>
  • Loading branch information
sfc-gh-rdurrani and sfc-gh-helmeleegy authored Sep 17, 2024
1 parent 6554f82 commit 6a3a77b
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 1 deletion.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

- Fixed a bug where an `Index` object created from a `Series`/`DataFrame` incorrectly updates the `Series`/`DataFrame`'s index name after an inplace update has been applied to the original `Series`/`DataFrame`.
- Suppressed an unhelpful `SettingWithCopyWarning` that sometimes appeared when printing `Timedelta` columns.

- Fixed `inplace` argument for `Series` objects derived from other `Series` objects.

## 1.22.1 (2024-09-11)
This is a re-release of 1.22.0. Please refer to the 1.22.0 release notes for detailed release content.
Expand Down
19 changes: 19 additions & 0 deletions src/snowflake/snowpark/modin/plugin/extensions/series_overrides.py
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,25 @@ def __init__(
self.name = name


@register_series_accessor("_update_inplace")
def _update_inplace(self, new_query_compiler) -> None:
"""
Update the current Series in-place using `new_query_compiler`.
Parameters
----------
new_query_compiler : BaseQueryCompiler
QueryCompiler to use to manage the data.
"""
super(Series, self)._update_inplace(new_query_compiler=new_query_compiler)
# Propagate changes back to parent so that column in dataframe had the same contents
if self._parent is not None:
if self._parent_axis == 1 and isinstance(self._parent, DataFrame):
self._parent[self.name] = self
else:
self._parent.loc[self.index] = self


# Since Snowpark pandas leaves all data on the warehouse, memory_usage's report of local memory
# usage isn't meaningful and is set to always return 0.
@_inherit_docstrings(native_pd.Series.memory_usage, apilink="pandas.Series")
Expand Down
16 changes: 16 additions & 0 deletions tests/integ/modin/series/test_fillna.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
#


import string

import modin.pandas as pd
import numpy as np
import pandas as native_pd
Expand Down Expand Up @@ -201,3 +203,17 @@ def inplace_fillna(df):
native_pd.DataFrame([[1, 2, 3], [4, None, 6]], columns=list("ABC")),
inplace_fillna,
)


@pytest.mark.parametrize("index", [list(range(8)), list(string.ascii_lowercase[:8])])
@sql_count_checker(query_count=1, join_count=4)
def test_inplace_fillna_from_series(index):
def inplace_fillna(series):
series.iloc[:4].fillna(14, inplace=True)
return series

eval_snowpark_pandas_result(
pd.Series([np.nan, 1, 2, 3, 4, 5, 6, 7], index=index),
native_pd.Series([np.nan, 1, 2, 3, 4, 5, 6, 7], index=index),
inplace_fillna,
)

0 comments on commit 6a3a77b

Please sign in to comment.