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-1344848: Suppress dynamic pivot warnings #1434

Conversation

sfc-gh-mvashishtha
Copy link
Contributor

@sfc-gh-mvashishtha sfc-gh-mvashishtha commented Apr 25, 2024

Please answer these questions before submitting your pull requests. Thanks!

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

    Fixes SNOW-1344848

  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.

    Stop warning the user about our internal usage of private preview
    dynamic pivot features. The user should have already been warned that
    Snowpark pandas is in public or private preview. They very likely don't
    know or care that we are using Snowpark DataFrame pivot() internally,
    let alone that we are using private preview features of Snowpark
    Python.

    I'm not adding a changelog note because we haven't released a version of
    Snowpark Python with this bug yet.

@sfc-gh-mvashishtha sfc-gh-mvashishtha added NO-CHANGELOG-UPDATES This pull request does not need to update CHANGELOG.md NO-PANDAS-CHANGELOG-UPDATES NO-PANDAS-CHANGEDOC-UPDATES This PR does not update Snowpark pandas docs labels Apr 25, 2024
@sfc-gh-mvashishtha sfc-gh-mvashishtha marked this pull request as ready for review April 26, 2024 02:41
@sfc-gh-mvashishtha sfc-gh-mvashishtha requested review from a team as code owners April 26, 2024 02:41
@sfc-gh-mvashishtha
Copy link
Contributor Author

I removed @sfc-gh-lmukhopadhyay because she's OOO

Copy link
Collaborator

@sfc-gh-yuwang sfc-gh-yuwang left a comment

Choose a reason for hiding this comment

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

LGTM, but I want to wait for pandas team reviewing this since we don't have tons of knowledge of it.

Copy link
Contributor

@sfc-gh-jkew sfc-gh-jkew left a comment

Choose a reason for hiding this comment

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

Too complicated of a solution for a minor problem.

src/snowflake/snowpark/_internal/utils.py Outdated Show resolved Hide resolved
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.

LGTM, but I agree with @sfc-gh-jkew's comment - think there may be an easier solution here.

Copy link
Collaborator

@sfc-gh-stan sfc-gh-stan left a comment

Choose a reason for hiding this comment

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

This seems to also surpress the warning when the user (1) imports Snowpark pandas but doesn't do anything and (2)uses dynamic pivot directly from Snowpark Python API. But I understand the code will be much more complicated to address this specific case, in the future we should work out a way to generalize this solution for if Snowpark Pandas uses an experimental/pre-GA public API from Snowpark Python, the warning should be surpressed.

@sfc-gh-mvashishtha
Copy link
Contributor Author

in the future we should work out a way to generalize this solution for if Snowpark Pandas uses an experimental/pre-GA public API from Snowpark Python, the warning should be surpressed

Sure, if this comes up at least twice again, we might want to add something like the log filter I added earlier to generalize the warning suppression here.

@sfc-gh-mvashishtha sfc-gh-mvashishtha merged commit 988279d into main Apr 30, 2024
26 checks passed
@sfc-gh-mvashishtha sfc-gh-mvashishtha deleted the mvashishtha/SNOW-1344848/suppress-dynamic-pivot-warnings branch April 30, 2024 12:00
@github-actions github-actions bot locked and limited conversation to collaborators Apr 30, 2024
@sfc-gh-evandenberg
Copy link
Collaborator

LGTM

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.

6 participants