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.

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

SNOW-1359484 Allow write Snowpark pandas DataFrame and Series in sess… #1563

Merged
merged 8 commits into from
May 15, 2024

Conversation

sfc-gh-azhan
Copy link
Collaborator

@sfc-gh-azhan sfc-gh-azhan commented May 10, 2024

…ion.write_pandas

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

    Fixes SNOW-1359484

  2. Fill out the following pre-review checklist:

    • I am adding a new automated test(s) to verify correctness of my new code
    • I am adding new logging messages
    • I am adding a new telemetry message
    • I am adding new credentials
    • I am adding a new dependency
  3. Please describe how your code solves the related issue.

    Please write a short description of how your code change solves the related issue.

Allow user to use session.write_pandas to write Snowpark pandas objects:

  • It reuse and does not change the interface of write_pandas
  • it reuse to_snowflake to implement the write logic
  • copied main tests from test_pandas_to_df.py and use them to test writing Snowpark pandas DataFrame.

@sfc-gh-azhan sfc-gh-azhan added the NO-PANDAS-CHANGEDOC-UPDATES This PR does not update Snowpark pandas docs label May 10, 2024
@sfc-gh-azhan sfc-gh-azhan requested review from a team as code owners May 10, 2024 22:16
@sfc-gh-azhan sfc-gh-azhan added the NO-CHANGELOG-UPDATES This pull request does not need to update CHANGELOG.md label May 13, 2024
@sfc-gh-azhan sfc-gh-azhan force-pushed the azhan-write-pd-1359484 branch from 1d3d78e to 972bceb Compare May 13, 2024 18:02
Copy link
Contributor

@sfc-gh-helmeleegy sfc-gh-helmeleegy 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. Just a few minor nits.

Copy link
Contributor

@sfc-gh-helmeleegy sfc-gh-helmeleegy 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 for taking care of this!

@sfc-gh-azhan sfc-gh-azhan force-pushed the azhan-write-pd-1359484 branch from d66dc4c to ae083e8 Compare May 13, 2024 21:48
Copy link
Contributor

@sfc-gh-jrose sfc-gh-jrose left a comment

Choose a reason for hiding this comment

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

It would be nice to have a test that validates the naming conventions as well to make sure the quote_id code paths are working correctly.

It's also surprising to me that new functionality would not require a CHANGELOG update.

src/snowflake/snowpark/session.py Outdated Show resolved Hide resolved
@sfc-gh-azhan
Copy link
Collaborator Author

It would be nice to have a test that validates the naming conventions as well to make sure the quote_id code paths are working correctly.

I have integ tests to cover the case where quote_id is true or false, and the code is basically the same as the existing code:

if quote_identifiers:

It's also surprising to me that new functionality would not require a CHANGELOG update.
The change log is added in pandas_changelog.md. In the next release, we will merge them together so this is not in changelog.md.

@sfc-gh-azhan sfc-gh-azhan requested a review from sfc-gh-jrose May 14, 2024 17:26
@sfc-gh-azhan sfc-gh-azhan force-pushed the azhan-write-pd-1359484 branch from ae083e8 to 25f6064 Compare May 14, 2024 18:43
src/snowflake/snowpark/session.py Outdated Show resolved Hide resolved
src/snowflake/snowpark/session.py Show resolved Hide resolved
@sfc-gh-azhan sfc-gh-azhan requested a review from sfc-gh-jdu May 14, 2024 21:01
@sfc-gh-azhan sfc-gh-azhan merged commit 75cba10 into main May 15, 2024
31 checks passed
@sfc-gh-azhan sfc-gh-azhan deleted the azhan-write-pd-1359484 branch May 15, 2024 18:21
@github-actions github-actions bot locked and limited conversation to collaborators May 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants