SNOW-1063347: Remove series.py #2205

sfc-gh-joshi · 2024-08-30T22:46:57Z

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

Fixes SNOW-1063347
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.
Please describe how your code solves the related issue.

This PR removes series.py (following up #2167). As before, any preserved overrides are given a reason in code comments. The following methods have been added to series_overrides.py:

init
sub/subtract, and most other binary operators
aggregate/agg
apply
map
argmax/argmin
case_when
dropna
duplicated
sum/std/var
cat/str/dt
name
empty
fillna
groupby
info
_qcut
replace
reset_index
set_axis
rename
sort_values
unstack
where
value_counts
shift
squeeze
_prepare_inter_op
_to_datetime
to_list
_create_or_update_from_compiler
to_frame
_to_pandas
to_numpy
setitem
repr

Because of differences in __getattr__ logic, we need to add dt, str, and columns to _ATTRS_NO_LOOKUP in this PR to prevent additional queries from being incurred. This will create slightly different behavior for attempting to access a named index element directly:

>>> s = pd.Series(["a", "b"], index=["str", "columns"])
>>> s.columns
AttributeError: 'Series' object has no attribute 'columns'
>>> s.to_pandas().columns  # DIFFERENT: treats as index access
'b'
>>> s.str
<modin.pandas.series_utils.StringMethods object at 0x135f57940>
>>> s.to_pandas().str  # SAME: gets accessor
<pandas.core.strings.accessor.StringMethods object at 0x135f93b50>

src/snowflake/snowpark/modin/pandas/__init__.py

sfc-gh-helmeleegy

LGTM, thanks!!

tests/unit/modin/test_unsupported.py

tests/integ/modin/test_to_numpy.py

tests/integ/modin/series/test_take.py

tests/integ/modin/series/test_getattr.py

src/snowflake/snowpark/modin/plugin/__init__.py

sfc-gh-yzou · 2024-09-04T21:18:34Z

src/snowflake/snowpark/modin/plugin/docstrings/base.py

@@ -1491,7 +1491,7 @@ def iloc():
        With a scalar integer.

        >>> type(df.iloc[0])
-        <class 'snowflake.snowpark.modin.pandas.series.Series'>
+        <class 'modin.pandas.series.Series'>


i felt little bit wired to have test shown the whole class path in the example, is that the only place we have this? we can fix it like this for now, but maybe later we can have a separate pr to remove those in all examples

A few other tests and docstrings (describe, info) do this; I'll do a more thorough pass in a later PR.

sfc-gh-yzou · 2024-09-04T21:19:51Z

src/snowflake/snowpark/modin/plugin/compiler/snowflake_query_compiler.py

@@ -12352,6 +12382,18 @@ def _quantiles_single_col(

        return SnowflakeQueryCompiler(internal_frame)

+    def mode(
+        self, axis: Axis = 0, numeric_only: bool = False, dropna: bool = True


can those method not implemented error be at the overwritten part? instead of query compiler like how other apis works today

I can move it to the frontend overrides, but if it is possible to push the error to the query compiler without changing existing behavior I think we should do so.

API not implemented errors today are all thrown at frontend api layer, so let's keep it the same everywhere, if we decide to do those not implemented error at query_compiler layer, let's do it the same for all API also

src/snowflake/snowpark/modin/plugin/compiler/snowflake_query_compiler.py

sfc-gh-yzou · 2024-09-04T21:26:21Z

src/snowflake/snowpark/modin/plugin/docstrings/series.py

-
-        For `Series` axis parameter is unused and defaults to 0.
-
-        >>> ser.take([0, 3], axis=1)


why this is removed now?

Silently accepting axis=1 didn't match pandas behavior. If we want to preserve the behavior of ignoring axis=1, we'd have to add back an override stub for it, but I don't think it makes sense do keep.

>>> native_pd.Series([1]).take([0], axis=1) Traceback (most recent call last): File "/Users/joshi/miniconda3/envs/snowpandas-39/lib/python3.9/site-packages/pandas/core/generic.py", line 575, in _get_axis_number return cls._AXIS_TO_AXIS_NUMBER[axis] KeyError: 1 During handling of the above exception, another exception occurred: Traceback (most recent call last): File "<stdin>", line 1, in <module> File "/Users/joshi/miniconda3/envs/snowpandas-39/lib/python3.9/site-packages/pandas/core/generic.py", line 4132, in take axis=self._get_block_manager_axis(axis), File "/Users/joshi/miniconda3/envs/snowpandas-39/lib/python3.9/site-packages/pandas/core/generic.py", line 595, in _get_block_manager_axis axis = cls._get_axis_number(axis) File "/Users/joshi/miniconda3/envs/snowpandas-39/lib/python3.9/site-packages/pandas/core/generic.py", line 577, in _get_axis_number raise ValueError(f"No axis named {axis} for object type {cls.__name__}") ValueError: No axis named 1 for object type Series

I see, do you know if it is a new behavior introduced in 2.2.* compare with 1.5.3

Yes, seems like it works in pandas 2.0.3 and fails in pandas 2.1.4.

Can you add a entry in the change log to indicate this difference?

src/snowflake/snowpark/modin/pandas/indexing.py

sfc-gh-yzou

@sfc-gh-joshi Thanks for taking care of the comment! Can you add one description in the changelog to indicate the change for take?

sfc-gh-yzou · 2024-09-05T18:42:40Z

src/snowflake/snowpark/modin/plugin/docstrings/series.py

-
-        For `Series` axis parameter is unused and defaults to 0.
-
-        >>> ser.take([0, 3], axis=1)


Can you add a entry in the change log to indicate this difference?

sfc-gh-joshi added NO-CHANGELOG-UPDATES This pull request does not need to update CHANGELOG.md NO-PANDAS-CHANGEDOC-UPDATES This PR does not update Snowpark pandas docs DO-NOT-MERGE labels Aug 30, 2024

github-actions bot added the snowpark-pandas label Aug 30, 2024

sfc-gh-joshi force-pushed the joshi/rip-series-2 branch 2 times, most recently from 293f95a to e18d646 Compare September 3, 2024 17:27

sfc-gh-joshi changed the title ~~[DRAFT] Remove series.py~~ SNOW-1063347: Remove series.py Sep 3, 2024

sfc-gh-joshi removed the DO-NOT-MERGE label Sep 3, 2024

sfc-gh-joshi marked this pull request as ready for review September 3, 2024 22:48

sfc-gh-joshi requested a review from a team as a code owner September 3, 2024 22:48

sfc-gh-joshi requested review from sfc-gh-evandenberg and sfc-gh-jkew September 3, 2024 22:48

sfc-gh-azhan reviewed Sep 4, 2024

View reviewed changes

src/snowflake/snowpark/modin/pandas/__init__.py Show resolved Hide resolved

sfc-gh-joshi force-pushed the joshi/rip-series-2 branch from be2b26c to 72cb1ca Compare September 4, 2024 18:42

sfc-gh-azhan approved these changes Sep 4, 2024

View reviewed changes

sfc-gh-helmeleegy approved these changes Sep 4, 2024

View reviewed changes

sfc-gh-yzou reviewed Sep 4, 2024

View reviewed changes

tests/unit/modin/test_unsupported.py Outdated Show resolved Hide resolved

sfc-gh-yzou reviewed Sep 4, 2024

View reviewed changes

tests/integ/modin/test_to_numpy.py Outdated Show resolved Hide resolved

sfc-gh-yzou reviewed Sep 4, 2024

View reviewed changes

sfc-gh-joshi force-pushed the joshi/rip-series-2 branch from 72cb1ca to 3958304 Compare September 4, 2024 22:05

sfc-gh-joshi requested a review from sfc-gh-yzou September 5, 2024 01:19

sfc-gh-joshi force-pushed the joshi/rip-series-2 branch from d67310f to 2f7406c Compare September 5, 2024 01:22

sfc-gh-yzou approved these changes Sep 5, 2024

View reviewed changes

sfc-gh-joshi added 7 commits September 5, 2024 11:45

fix references

c7dad40

remove series.py and move unimplemented stubs

e38d69f

add back telemetry

925bc17

fix overrides and whatever

deabfed

add overrides, fix docs

d37e3b4

fix binary ops

a4c7d61

fix iloc via to_list

21fe563

sfc-gh-joshi added 24 commits September 5, 2024 11:45

fix name, some override logic

4e7762b

fix telemetry class check

1471db9

fix telemetry tests

2e2f469

shift, add_prefix/suffix

993a035

to_frame

414d23f

accessors and relative imports

5250f63

final? fixes

3ca34c9

doctests

57b2e78

map

37e8657

telemetry cleanup

5cad4b2

fix compare + getattr

82a2edb

add telemetry back to repr

1156da6

add to_numpy and info

b2abdbe

remove series_utils.py

68c7d63

add comment for telemetry attrs

0640d12

override getattr

1d77551

fix getattr, empty

62390a1

put back qc to_list

aaddb4e

rebase timedelta check

97f0749

fix telemetry

b32fa28

remove qc to_list stub and revert test_to_numpy

513a09d

push unimplemented back to overrides

8b9f108

add tracking jira for to_numpy

5ca0761

changelog for take axis=1

643f7b9

sfc-gh-joshi force-pushed the joshi/rip-series-2 branch from 0325cb2 to 643f7b9 Compare September 5, 2024 18:45

sfc-gh-joshi enabled auto-merge (squash) September 5, 2024 18:46

fix tolist and to_dict

754d4fb

sfc-gh-joshi merged commit 20be5fb into main Sep 5, 2024
32 of 35 checks passed

sfc-gh-joshi deleted the joshi/rip-series-2 branch September 5, 2024 21:55

github-actions bot locked and limited conversation to collaborators Sep 5, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SNOW-1063347: Remove series.py #2205

SNOW-1063347: Remove series.py #2205

sfc-gh-joshi commented Aug 30, 2024 •

edited

Loading

sfc-gh-helmeleegy left a comment

sfc-gh-yzou Sep 4, 2024

sfc-gh-joshi Sep 4, 2024

sfc-gh-yzou Sep 4, 2024

sfc-gh-joshi Sep 4, 2024

sfc-gh-yzou Sep 5, 2024

sfc-gh-yzou Sep 4, 2024

sfc-gh-joshi Sep 4, 2024

sfc-gh-yzou Sep 5, 2024

sfc-gh-joshi Sep 5, 2024

sfc-gh-yzou Sep 5, 2024

sfc-gh-joshi Sep 5, 2024

sfc-gh-yzou left a comment

sfc-gh-yzou Sep 5, 2024


		For `Series` axis parameter is unused and defaults to 0.

		>>> ser.take([0, 3], axis=1)

SNOW-1063347: Remove series.py #2205

SNOW-1063347: Remove series.py #2205

Conversation

sfc-gh-joshi commented Aug 30, 2024 • edited Loading

sfc-gh-helmeleegy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sfc-gh-yzou left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sfc-gh-joshi commented Aug 30, 2024 •

edited

Loading