Skip to content
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.

Sign up for GitHub

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

Jump to bottom

SNOW-1445832: Added support for DataFrame.stack #1821

Merged
merged 13 commits into from
Jun 26, 2024
Merged

SNOW-1445832: Added support for DataFrame.stack #1821

merged 13 commits into from
Jun 26, 2024

Conversation

sfc-gh-nkrishna
Copy link
Contributor

@sfc-gh-nkrishna sfc-gh-nkrishna commented Jun 24, 2024 •
edited
Loading

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

    Fixes SNOW-1445832

  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 adds support for DataFrame.stack

@sfc-gh-nkrishna sfc-gh-nkrishna requested a review from a team as a code owner June 24, 2024 17:57
@sfc-gh-nkrishna sfc-gh-nkrishna changed the title SNOW-1448532: Added support for DataFrame.stack SNOW-1445832: Added support for DataFrame.stack Jun 25, 2024
sfc-gh-rdurrani
sfc-gh-rdurrani requested changes Jun 25, 2024
View reviewed changes
Copy link
Contributor

@sfc-gh-rdurrani sfc-gh-rdurrani left a comment

Choose a reason for hiding this comment

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

Left some quick comments, but mostly looks good!

src/snowflake/snowpark/modin/pandas/dataframe.py Show resolved Hide resolved
src/snowflake/snowpark/modin/plugin/compiler/snowflake_query_compiler.py Show resolved Hide resolved
src/snowflake/snowpark/modin/plugin/compiler/snowflake_query_compiler.py Show resolved Hide resolved
src/snowflake/snowpark/modin/plugin/docstrings/dataframe.py Show resolved Hide resolved
tests/integ/modin/frame/test_stack.py Show resolved Hide resolved
sfc-gh-dpetersohn
sfc-gh-dpetersohn reviewed Jun 26, 2024
View reviewed changes
Copy link
Contributor

@sfc-gh-dpetersohn sfc-gh-dpetersohn left a comment

Choose a reason for hiding this comment

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

Looks great overall. Left a couple of minor comments.

docs/source/modin/dataframe.rst Outdated Show resolved Hide resolved
src/snowflake/snowpark/modin/pandas/dataframe.py
):
return self._reduce_dimension(
query_compiler=self._query_compiler.stack(level, dropna)
query_compiler=self._query_compiler.stack(level, dropna, sort)
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like we need to upstream this change, I opened an issue in Modin to track this: modin-project/modin#7327

sfc-gh-dpetersohn
sfc-gh-dpetersohn approved these changes Jun 26, 2024
View reviewed changes
Copy link
Contributor

@sfc-gh-dpetersohn sfc-gh-dpetersohn left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@sfc-gh-nkrishna
fix combine cov
5fcb496 
Signed-off-by: Naren Krishna <[email protected]>
@sfc-gh-nkrishna sfc-gh-nkrishna merged commit c96417b into main Jun 26, 2024
35 checks passed
@sfc-gh-nkrishna sfc-gh-nkrishna deleted the nkrishna-SNOW-1448532-stack branch June 26, 2024 15:52
@github-actions github-actions bot locked and limited conversation to collaborators Jun 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@sfc-gh-dpetersohn sfc-gh-dpetersohn sfc-gh-dpetersohn approved these changes

@sfc-gh-rdurrani sfc-gh-rdurrani sfc-gh-rdurrani approved these changes

Assignees
No one assigned
Labels
snowpark-pandas
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sfc-gh-nkrishna @sfc-gh-rdurrani @sfc-gh-dpetersohn